stream: reduce per-chunk overhead in streams and webstreams#64312
stream: reduce per-chunk overhead in streams and webstreams#64312anonrig wants to merge 2 commits into
Conversation
fromList() re-read state.length and each chunk's length several times per call and re-loaded buffer[idx] around every copy, and read() loaded state.length three times in its read(0) check; the engine cannot fold these loads across the intervening copy and slice calls. Cache them in locals instead. Ported from Bun's fork of the same functions (src/js/internal/streams/readable.ts fromList/read), which carries these hoists on top of the shared readable-stream lineage. benchmark/compare.js against the unmodified baseline (60-run capture plus an independent 30-run repeat, Welch t-test): streams/readable-unevenread +0.65% (p=4.3e-4) / +0.59% (p=5.9e-3) and streams/pipe.js +0.74% (p=2.0e-4) / +0.52% (p=8.8e-3), with no significant regression across the captured streams benchmarks. Refs: https://github.com/oven-sh/bun/blob/main/src/js/internal/streams/readable.ts Signed-off-by: Yagiz Nizipli <yagiz@nizipli.com>
|
Review requested:
|
nodejs-github-bot
added
needs-ci
github-actions
Bot
removed
the
request-ci
anonrig
added
the
commit-queue-rebase
anonrig
added
the
author ready
anonrig
force-pushed
the
stream-perf-ring-buffer
branch
from
43e5e91 to
d4e4cf5
Compare
anonrig
added
the
commit-queue
github-actions
Bot
removed
the
request-ci
github-actions
Bot
removed
the
request-ci
| this.head = 0; | ||
| this.tail = 0; | ||
| if (this.list.length > 1024) { | ||
| this.list.length = 8; |
There was a problem hiding this comment.
For a stream that is not obeying backpressure signaling and ends up bursting, this might lead to some thrashing. Worth benchmarking a bit but not blocking.
There was a problem hiding this comment.
Benchmarked exactly that worst case: a producer ignoring backpressure that repeatedly bursts just past the 1024-slot shrink threshold (513+ pairs) and then fully drains, so every cycle shrinks 2048→8 and re-grows 8→2048 (8 doublings + re-linearize copies) — maximal thrash for the heuristic. Ring buffer (this PR) vs unmodified baseline, chunks/sec, best of 2×2000 cycles:
| burst (chunks) | baseline (array) | ring buffer | delta |
|---|---|---|---|
| 513 | 9.99M | 12.13M | +21% |
| 600 | 10.31M | 12.49M | +21% |
| 1000 | 10.73M | 12.73M | +19% |
| 5000 | 10.35M | 13.73M | +33% |
Even under maximal shrink/grow thrash the ring stays well ahead — the per-chunk savings (no wrapper alloc, no shift) dominate the ~11 array ops per 1000+ chunk cycle, and the baseline plain array pays its own re-allocation churn on the same pattern (push-growth + shift left-trimming). The 1024 threshold also means steady-state deep queues (≤512 pairs buffered) never shrink at all. Happy to tune the threshold if you'd prefer more hysteresis, but the data says the current heuristic is safe.
Resolves the tune-constants TODO from the refinement handoff. Cribs the values from nodejs/node#64312, which uses the same ring buffer for the same spec structure: initial capacity 8 (already matched) and a retain threshold of 1024 entries (was 512). The larger threshold keeps a ring that grew during a burst reusable across full drains (e.g. the buffered read n=1000 shape), while still dropping rings above the threshold when the queue empties so a one-off spike does not pin a large backing array. Comments updated to cite the upstream reference.
github-actions
Bot
removed
the
request-ci
The [[queue]] backing every default readable/writable controller was a
plain array of { value, size } wrappers consumed with
ArrayPrototypeShift, so each buffered chunk allocated a wrapper object
and each dequeue moved (or forced the engine to re-linearize) the
remaining elements; the byte controller queue paid the same shift cost
for its chunk descriptor records.
Replace the array with a power-of-two ring buffer. Default controller
queues store each entry as (value, size) in two consecutive slots, so
the per-chunk wrapper allocation disappears; the byte controller keeps
its descriptor records (they are mutated in place at the head) in
single slots. Controllers start from (and are reset to) a shared
immutable empty queue, so constructing a stream allocates no queue
storage until a chunk is actually buffered. Enqueues measured by the
internal default size algorithm (never observable by user code, always
returns 1, cannot throw) skip the algorithm call and its try/catch
entirely.
The layout mirrors what Bun/WebKit use for the same spec structure:
[[queue]] as a ring-buffer deque (WTF::Deque in Bun's
src/jsc/bindings/webcore/streams/StreamQueue.h), the pure-JS ring
buffer in Bun's src/js/internal/fifo.ts, and the trivial-size-algorithm
bypass in
src/jsc/bindings/webcore/streams/JSReadableStreamDefaultController.cpp.
benchmark/compare.js against the unmodified baseline (30-run capture
plus an independent 15-run repeat, Welch t-test, all p < 1e-5):
webstreams/pipe-to.js +12-17% across all sixteen high-water-mark
configurations, readable-read-buffered +20% (bufferSize=1) to +49%
(bufferSize=1000), readable-async-iterator +21%. No stable significant
regression across the rest of the webstreams suite: the creation.js and
readable-read.js deltas seen in the full-suite capture disappear in
isolated 60-run rechecks.
Refs: https://github.com/oven-sh/bun/blob/main/src/js/internal/fifo.ts
Signed-off-by: Yagiz Nizipli <yagiz@nizipli.com>
anonrig
force-pushed
the
stream-perf-ring-buffer
branch
from
d4e4cf5 to
c01723a
Compare
anonrig
added
request-ci
|
CI status summary for the last few runs, since the red X is misleading (each failure is a different platform, none reproduce, and none are stream-related):
Zero overlap between failing platforms across runs, and nodejs/reliability shows a 25.56% CI green rate with repo-wide infra errors ( Rather than burning another full 2h matrix on the lottery, could someone with Jenkins access resume-build just the failed |
6 participants
Two independent, behavior-preserving performance changes to the streams
implementations, one per commit (intended to land separately via
commit-queue-rebase):1.
stream: hoist repeated loads in readable pathsfromList()re-readstate.lengthand each chunk'slengthseveral times percall and re-loaded
buffer[idx]around every copy, andread()loadedstate.lengththree times in itsread(0)check. The engine cannot fold theseloads across the intervening copy/slice calls, so they are cached in locals.
This mirrors what Bun's fork of the same functions carries on top of the shared
readable-stream lineage (
bun/src/js/internal/streams/readable.ts).2.
stream: use ring buffer for WHATWG stream queuesThe
[[queue]]backing every default readable/writable controller was a plainarray of
{ value, size }wrappers consumed withArrayPrototypeShift: onewrapper allocation per buffered chunk, and element movement (or engine
re-linearization) per dequeue. The byte controller queue paid the same shift
cost for its chunk-descriptor records.
This replaces the array with a power-of-two ring buffer:
(value, size)in twoconsecutive slots — the per-chunk wrapper allocation disappears;
head) in single slots;
constructing a stream allocates no queue storage until a chunk is actually
buffered;
by user code, always returns
1, cannot throw) skip the algorithm call andits
try/catch.The layout mirrors what Bun/WebKit use for the same spec structure
(
WTF::Dequein WebCore'sStreamQueue.h, the pure-JS ring buffer in Bun'ssrc/js/internal/fifo.ts).Benchmark results
benchmark/compare.jsagainst an unmodified-HEAD baseline binary on anotherwise idle machine, Welch t-test (Rscript unavailable, so computed from
the CSV; happy to share the raw captures). Full webstreams suite at 30 runs
plus an independent 15-run repeat; streams set at 60 runs plus a 30-run
repeat. Improvements below are stable-sign across both captures.
Ring buffer (webstreams), all p < 1e-5:
webstreams/pipe-to.js(all 16 HWM configs)webstreams/readable-read-buffered.js bufferSize=1webstreams/readable-read-buffered.js bufferSize=10/100webstreams/readable-read-buffered.js bufferSize=1000webstreams/readable-async-iterator.jsReadable hoists (node:stream):
streams/readable-unevenread.jsstreams/pipe.jsNo statistically significant regression across the rest of the captured
streams/webstreams benchmarks (creation, js_transfer, readable-read,
boundaryread, bigread, uint8array, pipe-object-mode); two apparent full-suite
deltas (
creation kind='ReadableStream.tee',readable-read type='normal')disappear in isolated 60-run re-checks (p=0.50 / p=0.95) and are attributable
to thermal drift in the suite-ordered run.
Correctness
state behind
kState.test/parallel/test-stream*,test-whatwg-*stream*,test-webstream*, andtest/wpt/test-streams.jsall pass; fulltest/sequentialpasses.test-webstreams-queue-wraparound.jsdrives ringgrowth/wrap-around, drain-rewind/shrink, fractional/zero
size()accounting, BYOB partial in-place head consumption, and writable
backpressure drain through the public API only (it also passes on the
unpatched binary).