Skip to content

Commit 6108877

Browse files
committed
stream: keep stream/iter broadcast waiters in reused lists
Broadcast notified waiting consumers by swapping #waiters for a new SafeSet on every notification, so that a consumer re-waiting while being notified was not processed twice. That happens for every chunk that consumers wait for, and each Set operation allocates: the new Set, the for...of iterator, and clear() or deleting the last entries (120-150 bytes each). The waiters are now kept in two RingBuffers that are swapped back and forth; the notified list is drained with shift() and cleared, keeping its storage. A nested notification while the spare list is in use gets a new one. A consumer detaching removes itself with indexOf()/removeAt(), as it did with delete(); it is in the list at most once, as before. Broadcast.from() fan-out, async source, two consumers, 16-byte chunks: 2036 -> 1873 bytes allocated per chunk. Assisted-by: OpenCode
1 parent 8af1485 commit 6108877

2 files changed

Lines changed: 85 additions & 10 deletions

File tree

‎lib/internal/streams/iter/broadcast.js‎

Lines changed: 33 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -124,7 +124,12 @@ class BroadcastImpl {
124124
#buffer = new RingBuffer();
125125
#bufferStart = 0;
126126
#consumers = new SafeSet();
127-
#waiters = new SafeSet(); // Consumers with pending resolve
127+
// Consumers with a pending resolve, each at most once. A RingBuffer
128+
// rather than a Set: adding, clearing and iterating a Set allocate, and
129+
// this changes for every chunk that consumers wait for.
130+
#waiters = new RingBuffer(4);
131+
// The list #notifyConsumers() swaps in for #waiters (see there).
132+
#spareWaiters = new RingBuffer(4);
128133
#ended = false;
129134
#error;
130135
#errored = false;
@@ -224,7 +229,7 @@ class BroadcastImpl {
224229

225230
function detach() {
226231
state.detached = true;
227-
self.#waiters.delete(state);
232+
self.#deleteWaiter(state);
228233
if (state.resolve) {
229234
state.resolve(new IterResult(true, undefined));
230235
}
@@ -286,7 +291,7 @@ class BroadcastImpl {
286291
const reject = takeReject();
287292
state.resolve = resolve;
288293
state.reject = reject;
289-
self.#waiters.add(state);
294+
self.#waiters.push(state);
290295
return promise;
291296
},
292297

@@ -526,11 +531,30 @@ class BroadcastImpl {
526531

527532
#notifyConsumers() {
528533
const waiters = this.#waiters;
529-
if (waiters.size === 0) return;
534+
if (waiters.length === 0) return;
530535
// Swap out the waiters list so consumers that re-wait during
531-
// resolve don't get processed twice in this cycle.
532-
this.#waiters = new SafeSet();
533-
for (const consumer of waiters) {
536+
// resolve don't get processed twice in this cycle. This runs for every
537+
// write that consumers are waiting for, so the two lists are swapped
538+
// back and forth rather than a new one created each time; a nested
539+
// call, while the spare list is in use, gets a new one.
540+
this.#waiters = this.#spareWaiters ?? new RingBuffer(4);
541+
this.#spareWaiters = null;
542+
try {
543+
this.#notifyWaiters(waiters);
544+
} finally {
545+
waiters.clear();
546+
this.#spareWaiters = waiters;
547+
}
548+
}
549+
550+
#deleteWaiter(consumer) {
551+
const index = this.#waiters.indexOf(consumer);
552+
if (index !== -1) this.#waiters.removeAt(index);
553+
}
554+
555+
#notifyWaiters(waiters) {
556+
let consumer;
557+
while ((consumer = waiters.shift()) !== undefined) {
534558
if (consumer.resolve) {
535559
const bufferIndex = consumer.cursor - this.#bufferStart;
536560
if (bufferIndex < this.#buffer.length) {
@@ -549,11 +573,11 @@ class BroadcastImpl {
549573
if (consumer.detached && this.#deleteConsumer(consumer)) {
550574
this.#tryTrimBuffer();
551575
} else if (this.#promotePending(consumer)) {
552-
this.#waiters.add(consumer);
576+
this.#waiters.push(consumer);
553577
}
554578
} else {
555579
// Still waiting -- put back
556-
this.#waiters.add(consumer);
580+
this.#waiters.push(consumer);
557581
}
558582
}
559583
}

‎test/parallel/test-stream-iter-broadcast-waiter-cleanup.js‎

Lines changed: 52 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,4 +31,55 @@ async function testDetachedWaitersAreReleased() {
3131
shared.cancel();
3232
}
3333

34-
testDetachedWaitersAreReleased().then(common.mustCall());
34+
// Consumers waiting for writes are notified once per write, across many
35+
// writes: some wait with one next() at a time, one queues several next()
36+
// calls, and one detaches while it is waiting. Every remaining consumer
37+
// gets every chunk, in order.
38+
async function testWaitersAcrossWrites() {
39+
// A 4-byte budget, so that the writer also waits for the consumers.
40+
const { writer, broadcast: shared } = broadcast({ budget: 4 });
41+
const n = 200;
42+
const readAll = async (iterator) => {
43+
const seen = [];
44+
for (;;) {
45+
const { done, value } = await iterator.next();
46+
if (done) return seen;
47+
for (const chunk of value) seen.push(chunk[0]);
48+
}
49+
};
50+
const readQueued = async (iterator) => {
51+
const seen = [];
52+
for (;;) {
53+
const results = await Promise.all(
54+
[iterator.next(), iterator.next(), iterator.next()]);
55+
for (const { done, value } of results) {
56+
if (done) return seen;
57+
for (const chunk of value) seen.push(chunk[0]);
58+
}
59+
}
60+
};
61+
const a = readAll(shared.push()[Symbol.asyncIterator]());
62+
const b = readAll(shared.push()[Symbol.asyncIterator]());
63+
const c = readQueued(shared.push()[Symbol.asyncIterator]());
64+
for (let i = 0; i < n; i++) {
65+
if (i === n / 2) {
66+
// Joins at the current position, waits, and detaches while waiting.
67+
const detaching = shared.push()[Symbol.asyncIterator]();
68+
const pending = detaching.next();
69+
await detaching.return();
70+
await pending;
71+
}
72+
await writer.write(new Uint8Array([i % 256]));
73+
if (i % 7 === 0) await new Promise(setImmediate);
74+
}
75+
await writer.end();
76+
const expected = Array.from({ length: n }, (_, i) => i % 256);
77+
assert.deepStrictEqual(await a, expected);
78+
assert.deepStrictEqual(await b, expected);
79+
assert.deepStrictEqual(await c, expected);
80+
assert.strictEqual(shared.consumerCount, 0);
81+
}
82+
83+
testDetachedWaitersAreReleased()
84+
.then(testWaitersAcrossWrites)
85+
.then(common.mustCall());

0 commit comments

Comments
 (0)