Add a ring buffer for the streams' internal queues - #7405
Conversation
Every internal FIFO in the TypeScript streams implementation is a plain array dequeued with shift() or splice(0, n). V8 left-trims small arrays, so that is O(1) up to roughly 20k entries and O(n) beyond, which makes every backlog quadratic: reading 160k pre-buffered chunks takes 14 s, 160k unawaited writes 19 s, and 160k pending reads 18 s, against about 1-2 s for the C++ implementation (WS-P1-1). RingBuffer is an O(1) FIFO with indexed access: a power-of-two circular backing store allocated on the first push, doubling when full, with push/pop/shift/peek/get/trimFront/clear. trimFront(n) replaces splice(0, n); clear() drops the backing store for terminal paths. The grown store is filled by push rather than new Array(n), which above about 32k elements would start in V8's dictionary mode. Every read is bounds-checked against the size and consumed slots are overwritten with undefined, so the buffer never reads a hole and is immune to Array.prototype[N] pollution, one of the WS-P1-3 escape classes the array-backed queues were exposed to. Adapted from the RingBuffer in nodejs/node#webstream-perf (mcollina). A leaf module; the queue sites adopt it in the following commits.
StreamQueue's entries, QueueCursor's pending reads and ByteStreamCursor's pull-into descriptors were arrays dequeued with shift() and splice(0, n): #gc spliced the reclaimed prefix off the entries on every cursor advance, so reading a backlog cost O(n) per chunk once the array outgrew V8's left-trimming. They are RingBuffers now; #gc trims the reclaimed prefix in O(freed) and getEntry indexes from the head in O(1). #fillFromQueue no longer rescans the queue to count the bytes ahead of the cursor on every BYOB fill: byte entries are sized by byteLength, so the cursor's running total already is that count, prefix included, and every path that zeroes the total also detaches the cursor or empties the queue. The fill loop stops at a non-entry instead of assuming one. Probes, ms (before / after / C++): 160k pre-buffered chunks read back 14,060 / 297 / 988 80k chunks read through both tee branches 3,410 / 378 / 871 40k 1-byte chunks read into 64-byte BYOB views 1,191 / 19 / 33 160k unawaited reads, then enqueued 17,649 / 285 / 1,460 80k unawaited BYOB reads filled by one enqueue 3,506 / 245 / 919 //src/tests/streams/... and //src/wpt:streams-ts@ pass.
WritableStream's write requests and the controller's {chunk, size}
queue were arrays dequeued with shift(), so a burst of unawaited writes
cost O(n) per completed write once the arrays outgrew V8's
left-trimming. Both are RingBuffers now.
Probe, ms (before / after / C++): 160k unawaited writes drained by a
synchronous sink, 18,774 / 398 / 1,687.
//src/tests/streams/..., //src/wpt:streams-ts@, compression-ts@ and
encoding-ts@ pass.
…ng buffer The native pull conduit's request list and the chunk-snapshot FIFOs of the identity and compression streams were the remaining arrays dequeued with shift() or splice(0, 1). They are RingBuffers now, which leaves no array-backed queue in the implementation. native.ts requires the ring-buffer module, so it is no longer a leaf; ring-buffer.ts is, and the docs say so. Probes, ms (before / after / C++): 80k unawaited writes into an IdentityTransformStream, read back, 9,372 / 848 / 6,446; the same into a CompressionStream, 982 / 408 / 2,528. //src/tests/..., //src/wpt:streams-ts@, compression-ts@ and encoding-ts@ pass.
The streams suites' data volumes top out at 4,096 chunks, below the ~20k entries at which V8 stops left-trimming a shifted array, so the quadratic dequeue had no test. The new scaling suite drives each internal queue to 80k-160k entries: a pre-buffered backlog read back, alone and through both tee branches; unawaited reads satisfied by enqueues; BYOB fills over a backlog of one-byte chunks; unawaited BYOB reads filled by one enqueue; unawaited writes drained by a synchronous sink; and unawaited writes into an IdentityTransformStream read back. Every chunk or byte is index-checked. Each shape is timed at n and 8n entries (identity: 16n, since a write costs more than a dequeue there) and may grow by at most 4x the linear multiple. The ratio does not depend on the machine, unlike an absolute bound. Both implementations are linear on every shape. The suite has its own cells rather than joining the per-area suites: the C++ cell takes about 16 s (the TS cell 4 s), which would have multiplied the per-area suites' run times by 5-7x against their 15 s budget. The cells run with timeout = "long", are tagged no-asan, and have no @gc-stress variant. Red/green verified: with the pre-ring-buffer webstreams sources, six of the seven shapes fail their assertion on the TS cell with ratios of 52-500 against limits of 32-64, and the seventh runs into the target's timeout; the C++ cell passes either way.
A RingBuffer kept its peak capacity until the stream ended or errored: a stream that had once buffered 160k entries held a 2 MB slot array for the rest of its life, where a spliced array had shrunk as it drained. The store now shrinks to fit once occupancy falls to a quarter, to the smallest power of two holding twice the occupancy, and an emptied store above the initial capacity is released. Growth leaves a store half full and shrinking leaves it between a quarter and half full, so the next resize in either direction is at least a quarter of the capacity's worth of operations away: each resize is paid for by the operations since the previous one, and a buffer oscillating across a threshold cannot resize on every operation. Stores at the initial 16 slots never shrink, so small steady-state buffers keep their slots. Resizing itself is cheaper: the new store is pre-sized by setting an empty array's length, which V8 allocates in one step without ever normalizing to dictionary mode (unlike new Array(n) above ~32k elements), and the items are copied with an in-bounds loop instead of one push call per slot. The unwritten slots are holes that no read reaches. Measured in Node against the previous version, ns per operation: steady push/shift 1.8 -> 2.3; a 0<->24 oscillation 2.2 -> 5.2; a 0<->2000 oscillation 2.4 -> 3.9; a 0<->160k burst 2.6 -> 5.5. At the stream level the difference is within noise: a 200k-chunk pull-driven read 634-642 -> 632-643 ms, 4k bursts of 24 chunks 278-281 -> 285-291 ms. 64 buffers left with one entry after a 160k burst retain 0.1 MiB, down from 188.7 MiB. The WS-P1-1 probes are unchanged or faster. Model-checked against a plain array with capacity invariants (a store above the initial capacity is more than a quarter full; an emptied one is released) over 1.2M random operations. //src/tests/streams/..., //src/tests/node/stream/..., //src/wpt:streams-ts@ and the @gc-stress ts cells pass.
|
LGTM |
guybedford
left a comment
There was a problem hiding this comment.
LGTM. Verified locally: fuzzed RingBuffer against a plain-array reference (200 rounds x 5000 mixed push/pop/shift/trimFront/get/clear ops, plus burst grow/drain to exercise wrap and shrink, plus Array.prototype[N] pollution on empty reads); the scaling suite fails against main's webstreams/ (TIMEOUT, 56-127x ratios) and passes with this PR; //src/tests/streams/..., //src/tests/node/stream/..., //src/workerd/api/tests/... and //src/workerd/api/node/tests/... all pass.
A few non-blocking comment/description accuracy notes inline. One for the description itself: the #availableBytes() rescan -> remainingSize change in #fillFromQueue is a separate accounting change (not a ring-buffer substitution) and is what makes byobFillsOverBacklogScaleLinearly linear, so it deserves a line; and "filled by push rather than new Array(n)" is stale after the last commit.
|
The generated output of |
Co-authored-by: Guy Bedford <guybedford@gmail.com>
561a32a to
d865095
Compare
On CI the C++ scaling cell takes 35-60 s per variant: every shape runs 3-4x slower than locally, and the identity shape alone takes 30 s (an IdentityTransformStream write costs ~375 us there). Three variants across the x86 and ARM debug lanes ran into the 60 s budget. The ratio assertions themselves all held, on every lane. The C++ cell is the control, not the subject: the suite guards the TypeScript queues. It is now tagged off-by-default, so CI skips it and `--test_tag_filters=` runs it. Both cells move to timeout = "eternal": the TS cell took up to 29 s on the ARM debug lane, too close to 60 s under contention for an assertion that is meant to be robust to slow machines.
Every internal FIFO in the TypeScript streams implementation is a plain
array dequeued with shift() or splice(0, n). V8 left-trims small arrays,
so that is O(1) up to roughly 20k entries and O(n) beyond, which makes
every backlog quadratic: reading 160k pre-buffered chunks takes 14 s,
160k unawaited writes 19 s, and 160k pending reads 18 s, against about
1-2 s for the C++ implementation (WS-P1-1).
RingBuffer is an O(1) FIFO with indexed access: a power-of-two circular
backing store allocated on the first push, doubling when full, with
push/pop/shift/peek/get/trimFront/clear. trimFront(n) replaces
splice(0, n); clear() drops the backing store for terminal paths. The
grown store is filled by push rather than new Array(n), which above
about 32k elements would start in V8's dictionary mode.
Every read is bounds-checked against the size and consumed slots are
overwritten with undefined, so the buffer never reads a hole and is
immune to Array.prototype[N] pollution, one of the WS-P1-3 escape
classes the array-backed queues were exposed to.
An added test suite verifies that queues scale linearly.
Secondary, remaining size calculation was simplified.