Skip to content

perf: lazily materialize request headers - #389

Merged
yusukebe merged 6 commits into
honojs:mainfrom
BlankParticle:perf/lazy-request-headers
Aug 14, 2026
Merged

perf: lazily materialize request headers#389
yusukebe merged 6 commits into
honojs:mainfrom
BlankParticle:perf/lazy-request-headers

Conversation

@BlankParticle

Copy link
Copy Markdown
Contributor

@hono/node-server constructs a native Headers object as soon as req.headers is accessed
on the other hand srvx returns a lightweight header like class whose common operations read IncomingMessage.headers directly, and constructs a native Headers object when the need arises. see https://github.com/h3js/srvx/blob/4052594e76d5ead2cc4c7cf8f7fa6d5ea9558a0a/src/adapters/_node/headers.ts#L53

This implements the same behaviour in @hono/node-server and adds a benchmark for it, this makes simpler header ops upto 20% faster on my machine.

},
"dependencies": {
"@hono/node-server": "^1.19.9",
"@hono/node-server-dev": "file:../.."

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

pnpm was resolving a stale link some how, directly importing from ../../../dist/index.mjs works for dev case

Comment on lines +31 to +38
## Benchmark Environment

- **Machine**: Lenovo LOQ 15IRX9 (83DV)
- **CPU**: Intel Core i5-13450HX (10 cores, 16 threads)
- **Memory**: 24 GB
- **OS**: Arch Linux x86_64 (kernel 7.1.6)
- **Node.js**: 24.19.0

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I don't know which machine we get benchmark data from, my numbers were lower than the benchmark recorded one, so I added the env info
once we figure this out, I can replace it

Comment thread benchmarks/fetch/README.md Outdated
@yusukebe

yusukebe commented Aug 8, 2026

Copy link
Copy Markdown
Member

@BlankParticle

I'll check the details!

Btw, we had better add a benchmark to compare with other libraries, including "srvx"(I think we can win "srvx")

@usualoma

usualoma commented Aug 9, 2026

Copy link
Copy Markdown
Member

Hi @BlankParticle,
Thank you for creating this pull request!

This optimization clearly has a meaningful effect. Although not every request accesses its headers, request-header access is common enough that providing a fast path for it seems worthwhile. My current inclination is that we should merge this.

That said, the optimization does not benefit every request, and it adds a non-trivial amount of code. I think there is still room to consider whether merging it is the right overall tradeoff when taking both the performance improvement and the added complexity into account.

If we decide to merge it, I think the implementation should be adjusted as follows.

Keep the lazy Headers facade internal to incoming requests

Could we keep this facade internal to incoming request headers instead of replacing global.Headers?

The lazy optimization is useful for incoming requests, where request.headers.get() or has() can avoid constructing a native Headers. It does not benefit ordinary application-created or response Headers: in those cases the facade immediately constructs a native Headers, so new Headers() followed by set(), append(), or iteration only adds another allocation and method delegation.

Concretely, I suggest:

  • Do not replace global.Headers.
  • Make newHeadersFromIncoming() return the internal facade directly in the normal runtime, without installing the facade as the global constructor.
  • Remove the ordinary HeadersInit constructor path and the custom Symbol.hasInstance, since the facade is no longer used as the global constructor.
  • Keep only get() and has() as request-specific fast paths.
  • Let getSetCookie() and all iteration and mutation methods materialize the native Headers instance and delegate to it.

RequestHeaders.prototype can continue to inherit from the captured native Headers.prototype, so request.headers instanceof Headers remains true without replacing the global constructor or defining a custom Symbol.hasInstance.

To preserve the existing behavior for overrideGlobalObjects: false and Headers polyfills, if the live globalThis.Headers no longer matches the constructor captured when the module was initialized, newHeadersFromIncoming() should eagerly construct the headers with the live global constructor instead of returning the facade.

This limits the optimization to incoming requests, where it is useful, while keeping application-created and response Headers fully native. It also avoids changing global Headers subclass and instanceof behavior.

Use protocol-specific fast paths for lazy header reads

The lazy get() and has() paths should distinguish HTTP/1 from HTTP/2.

For HTTP/1, IncomingMessage.headers can be used directly for ordinary header names, including the benchmark’s x-test, because Node already joins their repeated values compatibly with WHATWG Headers.

However:

  • Node discards duplicates for a fixed set of HTTP/1 headers.
  • Http2ServerRequest.headers has different collapsing behavior.
  • set-cookie is represented as an array in IncomingMessage.headers.

I suggest:

  • Detect Http2ServerRequest with instanceof, consistently with the rest of this package.
  • Use IncomingMessage.headers directly for ordinary HTTP/1 header names.
  • Resolve HTTP/2 headers and HTTP/1 headers whose duplicates Node discards from rawHeaders.
  • Use the raw path for set-cookie, __proto__, and non-string values from synthesized incoming requests.
  • Snapshot rawHeaders when the raw path is first needed for HTTP/1, and at facade construction for HTTP/2.
  • Normalize and combine repeated raw values using the same separators as native Headers.
  • Reject values that native Headers would reject rather than returning them only on the lazy path.
  • Use the same cached raw snapshot for raw-path lookups and later materialization.
  • Validate invalid header names and throw TypeError directly, so a failed probe does not permanently materialize the facade.
  • Add duplicate-value regression tests for every HTTP/1 header name that Node discards by default.

The HTTP/1 fast path assumes that IncomingMessage.headers is the parser-produced object and is not mutated through c.env.incoming. Supporting such mutations while preserving snapshot semantics would require scanning or copying all headers and would remove much of the benefit of this fast path, so I think this limitation should be treated as an explicit contract of the optimization.

The facade can retain the incoming request object because it is owned by the request and has the same lifetime. This also allows the common HTTP/1 fast path to avoid accessing or copying rawHeaders unless the raw path is actually needed.

Reference implementation

For reference, I prepared these changes on the following branch:

The two commits on that branch correspond to the two suggestions above.

The branch does not need to be merged or cherry-picked as-is. It is only intended to show one possible implementation and the related tests.

@BlankParticle

Copy link
Copy Markdown
Contributor Author

@usualoma Thanks for your suggestions, I implemented the joined headers function without verifying node's own behaviour because srvx had them. I looked at your PR and it looked like you had already done the work you were suggesting and it looked right, so I just added your commit into this PR, let me know if your implementation missed anything and I will add more commits to address that

@yusukebe I added a srvx benchmark right now, I will later make a PR with more comprehensive benchmark that will cover more cases with all popular node fetch servers including @hono/node-server and srvx. And it would be run in a CI box so the env stays consistent and the numbers stay roughly the same.

@usualoma

usualoma commented Aug 9, 2026

Copy link
Copy Markdown
Member

@BlankParticle
Thank you. I'm in favor of merging this PR!

@yusukebe yusukebe left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM!

@yusukebe

Copy link
Copy Markdown
Member

@BlankParticle @usualoma

Awesome! I also thought we could improve the Node.js Adapter by using the methods srvx is using. You did well. Merging!

@yusukebe
yusukebe merged commit 5515aa0 into honojs:main Aug 14, 2026
5 checks passed
@BlankParticle
BlankParticle deleted the perf/lazy-request-headers branch August 14, 2026 06:26
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.

3 participants