perf: lazy-init mutable Headers copy in headersContextFromRequest#388
perf: lazy-init mutable Headers copy in headersContextFromRequest#388james-elicx merged 2 commits intomainfrom
Conversation
In Workerd, new Headers(request.headers) copies the entire header map
across the V8/C++ boundary on every request, showing up as ~815ms self-
time in production profiles for header-heavy requests.
Replace the eager copy with a Proxy that forwards reads directly to the
immutable request.headers. The mutable copy is only materialised on the
first write (.set/.delete/.append), which only occurs when middleware
calls NextResponse.next({ request: { headers } }). Pure read requests
(the vast majority) now pay zero copy cost.
Cookie parsing is also deferred behind a lazy getter: the cookie string
is not split until cookies() or draftMode() is first called during the
request, rather than unconditionally on every request entry.
commit: |
james-elicx
changed the title
|
|
/bigbonk review |
![ask-bonk[bot]](https://avatars.githubusercontent.com/in/2454580?s=60&v=4){kind=small}
There was a problem hiding this comment.
Good optimization. The lazy proxy approach correctly defers the expensive new Headers(request.headers) copy until a mutation actually occurs, and cookie parsing is deferred until first access. The proxy implementation is sound — reads forward to the original immutable headers, writes materialize a copy once, and subsequent operations use that copy.
All 581 tests pass, including the 4 new regression guards.
A few observations:
MUTATING_METHODS Set allocated per request
The new Set(["set", "delete", "append"]) at line 263 is created inside headersContextFromRequest, meaning every request allocates a new Set. This is cheap but unnecessary — hoist it to module scope since the contents are static:
const MUTATING_METHODS = new Set(["set", "delete", "append"]);
export function headersContextFromRequest(request: Request): HeadersContext {
// ...use MUTATING_METHODS from module scope...Stale cookie map after direct header mutation
The lazy cookie getter memoizes on first access (if (_cookies) return _cookies). Once parsed, the _cookies map is never invalidated. applyMiddlewareRequestHeaders works correctly because it explicitly calls ctx.cookies.clear() + repopulates. But if anything outside that function calls ctx.headers.set("cookie", "new=value") directly, the cached _cookies map will be stale.
This is safe today because the only cookie-header mutation path goes through applyMiddlewareRequestHeaders, and the old code had the same disconnect (cookies were a separate eagerly-parsed Map). Worth a comment noting this invariant, but not blocking.
Minor: wasteful parse on first middleware cookie rebuild
When applyMiddlewareRequestHeaders is called and cookies haven't been accessed yet, ctx.cookies.clear() triggers getCookies(), which parses the pre-middleware cookie header into a new Map, then immediately clear()s it and repopulates from the post-middleware header. This does one unnecessary parse. Not worth fixing — it only happens on the middleware path which is already paying for the Headers copy.
Overall this is a clean, well-tested change. The proxy approach is the right tradeoff for the Workerd environment.
|
Review posted. Here's a summary of what I found: Approved — the lazy proxy approach is correct and well-tested. Three non-blocking suggestions:
|
It was being allocated on every request inside headersContextFromRequest.
1 participant
Problem
headersContextFromRequestwas profiled at 815ms self-time per request with no children — all time spent in its own body. The two culprits:new Headers(request.headers)— in Workerd, this copies the entire header map across the V8/C++ boundary. On requests with many headers (e.g. a browser sending lots of cookies), this is very slow.cookieheader on;and=on every request, even whencookies()is never called.Both operations happened unconditionally on every request entry, even for routes that never read cookies or mutate headers.
Fix
Lazy mutable Headers proxy
Replace
new Headers(request.headers)with aProxyaround the original immutablerequest.headers:get,has,entries,forEach, …) are forwarded directly torequest.headers— zero copy cost.set,delete, orappend) materialisesnew Headers(request.headers)once and caches it. All subsequent operations use the copy.The copy is only needed when middleware calls
NextResponse.next({ request: { headers } })andapplyMiddlewareRequestHeaders()sets headers on the context. That path is uncommon. Pure read requests (the vast majority) now pay zero V8/C++ boundary crossing for headers.Lazy cookie parsing
Cookie parsing is moved behind a lazy getter on the
cookiesproperty. Thecookieheader string is not split untilcookies(),draftMode(), orapplyMiddlewareRequestHeaders()is first invoked for a request. Requests that don't touch cookies skip the parse entirely.Tests
Four new test cases added to
tests/shims.test.ts:defers new Headers() copy until first write— regression guard: reads must work before any writedefers cookie parsing until first access— cookies map is not populated until accessedcookie getter reflects middleware-modified cookie header— verifiesapplyMiddlewareRequestHeaderscorrectly invalidates and rebuilds the cookie map after middleware changes thecookieheaderBehavioural compatibility
The
HeadersContextinterface is unchanged.setHeadersContext({ headers, cookies })(used by tests and legacy callers) continues to accept plainHeaders/Mapobjects without change.