GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — http: cut per-message outgoing overhead by anonrig · Pull Request #64346 · nodejs/node

Conversation

@anonrig

@anonrig anonrig commented Jul 7, 2026

Copy link
Copy Markdown
Member

Behavior-preserving performance work on lib/_http_outgoing.js, reducing
per-message and per-header overhead on the outgoing-message hot paths:

  • Cache the lenient-header-validation decision per message. It was
    re-derived on every setHeader/appendHeader call (and per addTrailers
    key) through a six-branch this.req?.socket?.server? walk, although the
    inputs are fixed once the message is constructed. The decision is now
    resolved lazily on first use and cached on the instance.
  • Pre-filter the known-field matcher on (length, first character).
    matchHeader lower-cased every outgoing header name of length 4–17 just
    to feed a switch with eight known fields. Ordinary headers
    (X-Request-Id, Cache-Control, …) now skip the per-header
    toLowerCase() allocation; the filter only rejects names that cannot
    possibly match (| 0x20 lower-cases ASCII letters and maps no other token
    character onto a letter, and every known-field length is enumerated).
  • Load the socket prototype accessor once per body write instead of
    invoking the getter three times in write_.
  • Hoist repeated property loads the engine cannot fold across calls:
    _header in _send, headers.length across processHeader calls,
    _keepAliveTimeout/_maxRequestsPerSocket/_contentLength in
    _storeHeader, value.length in processHeader.
  • Flush corked chunked buffers through a shared callback runner instead
    of allocating a fresh closure per flush.

The hoists mirror what Bun's fork of these files carries on top of the shared
lineage (bun/src/js/node/_http_outgoing.ts: write_ msgSocket local,
_send header local, _storeHeader/processHeader length locals,
runChunkCallbacks, flat one-shot setHeader validation); the matcher
pre-filter is profiling-driven on top of that set.

Benchmarks

The stock benchmark/http serving benchmarks route through a load generator
(test-double/autocannon); on my machine their run-to-run CV is 8–25%, which
cannot resolve effects of this size — across ~60 captured configs they show
consistently positive means and zero statistically significant
regressions
(headers, incoming_headers, chunked, end-vs-write-end,
client-request-body, create-clientrequest, check_*; Welch t-test on
benchmark/compare.js CSVs, 30–90 runs).

The claimed effect is from mechanism benchmarks that drive the shipped
classes through the public API (http.createServer + new ServerResponse(req) + setHeader/flushHeaders), one fresh process per
sample, 30 interleaved samples per binary, Welch t-test:

case improvement p
flushHeaders() with five headers, per message +8.25% 6.2e-11
24-header response (_storeHeader dominated) +4.26% 6.7e-7
tight setHeader loop (three headers × 2e6) +2.64% 2.0e-3

Sign-stable in an independent 15-sample repeat (+6.8% / +4.7% / +0.5%).
Raw CSVs and the analysis scripts are available on request.

Correctness

  • No observable behavior change: header order/content, error types, and
    event ordering are preserved; the validation-leniency inputs are fixed at
    message construction, so caching the derived boolean is not observable
    through supported usage.
  • All test/parallel/test-http-*, test-http.js, and test-https-* pass.
  • The new test-http-outgoing-known-header-casing.js locks the matcher
    semantics: known fields keep being recognized in any casing
    (CoNnEcTiOn, CONTENT-LENGTH), and unknown names sharing a known
    field's length and first letter (Dote, Xonnection) are emitted
    verbatim without affecting framing. The test also passes on the
    unpatched binary.

AI assistance disclosure

This is an AI-assisted pull request. The survey of Bun's implementation, the
implementation, the benchmark methodology and analysis, the regression test,
and this description were produced by an AI agent (Grok, xAI) operating
under my direction; commits carry Assisted-by: Grok. I reviewed and take
responsibility for every change.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

styfle reacted with rocket emoji
All reactions
  • 🚀 1 reaction
