Skip to content

Commit a4627d9

Browse files
committed
stream: buffer single-chunk stream/iter push() writes without a batch entry
Every push() write allocated, before the chunk was buffered, a [chunk] array, a BatchEntry with a views array, and a FixedByteView; a read of several buffered writes then validated each into an array of its own and copied those into the batch it returns. That was most of what a write costs (192 of 223 bytes for a 16-byte chunk, without pointer compression). A single-chunk write() or writeSync() now buffers the chunk's FixedByteView itself, which has the byteLength a BatchEntry has; entries are told apart by `views`, which a FixedByteView lacks. writev() keeps its BatchEntry. A read validates the buffered views straight into the batch it returns, and a pending write is checked without collecting its chunks. The checks are the same as before. Allocation per write (16-byte chunks; the consumer reads the buffered writes as they come): - writeSync(), falling back to write(): 223 -> 70 bytes - await write(): 397 -> 246 bytes - strings (pooled encoding): 315 -> 185 bytes testBufferedEntriesReadTogether checks that single-chunk and batch writes read together are delivered in order, and that a view resized among them rejects the read. It also passes before this change. Assisted-by: OpenCode
1 parent 79f7b0c commit a4627d9

3 files changed

Lines changed: 81 additions & 19 deletions

File tree

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

Lines changed: 48 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -42,8 +42,10 @@ const {
4242
convertChunks,
4343
getWriterSignal,
4444
parsePullArgs,
45+
snapshotByteView,
4546
validateBatchEntry,
4647
validateBudget,
48+
validateByteView,
4749
newPendingPromise,
4850
takeReject,
4951
takeResolve,
@@ -193,11 +195,13 @@ class PushQueue {
193195
* Returns true if write completed, false if buffer is full.
194196
* @returns {boolean}
195197
*/
196-
writeSync(chunks) {
198+
// `entry` is a BatchEntry, or for a single chunk a FixedByteView (see
199+
// entryOfChunk()).
200+
writeSync(entry) {
197201
if (this.#writerState !== 'open') return false;
198202
if (this.#consumerState !== 'active') return false;
199203

200-
return this.#writeEntry(createBatchEntry(chunks));
204+
return this.#writeEntry(entry);
201205
}
202206

203207
#writeEntry(entry) {
@@ -249,7 +253,7 @@ class PushQueue {
249253
* failure.
250254
* @returns {Promise<void>}
251255
*/
252-
async writeAsync(chunks, signal) {
256+
async writeAsync(entry, signal) {
253257
// Check writer state before signal (spec order: state, then signal)
254258
if (this.#writerState === 'closed') {
255259
throw new ERR_INVALID_STATE.TypeError('Writer is closed');
@@ -268,7 +272,6 @@ class PushQueue {
268272
// Check for pre-aborted signal (after state checks per spec)
269273
signal?.throwIfAborted();
270274

271-
const entry = createBatchEntry(chunks);
272275
if (this.#writeEntry(entry)) {
273276
return;
274277
}
@@ -509,16 +512,24 @@ class PushQueue {
509512
#drain() {
510513
try {
511514
if (this.#slots.length === 1) {
512-
const result = validateBatchEntry(this.#slots.shift());
515+
const entry = this.#slots.shift();
516+
const result = entry.views === undefined ?
517+
[validateByteView(entry)] : validateBatchEntry(entry);
513518
this.#bufferedBytes = 0;
514519
return result;
515520
}
516521

522+
// The chunks of every entry, validated straight into one batch.
517523
const result = [];
518524
for (let i = 0; i < this.#slots.length; i++) {
519-
const batch = validateBatchEntry(this.#slots.get(i));
520-
for (let j = 0; j < batch.length; j++) {
521-
result[result.length] = batch[j];
525+
const entry = this.#slots.get(i);
526+
const views = entry.views;
527+
if (views === undefined) {
528+
result[result.length] = validateByteView(entry);
529+
continue;
530+
}
531+
for (let j = 0; j < views.length; j++) {
532+
result[result.length] = validateByteView(views[j]);
522533
}
523534
}
524535
this.#slots.clear();
@@ -587,7 +598,7 @@ class PushQueue {
587598
this.#bufferedBytes < this.#budget) {
588599
const pending = this.#pendingWrites.shift();
589600
try {
590-
validateBatchEntry(pending.batch);
601+
checkEntry(pending.batch);
591602
this.#slots.push(pending.batch);
592603
this.#bufferedBytes += pending.batch.byteLength;
593604
this.#bytesWritten += pending.batch.byteLength;
@@ -640,6 +651,26 @@ class PushQueue {
640651
}
641652
}
642653

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+
663+
// Check that the chunks of a buffered entry still have the byteLength they
664+
// were accepted with, as validateBatchEntry() does, without collecting them.
665+
function checkEntry(entry) {
666+
const views = entry.views;
667+
if (views === undefined) {
668+
validateByteView(entry);
669+
return;
670+
}
671+
for (let i = 0; i < views.length; i++) validateByteView(views[i]);
672+
}
673+
643674
// =============================================================================
644675
// PushWriter Implementation
645676
// =============================================================================
@@ -663,32 +694,30 @@ class PushWriter {
663694
}
664695

665696
write(chunk, options) {
666-
const bytes = toWriterUint8Array(chunk);
697+
const entry = entryOfChunk(toWriterUint8Array(chunk));
667698
const signal = getWriterSignal(options);
668699
if (!signal && this.#queue.canWriteSync()) {
669-
this.#queue.writeSync([bytes]);
700+
this.#queue.writeSync(entry);
670701
return kResolvedPromise;
671702
}
672-
return this.#queue.writeAsync([bytes], signal);
703+
return this.#queue.writeAsync(entry, signal);
673704
}
674705

675706
writev(chunks, options) {
676-
const bytes = convertChunks(chunks);
707+
const entry = createBatchEntry(convertChunks(chunks));
677708
const signal = getWriterSignal(options);
678-
if (!signal && this.#queue.writeSync(bytes)) {
709+
if (!signal && this.#queue.writeSync(entry)) {
679710
return kResolvedPromise;
680711
}
681-
return this.#queue.writeAsync(bytes, signal);
712+
return this.#queue.writeAsync(entry, signal);
682713
}
683714

684715
writeSync(chunk) {
685-
const bytes = toWriterUint8Array(chunk);
686-
return this.#queue.writeSync([bytes]);
716+
return this.#queue.writeSync(entryOfChunk(toWriterUint8Array(chunk)));
687717
}
688718

689719
writevSync(chunks) {
690-
const bytes = convertChunks(chunks);
691-
return this.#queue.writeSync(bytes);
720+
return this.#queue.writeSync(createBatchEntry(convertChunks(chunks)));
692721
}
693722

694723
end(options) {

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1070,5 +1070,6 @@ module.exports = {
10701070
validateRecordedChunks,
10711071
throwByteViewChanged,
10721072
validateByteView,
1073+
snapshotByteView,
10731074
yieldAbortable,
10741075
};

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

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,37 @@ async function testBufferedViewMutationRejected() {
4242
}
4343
}
4444

45+
// Several buffered writes, single chunks and batches, read as one batch:
46+
// they are delivered in order, and a view resized after it was written
47+
// among them rejects the read.
48+
async function testBufferedEntriesReadTogether() {
49+
{
50+
const { writer, readable } = push();
51+
writer.writeSync(Uint8Array.of(1));
52+
writer.writevSync([Uint8Array.of(2), Uint8Array.of(3)]);
53+
writer.writeSync(Uint8Array.of(4));
54+
writer.endSync();
55+
const batches = await array(readable);
56+
assert.deepStrictEqual(batches.map((c) => c[0]), [1, 2, 3, 4]);
57+
}
58+
for (const single of [true, false]) {
59+
const buffer = new ArrayBuffer(1, { maxByteLength: 2 });
60+
const { writer, readable } = push();
61+
writer.writeSync(Uint8Array.of(1));
62+
writer.writevSync([Uint8Array.of(2), Uint8Array.of(3)]);
63+
if (single) {
64+
writer.writeSync(new Uint8Array(buffer));
65+
} else {
66+
writer.writevSync([Uint8Array.of(4), new Uint8Array(buffer)]);
67+
}
68+
buffer.resize(2);
69+
await assert.rejects(
70+
readable[Symbol.asyncIterator]().next(),
71+
kResizeError,
72+
);
73+
}
74+
}
75+
4576
async function testDropOldestUsesAcceptedByteLength() {
4677
const buffer = new ArrayBuffer(16384, { maxByteLength: 16384 });
4778
const { writer, broadcast: bc } = broadcast({
@@ -336,6 +367,7 @@ Promise.all([
336367
testPipeRejectsDetachWithWriteOnly(),
337368
testFixedLengthViewOfResizedBuffer(),
338369
testBufferedViewMutationRejected(),
370+
testBufferedEntriesReadTogether(),
339371
testDropOldestUsesAcceptedByteLength(),
340372
testPendingWritesRejectResizedViews(),
341373
testBroadcastRejectsResizedBufferedView(),

0 commit comments

Comments
 (0)