-
-
Notifications
You must be signed in to change notification settings - Fork 427
fix: disable systemRouter when not using .listen() to prevent perform… #1611
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
…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
WalkthroughModified 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
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Poem
Pre-merge checks and finishing touches✅ Passed checks (5 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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. Comment |
There was a problem hiding this 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_isListeningworks, but condition could be tightened and lifecycle clarifiedThe lazy, cached
fetchplus_isListeningflag is a good way to avoid repeated compilation and to distinguish.listen()from manualBun.serve({ fetch: app.fetch }). A couple of edge cases are worth considering:
fetchcurrently disablesconfig.systemRouterwhenever 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 + }
_isListeningis set totrueinlistenand never reset. That means:
- If someone calls
app.fetch(...)before ever calling.listen(),systemRouteris permanently flipped off for that instance, even if.listen()is used later.- If an app is started with
.listen(), thenstop()is called, and later the same instance is passed toBun.serve({ fetch: app.fetch }),_isListeningis stilltrueso the fallback never triggers for the manualfetchusage.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
_isListeninginstop()so post‑stop manualfetchbehaves like a non‑listening instance.Also applies to: 8020-8032, 8083-8088
Description
Fixes #1604
This PR addresses the critical performance degradation (45x slower) when using
Bun.serve({ fetch: app.fetch })instead ofapp.listen().Problem
When
systemRouteris 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 callBun.serve()instead of.listen(), this configuration never happens, causing severe performance degradation:.listen(): ~175K RPSBun.serve()(before fix): ~2.2K RPS (45.7x slower)Solution
_isListeningflag to track whether.listen()was calledsystemRouterif.listen()wasn't called_compiledFetchto cache the compiled handlerResults
After the fix:
.listen(): ~174K RPS (unchanged)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 callingBun.serve().Testing
Tested with
wrkbenchmarking tool:Before fix:
After fix:
Screenshots
Before fix:
using .listen()

using .serve()

After fix:
using .serve()

Breaking Changes
None. This is a backward-compatible fix that only affects the behavior when
systemRouterwould have caused issues.And also all tests PASSED
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.