Skip to content

stream: reduce per-chunk work in WHATWG tee and BYOB reads - #64818

Merged
nodejs-github-bot merged 2 commits into
nodejs:mainfrom
mcollina:webstream-perf-round10
Jul 31, 2026
Merged

stream: reduce per-chunk work in WHATWG tee and BYOB reads#64818
nodejs-github-bot merged 2 commits into
nodejs:mainfrom
mcollina:webstream-perf-round10

Conversation

@mcollina

Copy link
Copy Markdown
Member

Two per-chunk reductions in the WHATWG streams implementation, continuing the series from #64431 / #64451.

1. Ring buffer for pending BYOB pull-into descriptors

The byte controller's [[pendingPullIntos]] list was the last per-chunk queue still backed by a plain array consumed with ArrayPrototypeShift. BYOB reads push and shift one descriptor per read, and Array.prototype.shift has real per-call cost even at length 1. This backs the list with the same lazily materialized Queue ring buffer used for the request queues in #64431, so constructing a byte stream still allocates no descriptor storage.

2. Reused tee read request

Both ReadableStreamDefaultTee and the byte tee's default-reader pull path allocated a fresh read request object (three computed-symbol methods) plus a fresh forwarding microtask closure for every chunk. Only one read is ever in flight per tee (guarded by the reading flag), so a single read request and forwarding function are now shared across all chunks, with the chunk handed over through a captured slot. The request is materialized lazily on the first pull so tee() itself stays allocation-light.

A steady-state tee benchmark is added (benchmark/webstreams/tee.js); tee previously only had creation coverage.

Benchmarks (benchmark/compare.js, 12–20 runs each, quiet machine):

                                                            confidence improvement accuracy
webstreams/tee.js type='normal' n=100000                           ***    +79.78 %  ±11.02%
webstreams/tee.js type='bytes' n=100000                            ***    +29.38 %   ±5.05%
webstreams/readable-read.js type='byob' n=100000                    **     +2.45 %   ±3.06%

All other rows (readable-read normal, read-buffered, async-iterator, creation incl. ReadableStream.tee, js_transfer) are neutral; the eager-request version of the tee change initially regressed tee creation by 24 % which is what motivated the lazy materialization (row now +0.49 % n.s.).

Gates: WPT streams/compression/encoding plus the whatwg/webstreams parallel suites all pass. One white-box test that planted a plain array as [[pendingPullIntos]] was updated to build the real Queue representation.

mcollina added 2 commits July 29, 2026 15:26
The byte controller's [[pendingPullIntos]] list was still a plain array
consumed with ArrayPrototypeShift, while every other per-chunk queue in
the WHATWG streams implementation has moved to the Queue ring buffer.
BYOB reads push and shift one descriptor per read, and Array.prototype
shift has real per-call cost even at length 1.

Back the descriptor list with the same lazily materialized Queue used
for the request queues, so constructing a byte stream still allocates
no descriptor storage.

Signed-off-by: Matteo Collina <hello@matteocollina.com>
Both readableStreamDefaultTee and the byte tee's default-reader pull
path allocated a fresh read request object (three closures) plus a
fresh forwarding microtask closure for every chunk. Only one read is
ever in flight per tee (guarded by the `reading` flag), so a single
read request and forwarding function can be shared across all chunks,
with the chunk handed over through a captured slot.

Add a steady-state tee benchmark covering the default and byte paths.

Signed-off-by: Matteo Collina <hello@matteocollina.com>
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

Review requested:

  • @nodejs/performance

@nodejs-github-bot nodejs-github-bot added needs-ci PRs that need a full CI run. web streams labels Jul 29, 2026
@mcollina mcollina added the request-ci Add this label to start a Jenkins CI on a PR. label Jul 29, 2026
@mcollina mcollina assigned aduh95 and unassigned aduh95 Jul 29, 2026
@mcollina
mcollina requested a review from aduh95 July 29, 2026 14:51
@codecov

codecov Bot commented Jul 29, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 86.95652% with 15 lines in your changes missing coverage. Please review.
✅ Project coverage is 90.15%. Comparing base (41525ab) to head (30a5834).
⚠️ Report is 36 commits behind head on main.

Files with missing lines Patch % Lines
lib/internal/webstreams/readablestream.js 86.95% 14 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main   #64818      +/-   ##
==========================================
- Coverage   90.16%   90.15%   -0.01%     
==========================================
  Files         746      746              
  Lines      242660   242691      +31     
  Branches    45720    45729       +9     
==========================================
+ Hits       218793   218803      +10     
- Misses      15360    15379      +19     
- Partials     8507     8509       +2     
Files with missing lines Coverage Δ
lib/internal/webstreams/readablestream.js 98.20% <86.95%> (+0.01%) ⬆️

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

@github-actions github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Jul 29, 2026
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

@gurgunday gurgunday added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Jul 30, 2026
@mcollina mcollina added the commit-queue Add this label to land a pull request using GitHub Actions. label Jul 30, 2026
@nodejs-github-bot nodejs-github-bot added commit-queue-failed An error occurred while landing this pull request using GitHub Actions. and removed commit-queue Add this label to land a pull request using GitHub Actions. labels Jul 31, 2026
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator
Commit Queue failed
- Loading data for nodejs/node/pull/64818
✔  Done loading data for nodejs/node/pull/64818
----------------------------------- PR info ------------------------------------
Title      stream: reduce per-chunk work in WHATWG tee and BYOB reads (#64818)
Author     Matteo Collina <matteo.collina@gmail.com> (@mcollina)
Branch     mcollina:webstream-perf-round10 -> nodejs:main
Labels     author ready, needs-ci, web streams
Commits    2
 - stream: use the ring buffer for pending BYOB pull-into descriptors
 - stream: reuse the tee read request across chunks
Committers 1
 - Matteo Collina <hello@matteocollina.com>
PR-URL: https://github.com/nodejs/node/pull/64818
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Paolo Insogna <paolo@cowtech.it>
Reviewed-By: Aviv Keller <me@aviv.sh>
Reviewed-By: Gürgün Dayıoğlu <hey@gurgun.day>
------------------------------ Generated metadata ------------------------------
PR-URL: https://github.com/nodejs/node/pull/64818
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Paolo Insogna <paolo@cowtech.it>
Reviewed-By: Aviv Keller <me@aviv.sh>
Reviewed-By: Gürgün Dayıoğlu <hey@gurgun.day>
--------------------------------------------------------------------------------
   ℹ  This PR was created on Wed, 29 Jul 2026 13:28:04 GMT
   ✔  Approvals: 4
   ✔  - Yagiz Nizipli (@anonrig) (TSC): https://github.com/nodejs/node/pull/64818#pullrequestreview-4809637182
   ✔  - Paolo Insogna (@ShogunPanda) (TSC): https://github.com/nodejs/node/pull/64818#pullrequestreview-4809873724
   ✔  - Aviv Keller (@avivkeller): https://github.com/nodejs/node/pull/64818#pullrequestreview-4810319030
   ✔  - Gürgün Dayıoğlu (@gurgunday): https://github.com/nodejs/node/pull/64818#pullrequestreview-4819552551
   ✔  Last GitHub CI successful
   ℹ  Last Full PR CI on 2026-07-29T15:37:13Z: https://ci.nodejs.org/job/node-test-pull-request/75293/
- Querying data for job/node-test-pull-request/75293/
✔  Build data downloaded
   ✔  Last Jenkins CI successful
--------------------------------------------------------------------------------
   ✔  No git cherry-pick in progress
   ✔  No git am in progress
   ✔  No git rebase in progress
--------------------------------------------------------------------------------
- Bringing origin/main up to date...
From https://github.com/nodejs/node
 * branch                  main       -> FETCH_HEAD
✔  origin/main is now up-to-date
- Downloading patch for 64818
From https://github.com/nodejs/node
 * branch                  refs/pull/64818/merge -> FETCH_HEAD
✔  Fetched commits as 598693b3a2f3..30a58345c4aa
--------------------------------------------------------------------------------
[main b648a47737] stream: use the ring buffer for pending BYOB pull-into descriptors
 Author: Matteo Collina <hello@matteocollina.com>
 Date: Wed Jul 29 14:25:10 2026 +0200
 2 files changed, 35 insertions(+), 22 deletions(-)
[main d6bbfe2165] stream: reuse the tee read request across chunks
 Author: Matteo Collina <hello@matteocollina.com>
 Date: Wed Jul 29 14:31:48 2026 +0200
 2 files changed, 129 insertions(+), 64 deletions(-)
 create mode 100644 benchmark/webstreams/tee.js
   ✔  Patches applied
There are 2 commits in the PR. Attempting autorebase.
(node:364) [DEP0190] DeprecationWarning: Passing args to a child process with shell option true can lead to security vulnerabilities, as the arguments are not escaped, only concatenated.
(Use `node --trace-deprecation ...` to show where the warning was created)
Rebasing (2/4)
Executing: git node land --amend --yes
--------------------------------- New Message ----------------------------------
stream: use the ring buffer for pending BYOB pull-into descriptors

The byte controller's [[pendingPullIntos]] list was still a plain array
consumed with ArrayPrototypeShift, while every other per-chunk queue in
the WHATWG streams implementation has moved to the Queue ring buffer.
BYOB reads push and shift one descriptor per read, and Array.prototype
shift has real per-call cost even at length 1.

Back the descriptor list with the same lazily materialized Queue used
for the request queues, so constructing a byte stream still allocates
no descriptor storage.

Signed-off-by: Matteo Collina <hello@matteocollina.com>
PR-URL: #64818
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Paolo Insogna <paolo@cowtech.it>
Reviewed-By: Aviv Keller <me@aviv.sh>
Reviewed-By: Gürgün Dayıoğlu <hey@gurgun.day>

[detached HEAD 4601132409] stream: use the ring buffer for pending BYOB pull-into descriptors
Author: Matteo Collina <hello@matteocollina.com>
Date: Wed Jul 29 14:25:10 2026 +0200
2 files changed, 35 insertions(+), 22 deletions(-)
Rebasing (3/4)
Rebasing (4/4)
Executing: git node land --amend --yes
--------------------------------- New Message ----------------------------------
stream: reuse the tee read request across chunks

Both readableStreamDefaultTee and the byte tee's default-reader pull
path allocated a fresh read request object (three closures) plus a
fresh forwarding microtask closure for every chunk. Only one read is
ever in flight per tee (guarded by the reading flag), so a single
read request and forwarding function can be shared across all chunks,
with the chunk handed over through a captured slot.

Add a steady-state tee benchmark covering the default and byte paths.

Signed-off-by: Matteo Collina <hello@matteocollina.com>
PR-URL: #64818
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Paolo Insogna <paolo@cowtech.it>
Reviewed-By: Aviv Keller <me@aviv.sh>
Reviewed-By: Gürgün Dayıoğlu <hey@gurgun.day>

[detached HEAD b6af368f67] stream: reuse the tee read request across chunks
Author: Matteo Collina <hello@matteocollina.com>
Date: Wed Jul 29 14:31:48 2026 +0200
2 files changed, 129 insertions(+), 64 deletions(-)
create mode 100644 benchmark/webstreams/tee.js
Successfully rebased and updated refs/heads/main.

ℹ Add commit-queue-squash label to land the PR as one commit, or commit-queue-rebase to land as separate commits.

https://github.com/nodejs/node/actions/runs/30637820753

@trivikr

This comment was marked as outdated.

@geeksilva97 geeksilva97 added commit-queue Add this label to land a pull request using GitHub Actions. commit-queue-squash Add this label to instruct the Commit Queue to squash all the PR commits into the first one. and removed commit-queue-failed An error occurred while landing this pull request using GitHub Actions. labels Jul 31, 2026
@nodejs-github-bot nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Jul 31, 2026
@nodejs-github-bot
nodejs-github-bot merged commit c543cfb into nodejs:main Jul 31, 2026
104 of 106 checks passed
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

Landed in c543cfb

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

Labels

author ready PRs that have at least one approval, no pending requests for changes, and a CI started. commit-queue-squash Add this label to instruct the Commit Queue to squash all the PR commits into the first one. needs-ci PRs that need a full CI run. web streams

Projects

None yet

Development

Successfully merging this pull request may close these issues.

9 participants