perf: cut the per-hop cost of the router dispatch chain - #366
Closed
nigrosimone wants to merge 1 commit into
Closed
Conversation
A chain of N middlewares cost N promises. _routeRequest was async and also
returned `new Promise`, so every hop allocated two, and advancing to the next
route did `resolve(this._routeRequest(...))` - resolving a promise with a
promise, which adds an adoption per hop that has to unwind back up the chain.
_routeRequest is now a thin wrapper that allocates one promise for the whole
chain and hands resolve/reject to _dispatchRoute, which walks the routes with
plain calls. Two things are kept deliberately: the microtask that resets max
call stack size every 300 routes, now driven by Promise.resolve().then() on the
same condition, and the rejection semantics - the recursion point is wrapped so
a synchronous throw still rejects instead of escaping up the stack.
getFullMountpath also joined the entire mount stack on every hop, which is
quadratic in the number of middlewares. Path-less app.use() pushes "", and
patternToRegex("", true) returns EMPTY_REGEX, so a stack of only those can skip
the join and return EMPTY_REGEX directly. Tracked with a count of non-empty
entries on the request, so anything that isn't exactly "" falls back to the
existing path.
Measured on a 100-middleware chain, 9 paired runs: 618 -> 225 ns per hop,
9.6k -> 15.5k req/sec (1.41x median). 10 middlewares: 1.069x, 8 of 9 pairs
favourable. No middlewares: 1.009x, unchanged.
Contributor
Author
Benchmark Comparison
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
routing/middlewares-100 speedup | 0.69x (MAIN) | 1.37x (PR)
The ratio understates it: uExpress gains 47% in absolute terms (17.00k -> 25.05k req/sec)
A chain of N middlewares currently costs N promises, plus a quadratic walk of the mount stack.
What was happening
One promise per hop, adopted back up the chain.
_routeRequestwasasyncand returnednew Promise, so every hop allocated two and the async wrapper had to adopt the inner one. Worse, advancing to the next route didresolve(this._routeRequest(...))resolving a promise with a promise, which adds an adoption per hop that has to unwind all the way back when the chain finally settles. With 100 middlewares that's a 100-deep adoption chain.A quadratic join.
getFullMountpathdidreq._stack.join("")on everyusehop. Path-lessapp.use(fn)pushes"", so for a chain of global middlewares the join walks the whole stack every hop just to produce""5050 element visits per request at N=100.What changed
_routeRequestis now a thin wrapper that allocates one promise for the whole chain and handsresolve/rejectto_dispatchRoute, which walks the routes with plain calls carrying the sameresolve. External callers are unchangedapplication.js,index.jsand the optimized-path handler stillawaita promise, and nested routers still cross a real promise boundary.For the join:
patternToRegex("", true)returnsEMPTY_REGEX, so a stack of only empty paths can returnEMPTY_REGEXdirectly without joining. A count of non-empty entries on the request gates it, and anything that isn't exactly""falls back to the existing path, so the fast path is equivalent by construction rather than an approximation.Two behaviours preserved deliberately:
awaitof a non-promise value; it is nowPromise.resolve().then()on the same condition._preprocessRequestis still called exactly once per route and does not touch_paramCallbacks.sizeorrouteCount, so evaluating the condition after the call is safe.async, a synchronous throw became a rejected promise._dispatchRouteis a plain function, so the recursion point is wrapped and rejects instead of letting the throw escape up the stack.Also added the
!== EMPTY_REGEXguard on the pop branch, which the push branch already had.