Uh oh!
There was an error while loading. Please reload this page.
stream: keep webstream stream states in fast-mode objects - #65625
stream: keep webstream stream states in fast-mode objects#65625mcollina wants to merge 2 commits into
Conversation
nodejs-github-bot
commented
Aug 28, 2026
Review requested:
|
Codecov Reportβ
All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@## main #65625 +/- ##
==========================================
- Coverage 90.13% 90.05% -0.08%
==========================================
Files 751 754 +3 Lines 253639 256415 +2776 Branches 47790 48509 +719 ==========================================
+ Hits 228618 230921 +2303 - Misses 16264 16620 +356 - Partials 8757 8874 +117
π New features to boost your workflow:
|
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Abhirup0
left a comment
There was a problem hiding this comment.
LGTM. Verified with %HasFastProperties() that state instances maintain fast-mode Maps without falling back to dictionary mode.
One minor observation: the writableState.state === 'erroring' branch in writeContinuation (lib/internal/webstreams/transformstream.js:628β633) is currently unhit in tests. It might be worth adding a quick test case where a write is parked under backpressure and the writable stream is aborted prior to the next read/pull.
36d534f to
961f2ceComparemcollina
commented
Aug 30, 2026
@Abhirup0 added test/parallel/test-whatwg-transformstream-parked-write-error.js covering that branch via readable.cancel(), controller.error(), and controller.terminate() with a parked write in flight (verified with an instrumented build that all three hit it). Interesting detail: writer.abort() alone does not reach it β the abort settles the in-flight write through the writable's erroring machinery without the backpressure flip β and a write issued before the start algorithm settles never parks at all, so the scenarios wait for start first. |
Benchmark GHA (webstreams / lifecycle.js): https://github.com/nodejs/node/actions/runs/33296873656 Results
Benchmark results:
|
961f2ce to
9f6906dCompareThe 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. Assisted-by: ChatGPT Signed-off-by: Matteo Collina <hello@matteocollina.com>
9f6906d to
2943389Comparenodejs-github-bot
commented
Sep 1, 2026
nodejs-github-bot
commented
Sep 2, 2026
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>
2943389 to
aaa47dcCompare
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.