Skip to content

http: batch body spans per parser execute - #67187

Merged
nodejs-github-bot merged 2 commits into
nodejs:mainfrom
araujogui:http-batch-chunked-body
Nov 5, 2026
Merged

nodejs-github-bot merged 2 commits into
nodejs:mainfrom
araujogui:http-batch-chunked-body

Conversation

@araujogui

@araujogui araujogui commented Nov 1, 2026 •

Copy link
Copy Markdown
Member

Buffer body spans in on_body() and deliver them to JS once per execute() call instead of once per chunk. A body sent as many small chunks no longer costs one C++ to JS call and Buffer per chunk.

Chunks that arrive in the same read now produce a single 'data' event on IncomingMessage. Chunk boundaries were never documented, but code that treats each 'data' event as a full message could break, so this needs a CITGM run.

Buffer body spans in on_body() and deliver them to JS once per
execute() call instead of once per chunk. A body sent as many small
chunks no longer costs one C++ to JS call and Buffer per chunk.

Signed-off-by: Guilherme Araújo <arauujogui@gmail.com>
Copilot AI balanced review requested due to automatic review settings November 1, 2026 17:55
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

Review requested:

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

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota. Try requesting a review at a lower effort level.

@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 Nov 1, 2026
@araujogui araujogui added the semver-major PRs that contain breaking changes and should be released in the next major version. label Nov 1, 2026
@ronag

ronag commented Nov 1, 2026

Copy link
Copy Markdown
Member

Any benchmark/perf numbers?

@araujogui

araujogui commented Nov 1, 2026 •

Copy link
Copy Markdown
Member Author

https://ci.nodejs.org/view/Node.js%20benchmark/job/benchmark-node-micro-benchmarks/1903/

Benchmark
++ ./node-master benchmark/compare.js --set CPUSET=0-11 --analyze --old ./node-master --new ./node-pr --filter bench-parser-chunked -- http
                                                             confidence   improvement   accuracy (*)    (**)   (***)
http/bench-parser-chunked.js n=500 len=65536 chunkSize=1     ***          +3726.84 %   ±15.61%  ±21.03%  ±27.92%
http/bench-parser-chunked.js n=500 len=65536 chunkSize=16    ***          +2122.98 %   ±38.26%  ±51.57%  ±68.46%
http/bench-parser-chunked.js n=500 len=65536 chunkSize=256   ***           +522.99 %   ±40.48%  ±54.55%  ±72.41%
http/bench-parser-chunked.js n=500 len=65536 chunkSize=4096  ***            +43.47 %   ±11.18%  ±14.93%  ±19.55%

                                                                -3742.4%           0%           +3742.4%
http/bench-parser-chunked.js n=500 len=65536 chunkSize=1                         |███████████████████  +3726.84% ***
http/bench-parser-chunked.js n=500 len=65536 chunkSize=16                        |██████████░          +2122.98% ***
http/bench-parser-chunked.js n=500 len=65536 chunkSize=256                       |██                   +522.99% ***
http/bench-parser-chunked.js n=500 len=65536 chunkSize=4096                      |                     +43.47% ***

Rates were scaled by 1000x into HdrHistogram (3 significant figures).
Use --scale to adjust precision if needed.

Be aware that when doing many comparisons the risk of a false-positive
result increases. In this case, there are 4 comparisons, you can thus
expect the following amount of false-positive results:
  0.20 false positives, when considering a   5% risk acceptance (*, **, ***),
  0.04 false positives, when considering a   1% risk acceptance (**, ***),
  0.00 false positives, when considering a 0.1% risk acceptance (***)

The stars above are per-benchmark and uncorrected. Adjusting for the size of
this comparison set (Holm-Bonferroni), 4 comparisons remain significant at 5%.

@ronag
ronag requested a review from mcollina November 1, 2026 19:15

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

RSLGTM

Comment thread src/node_http_parser.cc Resolved
Comment thread src/node_http_parser.cc Resolved
@codecov

codecov Bot commented Nov 1, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 79.41176% with 7 lines in your changes missing coverage. Please review.
✅ Project coverage is 90.71%. Comparing base (97aed16) to head (8f623b9).
⚠️ Report is 73 commits behind head on main.

Files with missing lines Patch % Lines
src/node_http_parser.cc 79.41% 4 Missing and 3 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main   #67187      +/-   ##
==========================================
+ Coverage   90.67%   90.71%   +0.03%     
==========================================
  Files         797      797              
  Lines      278773   278924     +151     
  Branches    53607    53639      +32     
==========================================
+ Hits       252778   253023     +245     
+ Misses      16356    16264      -92     
+ Partials     9639     9637       -2     
Files with missing lines Coverage Δ
src/node_http_parser.cc 84.96% <79.41%> (-0.38%) ⬇️

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

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

if we could somehow avoid the callback that would also be great.

Signed-off-by: Guilherme Araújo <arauujogui@gmail.com>
@araujogui
araujogui force-pushed the http-batch-chunked-body branch from bfabf27 to 8f623b9 Compare November 3, 2026 17:58
@araujogui

Copy link
Copy Markdown
Member Author

if we could somehow avoid the callback that would also be great.

I tried passing the body to kOnMessageComplete. It breaks test-http-pause-no-dump: kOnBody's MakeCallback drains nextTicks before end, and 'data' handlers depend on that.

@araujogui araujogui added the needs-citgm PRs that need a CITGM CI run. label Nov 3, 2026

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

@mcollina

mcollina commented Nov 4, 2026

Copy link
Copy Markdown
Member

Given the change, I don't think this needs to be semver-major.

@ronag ronag 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 semver-major PRs that contain breaking changes and should be released in the next major version. labels Nov 4, 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 Nov 4, 2026
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

@araujogui araujogui added the resume-ci Add this label to resume the latest eligible Jenkins CI run on a PR with an approving review. label Nov 5, 2026
@github-actions github-actions Bot removed the resume-ci Add this label to resume the latest eligible Jenkins CI run on a PR with an approving review. label Nov 5, 2026
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

@araujogui araujogui added the resume-ci Add this label to resume the latest eligible Jenkins CI run on a PR with an approving review. label Nov 5, 2026
@github-actions github-actions Bot removed the resume-ci Add this label to resume the latest eligible Jenkins CI run on a PR with an approving review. label Nov 5, 2026
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

@mcollina mcollina added semver-minor PRs that contain new features and should be released in the next minor version. commit-queue PRs queued for automated landing through the Commit Queue. labels Nov 5, 2026
@nodejs-github-bot
nodejs-github-bot merged commit 36452f1 into nodejs:main Nov 5, 2026
80 checks passed
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

Landed in 36452f1

@nodejs-github-bot nodejs-github-bot removed the commit-queue PRs queued for automated landing through the Commit Queue. label Nov 5, 2026
aduh95 pushed a commit that referenced this pull request Nov 11, 2026
Buffer body spans in on_body() and deliver them to JS once per
execute() call instead of once per chunk. A body sent as many small
chunks no longer costs one C++ to JS call and Buffer per chunk.

Signed-off-by: Guilherme Araújo <arauujogui@gmail.com>
PR-URL: #67187
Reviewed-By: Robert Nagy <ronagy@icloud.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
nodejs-github-bot added a commit that referenced this pull request Nov 11, 2026
Notable changes:

child_process:
  * (SEMVER-MINOR) add subprocess.timedOut (Marco) #66639
crypto:
  * (SEMVER-MINOR) make X.509 inspection guards explicit (Filip Skokan) #67245
  * (SEMVER-MINOR) add X.509 generation with CSR support (Filip Skokan) #67245
dns:
  * (SEMVER-MINOR) coalesce identical concurrent lookup() requests (Orgad Shaneh) #62599
doc:
  * add efekrskl to collaborators (Efe Karasakal) #67376
  * add Philipp Dunkel to contributors (Philipp Dunkel) #67378
  * add christianaurichzm to collaborators (Christian Aurich Zanettini Martins) #67377
http:
  * (SEMVER-MINOR) batch body spans per parser execute (Guilherme Araújo) #67187
sea:
  * (SEMVER-MINOR) generate a compile cache for the VFS modules (Matteo Collina) #66768
worker:
  * (SEMVER-MINOR) preload streams into startup snapshot (Matteo Collina) #66311

PR-URL: #67415
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. needs-citgm PRs that need a CITGM CI run. semver-minor PRs that contain new features and should be released in the next minor version.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants