Repository navigation
Conversation
Iterate the headers object passed to writeHead() with ObjectKeys() instead of for...in plus ObjectHasOwn(). Both yield the same own enumerable string keys in the same order, but an object with many headers is in dictionary mode, where for...in re-checks every key with extra hash lookups. matchHeader() now returns early unless a header name has the length and case-folded first letter of a field that affects message framing, which skips the toLowerCase() call for most other names. Names that reach it are validated tokens, so this cannot miss a match. Assisted-by: Grok Signed-off-by: Yagiz Nizipli <yagiz@nizipli.com>
Collaborator
|
Review requested:
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #66679 +/- ##
==========================================
- Coverage 92.82% 90.46% -2.36%
==========================================
Files 422 791 +369
Lines 194189 277113 +82924
Branches 30027 53282 +23255
==========================================
+ Hits 180249 250701 +70452
- Misses 13614 16813 +3199
- Partials 326 9599 +9273
🚀 New features to boost your workflow:
|
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.
writeHead(status, headers)walked a plain headers object withfor...inplusObjectHasOwn(). Objects with many headers end up in dictionary mode, wherefor...inre-checks every key with extra hash lookups. This switches toObjectKeys(), which yields the same own enumerable keys in the same order.matchHeader()also returns early now unless a name has the length and case-folded first letter of a field that affects framing (Connection,Content-Length,Transfer-Encoding,Date,Expect,Trailer,Keep-Alive). That skips thetoLowerCase()call for most other headers. Names that reach it are validated tokens, so it can't miss a match.The new test covers key order and enumerability, a 600-header object, every framing field in odd casing, look-alike names, the
setHeader()path, and a real round trip.Benchmarks
writeHead()on its own, through the public API (fresh process per sample, 20 interleaved samples per binary, Welch's t-test):End to end it's small. Server CPU per request on
simple.js-style responses drops 0.6–2.7%, which isn't significant. The fullbenchmark/httpruns below (simple.js,headers.js,incoming_headers.js,end-vs-write-end.js; 10 runs each, twice) stay within noise. My machine was shared and loaded during them, so the intervals are wide (about ±15–20%).simple.js type=buffer len=102400 chunks=1 c=50 chunkedEnc=1looked 5–16% lower in the loaded runs, so I checked it on its own. With 30 rounds in rotating order, plus a second baseline arm as an A/A control, it's +2.93% by median (p=0.83) and −2.50% by mean (p=0.49). Two identical binaries differ by +1.14% (p=0.95), so that was noise. The JS work per request on that path went down (1.69 → 1.48 µs).compare.js, run 1 (10 runs)
compare.js, run 2 (10 runs, independent)