Skip to content

Commit be801cd

Browse files
committed
stream: buffer single-chunk stream/iter broadcast() and share() batches lightly
As for push() writes, broadcast() writes and the source batches share() buffers were each kept as a BatchEntry with an array of views, also for the common case of a single chunk, and broadcast's write() first wrapped its chunk in an array. createEntry() returns the chunk's FixedByteView for a single-chunk batch and a BatchEntry otherwise; validateBatchEntry() and splitBatchEntry() take either. broadcast() and share() buffer their batches with it, and push()'s writev() too. Allocation per chunk, two consumers, 16-byte chunks: - share(), sync source: 722 -> 626 bytes; async source: 1944 -> 1848 - Broadcast.from(), sync source: 868 -> 752 bytes; async: 1873 -> 1783 testBufferedBatchViewResizeRejected checks that a view resized in a buffered multi-chunk batch still rejects the read, for broadcast() and share(). It also passes before this change. Assisted-by: OpenCode
1 parent a4627d9 commit be801cd

5 files changed

Lines changed: 62 additions & 30 deletions

File tree

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

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,8 @@ const {
6464
kNullOnceOption,
6565
kResolvedPromise,
6666
convertChunks,
67-
createBatchEntry,
67+
createEntry,
68+
snapshotByteView,
6869
getProtocolMethod,
6970
getWriterSignal,
7071
getMinCursor,
@@ -684,18 +685,18 @@ class BroadcastWriter {
684685
const signal = getWriterSignal(options);
685686
// Fast path: no signal, writer open, buffer has space
686687
if (this.#canUseWriteFastPath(signal)) {
687-
const batch = createBatchEntry([converted]);
688+
const batch = snapshotByteView(converted);
688689
this.#broadcast[kWrite](batch);
689690
this.#totalBytes += batch.byteLength;
690691
return kResolvedPromise;
691692
}
692-
return this.#writeBatchSlow(createBatchEntry([converted]), signal);
693+
return this.#writeBatchSlow(snapshotByteView(converted), signal);
693694
}
694695

695696
writev(chunks, options) {
696697
const converted = convertChunks(chunks);
697698
const signal = getWriterSignal(options);
698-
const batch = createBatchEntry(converted);
699+
const batch = createEntry(converted);
699700
// Fast path: no signal, writer open, buffer has space
700701
if (this.#canUseWriteFastPath(signal)) {
701702
if (this.#state === 'open' && this.#broadcast[kWrite](batch)) {
@@ -740,7 +741,7 @@ class BroadcastWriter {
740741
writeSync(chunk) {
741742
const converted = toWriterUint8Array(chunk);
742743
if (this.#state !== 'open') return false;
743-
const batch = createBatchEntry([converted]);
744+
const batch = snapshotByteView(converted);
744745
if (this.#broadcast[kWrite](batch)) {
745746
this.#totalBytes += batch.byteLength;
746747
return true;
@@ -751,7 +752,7 @@ class BroadcastWriter {
751752
writevSync(chunks) {
752753
const converted = convertChunks(chunks);
753754
if (this.#state !== 'open') return false;
754-
const batch = createBatchEntry(converted);
755+
const batch = createEntry(converted);
755756
if (this.#broadcast[kWrite](batch)) {
756757
this.#totalBytes += batch.byteLength;
757758
return true;
@@ -761,7 +762,7 @@ class BroadcastWriter {
761762

762763
[kWriteBatchSync](chunks) {
763764
if (this.#state !== 'open') return false;
764-
const batch = createBatchEntry(chunks);
765+
const batch = createEntry(chunks);
765766
if (this.#broadcast[kWrite](batch)) {
766767
this.#totalBytes += batch.byteLength;
767768
return true;
@@ -770,7 +771,7 @@ class BroadcastWriter {
770771
}
771772

772773
[kWriteBatch](chunks, signal) {
773-
return this.#writeBatchSlow(createBatchEntry(chunks), signal);
774+
return this.#writeBatchSlow(createEntry(chunks), signal);
774775
}
775776

776777
end(options) {

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

Lines changed: 6 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ const {
3636
kNullOnceOption,
3737
kPushDefaultBudget,
3838
kResolvedPromise,
39-
createBatchEntry,
39+
createEntry,
4040
onSignalAbort,
4141
toWriterUint8Array,
4242
convertChunks,
@@ -196,7 +196,7 @@ class PushQueue {
196196
* @returns {boolean}
197197
*/
198198
// `entry` is a BatchEntry, or for a single chunk a FixedByteView (see
199-
// entryOfChunk()).
199+
// createEntry()).
200200
writeSync(entry) {
201201
if (this.#writerState !== 'open') return false;
202202
if (this.#consumerState !== 'active') return false;
@@ -651,15 +651,6 @@ class PushQueue {
651651
}
652652
}
653653

654-
// The buffered entry of a single-chunk write: the chunk's FixedByteView, which
655-
// has the byteLength a BatchEntry has, instead of a BatchEntry holding an
656-
// array with it. Writing one chunk is the common case, and this saves the
657-
// write the BatchEntry, its views array, and the [chunk] array it was made
658-
// from. Entries are told apart by `views`, which a FixedByteView lacks.
659-
function entryOfChunk(chunk) {
660-
return snapshotByteView(chunk);
661-
}
662-
663654
// Check that the chunks of a buffered entry still have the byteLength they
664655
// were accepted with, as validateBatchEntry() does, without collecting them.
665656
function checkEntry(entry) {
@@ -694,7 +685,7 @@ class PushWriter {
694685
}
695686

696687
write(chunk, options) {
697-
const entry = entryOfChunk(toWriterUint8Array(chunk));
688+
const entry = snapshotByteView(toWriterUint8Array(chunk));
698689
const signal = getWriterSignal(options);
699690
if (!signal && this.#queue.canWriteSync()) {
700691
this.#queue.writeSync(entry);
@@ -704,7 +695,7 @@ class PushWriter {
704695
}
705696

706697
writev(chunks, options) {
707-
const entry = createBatchEntry(convertChunks(chunks));
698+
const entry = createEntry(convertChunks(chunks));
708699
const signal = getWriterSignal(options);
709700
if (!signal && this.#queue.writeSync(entry)) {
710701
return kResolvedPromise;
@@ -713,11 +704,11 @@ class PushWriter {
713704
}
714705

715706
writeSync(chunk) {
716-
return this.#queue.writeSync(entryOfChunk(toWriterUint8Array(chunk)));
707+
return this.#queue.writeSync(snapshotByteView(toWriterUint8Array(chunk)));
717708
}
718709

719710
writevSync(chunks) {
720-
return this.#queue.writeSync(createBatchEntry(convertChunks(chunks)));
711+
return this.#queue.writeSync(createEntry(convertChunks(chunks)));
721712
}
722713

723714
end(options) {

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ const {
4040
const {
4141
IterResult,
4242
kMultiConsumerDefaultBudget,
43-
createBatchEntry,
43+
createEntry,
4444
getProtocolMethod,
4545
getMinCursor,
4646
onSignalAbort,
@@ -620,7 +620,7 @@ class ShareImpl {
620620
}
621621

622622
#bufferBatch(batch) {
623-
const entry = createBatchEntry(batch);
623+
const entry = createEntry(batch);
624624
// 'drop-oldest' evicts whole entries. A single pulled batch can be much
625625
// larger than the budget (for example when from() combines many values
626626
// of a sync source), and evicting it would discard every chunk in it,
@@ -948,7 +948,7 @@ class SyncShareImpl {
948948
}
949949

950950
#bufferBatch(batch) {
951-
const entry = createBatchEntry(batch);
951+
const entry = createEntry(batch);
952952
// 'drop-oldest' evicts whole entries. A single pulled batch can be much
953953
// larger than the budget (for example when from() combines many values
954954
// of a sync source), and evicting it would discard every chunk in it,

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

Lines changed: 20 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -789,6 +789,17 @@ function createBatchEntry(chunks) {
789789
return new BatchEntry(views, byteLength);
790790
}
791791

792+
// The entry a buffer (push(), broadcast(), share()) keeps for a written or
793+
// read batch: for a single chunk, its FixedByteView, which has the
794+
// byteLength a BatchEntry has, rather than a BatchEntry and an array of
795+
// views. A batch of one chunk is the common case. The two are told apart
796+
// by `views`, which a FixedByteView lacks; validateBatchEntry() and
797+
// splitBatchEntry() take either.
798+
function createEntry(chunks) {
799+
if (chunks.length === 1) return snapshotByteView(chunks[0]);
800+
return createBatchEntry(chunks);
801+
}
802+
792803
/**
793804
* Split a batch entry into consecutive entries that are each smaller than
794805
* `limit` bytes, preserving chunk order. A single chunk of `limit` bytes or
@@ -799,7 +810,9 @@ function createBatchEntry(chunks) {
799810
*/
800811
function splitBatchEntry(entry, limit) {
801812
const { views } = entry;
802-
if (entry.byteLength < limit || views.length < 2) return undefined;
813+
if (entry.byteLength < limit || views === undefined || views.length < 2) {
814+
return undefined;
815+
}
803816
const entries = [];
804817
let current = [];
805818
let byteLength = 0;
@@ -818,9 +831,11 @@ function splitBatchEntry(entry, limit) {
818831
}
819832

820833
function validateBatchEntry(entry) {
821-
const chunks = new Array(entry.views.length);
822-
for (let i = 0; i < entry.views.length; i++) {
823-
chunks[i] = validateByteView(entry.views[i]);
834+
const views = entry.views;
835+
if (views === undefined) return [validateByteView(entry)];
836+
const chunks = new Array(views.length);
837+
for (let i = 0; i < views.length; i++) {
838+
chunks[i] = validateByteView(views[i]);
824839
}
825840
return chunks;
826841
}
@@ -1048,6 +1063,7 @@ module.exports = {
10481063
convertChunks,
10491064
checkFixedBatchChunk,
10501065
createBatchEntry,
1066+
createEntry,
10511067
fixedBatchToEntry,
10521068
snapshotFixedBatch,
10531069
recordChunk,

‎test/parallel/test-stream-iter-resizable-buffers.js‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -134,6 +134,29 @@ async function testBroadcastRejectsResizedBufferedView() {
134134
await assert.rejects(writer.end(), kResizeError);
135135
}
136136

137+
// The same, for a view buffered in a batch of several chunks.
138+
async function testBufferedBatchViewResizeRejected() {
139+
{
140+
const buffer = new ArrayBuffer(1, { maxByteLength: 2 });
141+
const { writer, broadcast: bc } = broadcast();
142+
const iterator = bc.push()[Symbol.asyncIterator]();
143+
assert.strictEqual(
144+
writer.writevSync([Uint8Array.of(1), new Uint8Array(buffer)]), true);
145+
buffer.resize(2);
146+
await assert.rejects(iterator.next(), kResizeError);
147+
await assert.rejects(writer.end(), kResizeError);
148+
}
149+
{
150+
const buffer = new ArrayBuffer(1, { maxByteLength: 2 });
151+
const shared = share([[Uint8Array.of(1), new Uint8Array(buffer)]]);
152+
const first = shared.pull()[Symbol.asyncIterator]();
153+
const second = shared.pull()[Symbol.asyncIterator]();
154+
assert.strictEqual((await first.next()).value.length, 2);
155+
buffer.resize(2);
156+
await assert.rejects(second.next(), kResizeError);
157+
}
158+
}
159+
137160
async function testShareRejectsResizedBufferedView() {
138161
const asyncBuffer = new ArrayBuffer(1, { maxByteLength: 2 });
139162
const shared = share([[new Uint8Array(asyncBuffer)]]);
@@ -372,6 +395,7 @@ Promise.all([
372395
testPendingWritesRejectResizedViews(),
373396
testBroadcastRejectsResizedBufferedView(),
374397
testShareRejectsResizedBufferedView(),
398+
testBufferedBatchViewResizeRejected(),
375399
testConsumersRejectResizedViews(),
376400
testConsumersRejectDetachedViews(),
377401
testPipeRejectsWriterResize(),

0 commit comments

Comments
 (0)