Conversation
Collaborator
|
Review requested:
|
mcollina
force-pushed
the
http-parser-lookup-header-cache
branch
from
September 20, 2026 07:45
d12ac94 to
2263e95
Compare
Member
Author
|
@nodejs/diagnostics ptal. Not sure if anyone is monkeypatching those callbacks on the fly. |
ronag
reviewed
Sep 20, 2026
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
force-pushed
the
http-parser-lookup-header-cache
branch
from
September 20, 2026 07:58
2263e95 to
781531a
Compare
ronag
approved these changes
Sep 20, 2026
Collaborator
Codecov Report✅ All modified and coverable lines are covered by tests. 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
🚀 New features to boost your workflow:
|
anonrig
approved these changes
Sep 20, 2026
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.
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_bodyandon_message_completedo anobject()->Get()for their JS callback (kOnHeadersComplete/kOnBody/kOnMessageComplete) on every message. Those callbacks are assigned once when the parser object is created (in theparsersFreeList factory) and never change afterwards, so the lookup is cached per parser in av8::Global. The cache is cleared inInit()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::kInternalizedinstead ofkNormal. 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 fulltest-http-*,test-http2-*,test-https-*,test-net-*,test-stream-*andasync-hookssuites pass (1254 tests), plus AsyncLocalStorage propagation is unaffected.Benchmark (i7-7700, server pinned to one core,
wrk -t2 -c50on separate physical cores, CPU from/proc/<pid>/stat, 5 interleaved rounds):A small (~2%) but consistent reduction.
—-
AI generated, humanly reviewed.