Skip to content

Conversation

@SimoHypers
Copy link

@SimoHypers SimoHypers commented Dec 9, 2025

Description

Fixes #1604

This PR addresses the critical performance degradation (45x slower) when using Bun.serve({ fetch: app.fetch }) instead of app.listen().

Problem

When systemRouter is enabled by default (which it is on Bun), Elysia's compiled fetch handler expects Bun's native router to be configured. However, when users manually call Bun.serve() instead of .listen(), this configuration never happens, causing severe performance degradation:

  • With .listen(): ~175K RPS
  • Manual Bun.serve() (before fix): ~2.2K RPS (45.7x slower)

Solution

  • Added _isListening flag to track whether .listen() was called
  • Modified the lazy fetch compilation to automatically disable systemRouter if .listen() wasn't called
  • Used _compiledFetch to cache the compiled handler

Results

After the fix:

  • With .listen(): ~174K RPS (unchanged)
  • Manual Bun.serve() (after fix): ~104K RPS (46x improvement)

The remaining performance difference (104K vs 174K) is expected, as .listen() provides additional optimizations like static routes and Bun's native router that aren't available when manually calling Bun.serve().

Testing

Tested with wrk benchmarking tool:

Before fix:

# Manual Bun.serve()
Requests/sec: 2,243

After fix:

# With .listen()
Requests/sec: 174,324

# Manual Bun.serve()  
Requests/sec: 104,544

Screenshots

Before fix:

using .listen()
image

using .serve()
image

After fix:

using .serve()
image

Breaking Changes

None. This is a backward-compatible fix that only affects the behavior when systemRouter would have caused issues.
And also all tests PASSED

Summary by CodeRabbit

  • Refactor
    • Optimized internal fetch compilation with lazy caching to reduce overhead and improve performance.
    • Enhanced internal state management for listener tracking and system resource coordination.

✏️ Tip: You can customize this high-level summary in your review settings.

…ance degradation

- Add _isListening flag to track if .listen() was called
- Disable systemRouter automatically when using manual Bun.serve()
- Use _compiledFetch to cache compiled handler and prevent recompilation
- Fixes performance degradation where it used to be from 175K RPS to 2K RPS when using manual Bun.serve(), now from 175K to 105k

Resolves elysiajs#1604
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Dec 9, 2025

Walkthrough

Modified Elysia's core request handling to implement lazy, cached fetch compilation and listening state tracking. The fetch function now compiles once and reuses the result, while systemRouter is conditionally disabled during compilation if the instance hasn't started listening yet.

Changes

Cohort / File(s) Summary
Fetch compilation caching and listening state
src/index.ts
Added _isListening flag to track server state. Introduced _compiledFetch for lazy, cached fetch compilation. Updated fetch method to compile once and reuse the compiled function. Modified listen to set _isListening = true. Conditionally disables systemRouter during fetch compilation when not yet listening.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

  • Specific areas requiring attention:
    • Verify the lazy compilation logic correctly handles edge cases where fetch is called before/after listen
    • Confirm systemRouter disabling during compilation doesn't break expected router behavior
    • Ensure the caching mechanism properly invalidates if routes are modified after compilation
    • Check for any race conditions between multiple fetch invocations and the listening state flag

Poem

🐰 Fetch once, compile once, cache it well,
No more recompilation's bell,
Listen first, then systemRouter rest,
Lazy loading puts us to the test,
Bun's now flying, performance blessed! ✨

Pre-merge checks and finishing touches

✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed Title accurately summarizes the main change: disabling systemRouter when .listen() is not used to fix performance issues.
Linked Issues check ✅ Passed Changes directly address #1604 by adding _isListening tracking and disabling systemRouter when fetch is compiled without .listen() being called, matching the root cause analysis.
Out of Scope Changes check ✅ Passed All changes (adding _isListening flag, lazy compiled fetch caching, and systemRouter disabling logic) are directly scoped to fixing the performance regression identified in #1604.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (1)
src/index.ts (1)

224-224: System-router fallback via _isListening works, but condition could be tightened and lifecycle clarified

The lazy, cached fetch plus _isListening flag is a good way to avoid repeated compilation and to distinguish .listen() from manual Bun.serve({ fetch: app.fetch }). A couple of edge cases are worth considering:

  • fetch currently disables config.systemRouter whenever it is first called with _isListening === false, regardless of adapter type. On non‑Bun (web‑standard) adapters this flag is likely ignored, but you could make the intent explicit and avoid unnecessary mutation by additionally checking the adapter, e.g.:
-		if (!this._compiledFetch) {
-			if (!this._isListening && this.config.systemRouter) {
-				this.config.systemRouter = false
-			}
+		if (!this._compiledFetch) {
+			if (
+				!this._isListening &&
+				this.config.systemRouter &&
+				!this['~adapter'].isWebStandard
+			) {
+				this.config.systemRouter = false
+			}
  • _isListening is set to true in listen and never reset. That means:
    • If someone calls app.fetch(...) before ever calling .listen(), systemRouter is permanently flipped off for that instance, even if .listen() is used later.
    • If an app is started with .listen(), then stop() is called, and later the same instance is passed to Bun.serve({ fetch: app.fetch }), _isListening is still true so the fallback never triggers for the manual fetch usage.

If those sequences are considered out‑of‑scope that’s fine, but if you want stricter semantics of “systemRouter only when going through adapter.listen”, you might either (a) base the condition on the adapter (as above) plus whether a server has been attached, or (b) reset _isListening in stop() so post‑stop manual fetch behaves like a non‑listening instance.

Also applies to: 8020-8032, 8083-8088

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between a480edb and c5f4436.

📒 Files selected for processing (1)
  • src/index.ts (3 hunks)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Huge performance degrade from v1.2 to v.1.4 (16.8x) with default params.

1 participant