Skip to content

stream: avoid per-chunk promises in webstream adapters - #65548

Open
mcollina wants to merge 1 commit into
nodejs:mainfrom
mcollina:webstream-perf-round15
Open

stream: avoid per-chunk promises in webstream adapters#65548
mcollina wants to merge 1 commit into
nodejs:mainfrom
mcollina:webstream-perf-round15

Conversation

@mcollina

Copy link
Copy Markdown
Member

Continuing the WHATWG streams optimization work, this round targets the stream/webstream adapters (Readable.fromWeb(), Writable.fromWeb(), Duplex.fromWeb()), which had never been profiled.

Readable.fromWeb() and the read side of Duplex.fromWeb() allocated a promise, a read-result object, and two reaction closures for every chunk through reader.read(). Only one read is ever in flight (_read is not called again before push()), so a single reused read request now delivers chunks through readableStreamDefaultReaderRead(), forwarding each chunk in a microtask to keep the previous delivery order relative to errors and destroy.

Writable.fromWeb() and the write side of Duplex.fromWeb() paid two derived promises off writer.ready plus the writer.write() promise and a fresh closure pair per chunk. A single shared write request (the same contract pipeTo uses since #64890) now dispatches chunks directly and settles the node callback. Failures are delivered in a microtask because the callback can destroy the stream while the writable machinery is mid-transition.

Also adds benchmark/webstreams/adapters.js — the suite had no rows for the adapter paths.

Results (30 runs):

                                                         confidence improvement accuracy (*)   (**)   (***)
webstreams/adapters.js kind='readable-from-web' n=100000          *      5.07 %       ±4.84% ±6.44%  ±8.39%
webstreams/adapters.js kind='readable-to-web' n=100000                   0.92 %       ±6.25% ±8.32% ±10.83%
webstreams/adapters.js kind='writable-from-web' n=100000        ***     39.27 %       ±7.19% ±9.58% ±12.48%
webstreams/adapters.js kind='writable-to-web' n=100000                  -1.74 %       ±6.71% ±8.94% ±11.63%

The two toWeb rows are untouched paths and neutral, included for coverage. Beyond the test suite and WPT, the change was validated with a differential stress harness (error mid-write, writev with cork, erroring controller, destroy during data, pre-closed/pre-errored streams, duplex echo, slow-sink backpressure): the observable event logs are byte-identical to the previous implementation.

Readable.fromWeb() and the read side of Duplex.fromWeb() allocated a
promise, a read-result object, and two reaction closures for every
chunk through reader.read(). Only one read is ever in flight, so a
single reused read request delivers chunks through
readableStreamDefaultReaderRead() instead, forwarding each chunk in a
microtask to keep the previous delivery order relative to errors and
destroy.

Writable.fromWeb() and the write side of Duplex.fromWeb() paid two
derived promises off writer.ready plus the writer.write() promise and
a fresh closure pair per chunk. A single shared write request (the
same contract pipeTo uses) now dispatches chunks directly and settles
the node callback, with failures delivered in a microtask because the
callback can destroy the stream while the writable machinery is
mid-transition.

Also add benchmark/webstreams/adapters.js; the suite had no rows for
the adapter paths.

                                       confidence improvement accuracy (*)   (**)  (***)
webstreams/adapters.js kind='readable-from-web' n=100000    *      5.07 %       ±4.84% ±6.44%  ±8.39%
webstreams/adapters.js kind='readable-to-web' n=100000             0.92 %       ±6.25% ±8.32% ±10.83%
webstreams/adapters.js kind='writable-from-web' n=100000  ***     39.27 %       ±7.19% ±9.58% ±12.48%
webstreams/adapters.js kind='writable-to-web' n=100000            -1.74 %       ±6.71% ±8.94% ±11.63%

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 Aug 25, 2026
@codecov

codecov Bot commented Aug 25, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 90.62500% with 15 lines in your changes missing coverage. Please review.
✅ Project coverage is 90.12%. Comparing base (7aaf9b4) to head (5095bc9).

Files with missing lines Patch % Lines
lib/internal/webstreams/adapters.js 90.44% 14 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main   #65548      +/-   ##
==========================================
- Coverage   90.13%   90.12%   -0.01%     
==========================================
  Files         751      751              
  Lines      253639   253714      +75     
  Branches    47790    47797       +7     
==========================================
+ Hits       228618   228663      +45     
- Misses      16264    16309      +45     
+ Partials     8757     8742      -15     
Files with missing lines Coverage Δ
lib/internal/webstreams/readablestream.js 87.21% <100.00%> (+<0.01%) ⬆️
lib/internal/webstreams/adapters.js 88.03% <90.44%> (+1.42%) ⬆️

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

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

Labels

needs-ci PRs that need a full CI run. web streams

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants