Skip to content

http: cache parser callbacks - #66152

Open
mcollina wants to merge 1 commit into
nodejs:mainfrom
mcollina:http-parser-lookup-header-cache
Open

mcollina wants to merge 1 commit into
nodejs:mainfrom
mcollina:http-parser-lookup-header-cache

Conversation

@mcollina

@mcollina mcollina commented Sep 20, 2026

Copy link
Copy Markdown
Member

Two low-level reductions in the HTTP parser's per-request work, continuing the server hot-path cleanup from #65732, #65749 and #65802.

Cache the per-message callback lookups. on_headers_complete, on_body and on_message_complete do an object()->Get() for their JS callback (kOnHeadersComplete / kOnBody / kOnMessageComplete) on every message. Those callbacks are assigned once when the parser object is created (in the parsers FreeList factory) and never change afterwards, so the lookup is cached per parser in a v8::Global. The cache is cleared in Init() so a parser object that is re-initialized with different callbacks — which direct users of the binding do — stays correct.

Intern header field-name strings. When building the header array, field names are created with NewStringType::kInternalized instead of kNormal. Header field names repeat heavily across requests on a keep-alive connection (Host, User-Agent, Accept, …), so interning lets V8's string table return the same string object for each rather than allocating a fresh one per request. Header values are unique per request and stay non-interned.

No observable behavior change: field names keep their original case in rawHeaders, duplicate/unknown/mixed-case header handling is unchanged, and the full test-http-*, test-http2-*, test-https-*, test-net-*, test-stream-* and async-hooks suites pass (1254 tests), plus AsyncLocalStorage propagation is unaffected.

Benchmark (i7-7700, server pinned to one core, wrk -t2 -c50 on separate physical cores, CPU from /proc/<pid>/stat, 5 interleaved rounds):

workload main this PR
hello world, 1 request header ~24.9 µs/req ~24.4 µs/req
hello world, 8 request headers ~28.5 µs/req ~28.0 µs/req (lower in all 5 rounds)

A small (~2%) but consistent reduction.

—-

AI generated, humanly reviewed.

@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

Review requested:

  • @nodejs/http
  • @nodejs/net

@nodejs-github-bot nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. http_parser Issues and PRs related to the HTTP parser implementation or http_parser binding. needs-ci PRs that need a full CI run. labels Sep 20, 2026
@mcollina
mcollina force-pushed the http-parser-lookup-header-cache branch from d12ac94 to 2263e95 Compare September 20, 2026 07:45
@mcollina

Copy link
Copy Markdown
Member Author

@nodejs/diagnostics ptal. Not sure if anyone is monkeypatching those callbacks on the fly.

Comment thread src/node_http_parser.cc Outdated
Cache the kOnHeadersComplete, kOnBody and kOnMessageComplete callback
lookups per parser instead of doing an object property Get on every
message. These callbacks are assigned once when the parser object is
created and never change; the cache is cleared in Init() so
re-initialized parsers (direct binding users) stay correct.

No observable behavior change; all header/parser tests pass. On a
keep-alive server this is a small but consistent CPU reduction.

Signed-off-by: Matteo Collina <hello@matteocollina.com>
@mcollina
mcollina force-pushed the http-parser-lookup-header-cache branch from 2263e95 to 781531a Compare September 20, 2026 07:58
@mcollina mcollina changed the title http: cache parser callbacks, intern header names http: cache parser callbacks Sep 20, 2026
@mcollina mcollina added the request-ci Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review. label Sep 20, 2026
@github-actions github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review. label Sep 20, 2026
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

@codecov

codecov Bot commented Sep 20, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 90.28%. Comparing base (bf9605d) to head (781531a).
⚠️ Report is 26 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff            @@
##             main   #66152    +/-   ##
========================================
  Coverage   90.28%   90.28%            
========================================
  Files         790      790            
  Lines      271642   272030   +388     
  Branches    51853    51934    +81     
========================================
+ Hits       245241   245612   +371     
+ Misses      16918    16906    -12     
- Partials     9483     9512    +29     
Files with missing lines Coverage Δ
src/node_http_parser.cc 85.32% <100.00%> (+0.15%) ⬆️

... and 45 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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

c++ Issues and PRs that require attention from people who are familiar with C++. http_parser Issues and PRs related to the HTTP parser implementation or http_parser binding. needs-ci PRs that need a full CI run.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants