stream: keep webstream stream states in fast-mode objects - #65625
Open
mcollina wants to merge 2 commits into
Open
stream: keep webstream stream states in fast-mode objects#65625mcollina wants to merge 2 commits into
mcollina wants to merge 2 commits into
Conversation
The per-stream state records were built as object literals with
__proto__: null, which V8 creates as dictionary-mode objects: roughly
50x slower to allocate, and every subsequent property load on them is a
dictionary lookup. These records back every hot path, so both stream
construction and per-chunk field accesses were paying for it.
Replace the literals with classes whose prototype has a null prototype,
so instances stay in fast mode while Object.prototype remains excluded
from the lookup chain. Every field ever assigned is declared up front
so the shape never transitions.
Also add benchmark/webstreams/lifecycle.js covering the short-lived
stream pattern (create, few chunks, close) that first exposed this.
confidence improvement accuracy (*) (**) (***)
webstreams/creation.js kind='ReadableStream' n=50000 *** 138.13 % ±14.29% ±19.14% ±25.17%
webstreams/creation.js kind='TransformStream' n=50000 *** 133.64 % ±8.61% ±11.46% ±14.94%
webstreams/creation.js kind='WritableStream' n=50000 *** 204.00 % ±7.70% ±10.26% ±13.37%
webstreams/lifecycle.js kind='pipe-through' n=50000 *** 96.41 % ±3.41% ±4.58% ±6.03%
webstreams/lifecycle.js kind='readable' n=50000 *** 80.04 % ±3.10% ±4.15% ±5.45%
webstreams/pipe-through.js kind='default' n=500000 *** 91.16 % ±2.56% ±3.43% ±4.52%
webstreams/pipe-to.js highWaterMarkW=1 highWaterMarkR=1 n=500000 *** 110.48 % ±2.91% ±3.89% ±5.11%
webstreams/readable-read.js type='normal' n=100000 *** 34.63 % ±4.79% ±6.38% ±8.31%
Signed-off-by: Matteo Collina <hello@matteocollina.com>
A transform sink write arriving under backpressure parked the chunk together with a PromiseWithResolvers record whose promise was returned to the writable controller and later resolved with the perform-transform promise. The writable's write reactions already exist before the write algorithm runs, so the parked write can instead return the parked-result sentinel and have the continuation wire the perform-transform promise directly to those reactions, dropping the per-chunk promise record and the thenable adoption hop. Failures while erroring are delivered in a microtask, preserving the old rejection position. Signed-off-by: Matteo Collina <hello@matteocollina.com>
Collaborator
|
Review requested:
|
nodejs-github-bot
added
needs-ci
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #65625 +/- ##
==========================================
- Coverage 90.13% 90.07% -0.07%
==========================================
Files 751 751
Lines 253639 255020 +1381
Branches 47790 48157 +367
==========================================
+ Hits 228618 229703 +1085
- Misses 16264 16482 +218
- Partials 8757 8835 +78
🚀 New features to boost your workflow:
|
H4ad
approved these changes
Aug 28, 2026
Comment on lines
+40
to
+41
| await new ReadableStream(makeSource()) | ||
| .pipeTo(new WritableStream({ write() {} })); |
Member
There was a problem hiding this comment.
nit: unlikely but we can add asserts for dead-code elimination
Comment on lines
+1422
to
+1428
| // The state records are classes whose prototype chain ends at null | ||
| // instead of `__proto__: null` object literals: the literals fall back | ||
| // to dictionary-mode objects in V8 (~50x slower to create, and every | ||
| // later property load is a dictionary lookup), while class instances | ||
| // stay in fast mode with the same protection against Object.prototype | ||
| // pollution. Every field ever assigned is declared so the shape never | ||
| // transitions. |
Member
There was a problem hiding this comment.
nit:
I remember to read few comments like this in the past due to assumptions of V8 at the time, but they usually are very niche/unique/different than a normal implementation
This one does not look weird/hacky, so I would say we could just remove those comments since they don't agreggate much value and you have added benchmarks so we can catch regressions
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
While profiling short-lived streams (the per-request create → few chunks → close pattern),
createReadableStreamStateshowed up at 20% self time. The cause: the per-stream state records are object literals with__proto__: null, which V8 creates as dictionary-mode objects — roughly 50x slower to allocate (~500-1500ns vs ~30ns), and every subsequent property load on them is a dictionary lookup. These records back every hot path, so both construction and per-chunk[kState]accesses have been paying for it.The first commit replaces the four per-stream state literals (and their nested transfer records) with classes whose prototype has a null prototype: instances stay in fast mode, while
Object.prototyperemains excluded from the lookup chain, preserving the pollution protection the literals were there for. Every field ever assigned is declared up front so the shape never transitions. (The controller and reader states were already plain literals and unaffected.) It also addsbenchmark/webstreams/lifecycle.jscovering the short-lived stream pattern that exposed this.The second commit removes the per-chunk
PromiseWithResolversfor transform sink writes parked on backpressure: the writable controller's write reactions exist before the write algorithm runs, so the parked-write continuation can wire the perform-transform promise directly to them via the parked-result sentinel introduced in #65143, dropping the promise record and the thenable adoption hop.Results (30 runs, all 36 rows significant at
***, no regressions — full table below):Validated with the full webstreams test suite, WPT streams/compression/encoding, and two differential stress harnesses (adapter scenarios and transform parked-write error/abort/terminate/reentrancy scenarios) whose observable event logs are byte-identical to the previous implementation.
Other
lib/modules use the same__proto__: nullliteral pattern on hot paths; I'll follow up separately after auditing them.