@nodejs-github-bot

nodejs-github-bot commented Jul 7, 2026

Copy link
Copy Markdown
Collaborator

Review requested:

  • @nodejs/http
  • @nodejs/net
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@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 Jul 7, 2026
@anonrig anonrig 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 Jul 7, 2026
@anonrig
anonrig requested review from jasnell, joyeecheung, mcollina and ronag July 7, 2026 20:25
jasnell
jasnell reviewed Jul 7, 2026
View reviewed changes
Comment thread lib/_http_outgoing.js Resolved

Uh oh!

There was an error while loading. Please reload this page.

jasnell
jasnell reviewed Jul 7, 2026
View reviewed changes
Comment thread lib/_http_outgoing.js Resolved

Uh oh!

There was an error while loading. Please reload this page.

mcollina
mcollina reviewed Jul 7, 2026
View reviewed changes
Comment thread lib/_http_outgoing.js Outdated
const cached = this[kLenientValidation];
if (cached !== undefined)
return cached;
return this[kLenientValidation] = lenientHeaderValidation(this);

@mcollina mcollina Jul 7, 2026

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

So we need to call this per message? Can this be calculated once to the server when this is constructed?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

@anonrig anonrig Jul 7, 2026

Copy link
Copy Markdown
Member Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Partially — the decision is not server-scoped in general: for ClientRequest it depends on the per-request httpValidation/insecureHTTPParser options, and for ServerResponse on the server options plus the global flag, so the per-message lazy cache is the common denominator that covers both sides uniformly (one slot read per call after the first). We could additionally cache the server-side walk on the Server instance and seed the message cache from it at construction, but that only saves the one-time six-branch walk per response — happy to do it as a follow-up if you think it's worth the extra plumbing.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
jasnell
jasnell reviewed Jul 7, 2026
View reviewed changes
Comment thread lib/_http_outgoing.js
case 14: if (c !== 0x63) return; break; // Content-Length
case 17: if (c !== 0x74) return; break; // Transfer-Encoding
default: return; // No known field has this length
}

@jasnell jasnell Jul 7, 2026

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Not blocking but I doubt this is going to save much. There's also an inherent risk of false positives (e.g. that first clause also matches any other 4-letter string that starts with d). My fear is that this is straining into unnecessary micro-optimization territory.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

@anonrig anonrig Jul 7, 2026

Copy link
Copy Markdown
Member Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

On false positives: they're safe by design — a 4-letter d-starting name that isn't date passes the pre-filter and then falls through the toLowerCase()+switch exactly as before; the filter only skips work for names that provably cannot match, it never misclassifies (test-http-outgoing-known-header-casing.js locks exactly this: Dote/Xonnection etc. are emitted verbatim, odd-cased known fields still recognized).

On whether it saves much: this line is what carries most of the PR's measured effect. Without it, the hoists alone were inside the noise (+0.9%, n.s.); with it, the mechanism benchmarks (fresh process per sample, 30 interleaved samples per binary, Welch t-test, re-confirmed against a clean rebuilt baseline on an idle machine) show flushHeaders with five headers +10.8% (p=1.5e-25) and a 24-custom-header response +6.0% (p=5.1e-12) — the per-header lowercase allocation dominates _storeHeader for responses with ordinary X-*/custom headers, which never match the switch. Happy to drop it if you feel the extra branching isn't worth it, but the data says it's the payload of this PR rather than the garnish.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

@jasnell jasnell Jul 7, 2026

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

I'm not worried about safety. Raw performance isn't the only goal. Keeping code easily readable, maintainable, etc is also important. Running a local artificial benchmark here, the impact is noticeable in isolation but shows zero statistically relevant impact in any of the http benchmarks.

I suspect caching the lenient header validation is likely a more meaningful change.

Not blocking, but I'm not convinced the additional complexity is actually worthwhile.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
jasnell
jasnell approved these changes Jul 7, 2026
View reviewed changes
@joharikapil31-ops

This comment was marked as low quality.

Sign in to view
@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 Jul 7, 2026
@nodejs-github-bot

nodejs-github-bot commented Jul 7, 2026

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/74645/

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@anonrig
http: cut per-message outgoing overhead
85c887a
Cache the lenient-header-validation decision per message instead of
re-deriving it through the six-branch req/socket/server walk on every
setHeader/appendHeader call (and per addTrailers key); the inputs are
fixed once the message is constructed. Pre-filter the known-field
matcher on (length, first character) so ordinary headers skip the
per-header toLowerCase() allocation; the filter only rejects names that
cannot match. Load the `socket` prototype accessor once per body write
instead of three times, hoist the repeated _header/_keepAliveTimeout/
_maxRequestsPerSocket/_contentLength/headers.length loads the engine
cannot fold across calls, and flush corked chunked buffers through a
shared callback runner instead of allocating a closure per flush.

The hoists mirror what Bun's fork of these files carries on top of the
shared lineage (bun/src/js/node/_http_outgoing.ts: write_ msgSocket,
_send header local, _storeHeader/processHeader length locals,
runChunkCallbacks, flat one-shot setHeader validation).

Mechanism benchmarks driving the shipped classes through the public API
(fresh process per sample, 30 interleaved samples per binary, Welch
t-test): flushHeaders with five headers +8.25% (p=6.2e-11), a 24-header
response +4.26% (p=6.7e-7), tight setHeader loop +2.64% (p=2.0e-3);
sign-stable in an independent 15-sample repeat. No statistically
significant regression across the captured benchmark/http suite
(headers, incoming_headers, chunked, end-vs-write-end,
client-request-body, create-clientrequest, check_*).

The new test locks the matcher semantics: known fields keep being
recognized in any casing, and unknown names sharing a known field's
length and first letter are emitted verbatim.

Refs: https://github.com/oven-sh/bun/blob/main/src/js/node/_http_outgoing.ts
Assisted-by: Grok
Signed-off-by: Yagiz Nizipli <[email protected]>
@anonrig
anonrig force-pushed the http-outgoing-overhead branch from dbb8fbe to 85c887a Compare July 7, 2026 22:08
@anonrig anonrig 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 Jul 7, 2026
@anonrig anonrig mentioned this pull request Jul 7, 2026
http: reduce per-request server overhead #64348
Open
@anonrig

anonrig commented Jul 7, 2026

Copy link
Copy Markdown
Member Author

CI note: the current Jenkins wave went red on 14 subjobs at once (every linked-* containered variant plus aix/smartos/fedora/rhel9). This is infrastructure, not the diff: #64312 — unrelated, previously green three times, untouched since — went red in the same window on two different platforms (osx, linked-openssl111), and a full local test/parallel sweep on this exact head (85c887a) shows no failures beyond the known flaky clusters already tracked in nodejs/reliability. Holding off on re-running CI until the infra recovers, then will do a single clean request-ci.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@codecov

codecov Bot commented Jul 7, 2026 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 98.75000% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 90.25%. Comparing base (4ee2117) to head (85c887a).
⚠️ Report is 63 commits behind head on main.

Files with missing lines Patch % Lines
lib/_http_outgoing.js 98.75% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main   #64346      +/-   ##
==========================================
+ Coverage   90.23%   90.25%   +0.01%     
==========================================
  Files         741      741              
  Lines      240979   241026      +47     
  Branches    45401    45411      +10     
==========================================
+ Hits       217449   217529      +80     
+ Misses      15112    15065      -47     
- Partials     8418     8432      +14     
Files with missing lines Coverage Δ
lib/_http_outgoing.js 97.70% <98.75%> (+0.08%) ⬆️

... and 30 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.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@MuruganBalachandran

This comment was marked as low quality.

Sign in to view
@anonrig
anonrig requested a review from mcollina July 8, 2026 14:01
@anonrig

anonrig commented Jul 9, 2026

Copy link
Copy Markdown
Member Author

cc @jasnell @ronag @mcollina can i get a review on this please?

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

ronag
ronag approved these changes Jul 9, 2026
View reviewed changes
@anonrig anonrig added request-ci Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review. and removed request-ci Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review. labels Jul 9, 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 Jul 9, 2026
@nodejs-github-bot

nodejs-github-bot commented Jul 9, 2026

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/74696/

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

pimterry
pimterry approved these changes Jul 9, 2026
View reviewed changes
@coderabbitai coderabbitai Bot mentioned this pull request Jul 9, 2026
node:http: emit writeHead raw-array headers in the caller's order oven-sh/bun#33870
Closed
mcollina
mcollina approved these changes Jul 13, 2026
View reviewed changes

@mcollina mcollina left a comment

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

lgtm

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
@nodejs-github-bot

nodejs-github-bot commented Jul 14, 2026

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/74799/

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@nodejs-github-bot

nodejs-github-bot commented Jul 14, 2026

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/74817/

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@anonrig

anonrig commented Jul 14, 2026

Copy link
Copy Markdown
Member Author

@nodejs/build @nodejs/distros RHEL and DEBIAN builds are failing with errors like this...

10:58:18 CONFLICT (add/add): Merge conflict in tools/doc/package-lock.json
10:58:18 Auto-merging tools/doc/package.json
10:58:18 CONFLICT (add/add): Merge conflict in tools/doc/package.json
10:58:18 Auto-merging tools/eslint/package-lock.json
10:58:18 CONFLICT (add/add): Merge conflict in tools/eslint/package-lock.json
10:58:18 Auto-merging tools/eslint/package.json
10:58:18 CONFLICT (add/add): Merge conflict in tools/eslint/package.json
10:58:18 Auto-merging tools/nix/pkgs.nix
10:58:18 CONFLICT (add/add): Merge conflict in tools/nix/pkgs.nix
10:58:18 Auto-merging tools/v8_gypfiles/v8.gyp
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@kapouer

kapouer commented Jul 14, 2026

Copy link
Copy Markdown
Contributor

@nodejs/build @nodejs/distros RHEL and DEBIAN builds are failing with errors like this...

10:58:18 CONFLICT (add/add): Merge conflict in tools/doc/package-lock.json
10:58:18 Auto-merging tools/doc/package.json
10:58:18 CONFLICT (add/add): Merge conflict in tools/doc/package.json
10:58:18 Auto-merging tools/eslint/package-lock.json
10:58:18 CONFLICT (add/add): Merge conflict in tools/eslint/package-lock.json
10:58:18 Auto-merging tools/eslint/package.json
10:58:18 CONFLICT (add/add): Merge conflict in tools/eslint/package.json
10:58:18 Auto-merging tools/nix/pkgs.nix
10:58:18 CONFLICT (add/add): Merge conflict in tools/nix/pkgs.nix
10:58:18 Auto-merging tools/v8_gypfiles/v8.gyp

What do you mean by that ? What is this Auto-merging step ?
How can I reproduce ?

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@GetThatCookie GetThatCookie mentioned this pull request Aug 3, 2026
http,net,stream: reconcile read, write, and parser paths #64980
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jasnell jasnell jasnell approved these changes
@mcollina mcollina mcollina approved these changes
@pimterry pimterry pimterry approved these changes
@ronag ronag ronag approved these changes
@joyeecheung joyeecheung Awaiting requested review from joyeecheung

At least 1 approving review is required to merge this pull request.

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

http Issues and PRs related to the http subsystem. needs-ci PRs that need a full CI run.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

9 participants

@anonrig @nodejs-github-bot @joharikapil31-ops @MuruganBalachandran @kapouer @mcollina @jasnell @pimterry @ronag
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.
You are visitor 01402520 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.