Skip to content

Speed up HTTP header materialization - #66257

Open
pimterry wants to merge 3 commits into
nodejs:mainfrom
pimterry:http-header-perf
Open

pimterry wants to merge 3 commits into
nodejs:mainfrom
pimterry:http-header-perf

Conversation

@pimterry

Copy link
Copy Markdown
Member

When you read headers from a request or response, it's lazily instantiated by iterating over raw headers and calling _addHeaderLine to combine each value into the final object. This method calls matchKnownFields to check how duplicate headers should be combined based on various known header names.

To do this, matchKnownFields returned a composite string embedding the merge rule: a 0/1/2 control character defining the merge rule (combine with comma, with semi-colon, or set-cookie array special case) concatenated with the header name itself, like \u0002cookie. Only known single-value headers (like host) were passed through untouched.

Then, in _addHeaderLine we had to check and and read the control character, slice out the rest of the header name, and manage duplicates according to the relevant control char rule.

This results in a string concat, chatAt(0) check, and then slice(1) with a new string allocation for each header in most cases. These fresh string slices are then each used as keys in the resulting headers object, which is itself expensive for fresh strings. Each of these is more allocations and work on a hot & frequently used path.

With this PR, I've replaced the control char approach with pregenerated { name, merge } objects with a fixed string & merge value for each known header. Unknown headers are returned as lowercase strings (so they keep lowercasing as before when required, but lose the \u0000 concat and then slice step). Then matchKnownFields just returns the existing object, and _addHeaderLine just checks the merge policy and uses the existing string from the pregenerated object. No allocations on the hot path for known headers or lowercase unknown headers.

This PR also fixes the incoming_headers benchmark, which shadowed the headers param so this was never actually used, and extends it with a new param to test performance when req.headers is read.

I've looked at other extensions like precalculating more of this in C++ up front - I think this captures most of the benefit here already quite simply with no downside, so we can leave exploring that more to future PRs.

Microbenchmarking, the change here speeds up req.headers and res.headers up to 45% in the best case (a small set of known headers) down to about 7% speed-up for sets of mostly unknown headers.

End to end, I see incoming headers with headers=20 and req.headers boosting RPS up by 4%, using the existing (but fixed) incoming_headers benchmark example. In best cases (small sets of all-known headers) this boost RPS for me up to 13% end to end. No performance regressions visible anywhere else.

Previously the inner 'headers' variable shadowed the outer, meaning that
headers=20 sends the same 7 headers as headers=0.

Signed-off-by: Tim Perry <pimterry@gmail.com>
Signed-off-by: Tim Perry <pimterry@gmail.com>
Signed-off-by: Tim Perry <pimterry@gmail.com>
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

Review requested:

  • @nodejs/http
  • @nodejs/net
  • @nodejs/performance

@nodejs-github-bot nodejs-github-bot added http Issues and PRs related to the http subsystem. needs-ci PRs that need a full CI run. labels Sep 24, 2026
@github-actions

Copy link
Copy Markdown
Contributor

Benchmark GHA (http / incoming_headers.js): https://github.com/nodejs/node/actions/runs/35988785748

@codecov

codecov Bot commented Sep 24, 2026 •

Copy link
Copy Markdown

Codecov Report

βœ… All modified and coverable lines are covered by tests.
βœ… Project coverage is 90.28%. Comparing base (a2c8da5) to head (55626c6).
⚠️ Report is 82 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main   #66257   +/-   ##
=======================================
  Coverage   90.28%   90.28%           
=======================================
  Files         789      789           
  Lines      272878   272934   +56     
  Branches    52097    52103    +6     
=======================================
+ Hits       246360   246421   +61     
+ Misses      16975    16967    -8     
- Partials     9543     9546    +3     
Files with missing lines Coverage Ξ”
lib/_http_incoming.js 98.18% <100.00%> (+0.18%) ⬆️

... and 24 files with indirect coverage changes

πŸš€ New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • πŸ“¦ JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@pimterry

Copy link
Copy Markdown
Member Author

Benchmark job is broken until #66284 hits main I think, but the headline number is easy to repro:

  • Check out this branch locally
  • Build as node, build base for comparison as node-main.
  • ./node benchmark/compare.js --old ./node-main --new ./node --runs 50 \
      --filter incoming_headers.js --set w=0 --set benchmarker=autocannon --analyze -- http

On my machine gives 3-4% boost for requests that read req.headers, 0% impact on those that don't.

@efekrskl efekrskl 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.

Impressive! Just the day before I was going through this piece of code wondering if this could be improved. I guess I gave up too soon πŸ₯²

I ran the benchmarks, and I can also confirm approx 4% boost for requests that read req.headers

@gurgunday gurgunday 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

@panva panva added author ready PRs with CI started, the required approvals, and no outstanding review comments. request-ci Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review. labels Sep 25, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

author ready PRs with CI started, the required approvals, and no outstanding review comments. http Issues and PRs related to the http subsystem. needs-ci PRs that need a full CI run. request-ci Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants