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.

@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 Issues and PRs related to the Web Streams API. 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.00%. Comparing base (7aaf9b4) to head (9c2217a).
⚠️ Report is 156 commits behind head on main.

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.00%   -0.14%     
==========================================
  Files         751      757       +6     
  Lines      253639   257451    +3812     
  Branches    47790    48799    +1009     
==========================================
+ Hits       228618   231715    +3097     
- Misses      16264    16831     +567     
- Partials     8757     8905     +148     
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 121 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.

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(); a single reused read request now delivers
chunks through readableStreamDefaultReaderRead() instead, forwarding
each chunk in a microtask to preserve the 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 now
dispatches chunks directly and settles the node callback, with failures
delivered in a microtask.

Signed-off-by: Matteo Collina <hello@matteocollina.com>
@mcollina
mcollina force-pushed the webstream-perf-round15 branch from 5095bc9 to 9c2217a Compare September 2, 2026 11:00

@gurgunday gurgunday 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 added the request-ci Add this label to start a Jenkins CI on a PR. label Sep 3, 2026
@github-actions github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Sep 3, 2026
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

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 Issues and PRs related to the Web Streams API.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants