Skip to content

Commit 4ceb468

Browse files
committed
stream: check stream/iter pipe writes more cheaply
pipeTo() and pipeToSync() check that each chunk is not resized or detached while the writer uses it: around writeSync() with callWithByteView() for batches of one chunk, and with a batch entry of snapshots, one object for every chunk, for larger batches written one chunk at a time. A non-empty view of a fixed-length, non-shared ArrayBuffer can only change by the buffer being detached, which makes its byteLength 0 (see FixedByteView). For such views, check only the byteLength around writeSync(), and snapshot batches as a FixedBatch of their chunks and byteLengths, checking each buffer once when chunks share it. Other views are checked as before. Piping a sync generator yielding 16-byte chunks one per batch is about 15% faster with pipeTo() and 19% faster with pipeToSync(); with batches of 128 chunks, pipeTo() is about 28% faster and pipeToSync() 20%. Assisted-by: OpenCode Signed-off-by: James M Snell <jasnell@gmail.com>
1 parent 599ca8b commit 4ceb468

3 files changed

Lines changed: 167 additions & 0 deletions

File tree

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

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -62,10 +62,13 @@ const {
6262
kResolvedPromise,
6363
kStart,
6464
callWithByteView,
65+
checkFixedBatchChunk,
6566
createBatchEntry,
6667
createOperationQueue,
68+
fixedBatchToEntry,
6769
isTransformObject,
6870
parsePullArgs,
71+
snapshotFixedBatch,
6972
snapshotTransform,
7073
toUint8Array,
7174
validateBatchEntry,
@@ -1546,6 +1549,24 @@ function pipeToSync(source, ...args) {
15461549
totalBytes += TypedArrayPrototypeGetByteLength(chunk);
15471550
continue;
15481551
}
1552+
if (!hasWritevSync) {
1553+
// Chunks written one at a time: snapshot them without an object for
1554+
// every chunk when possible.
1555+
const fixed = snapshotFixedBatch(batch);
1556+
if (fixed !== null) {
1557+
for (let i = 0; i < batch.length; i++) {
1558+
const chunk = checkFixedBatchChunk(fixed, i);
1559+
const accepted = writer.writeSync(chunk);
1560+
checkFixedBatchChunk(fixed, i);
1561+
if (accepted === false) {
1562+
throw new ERR_OUT_OF_RANGE(
1563+
'write', 'within byte budget', 'budget exhausted');
1564+
}
1565+
totalBytes += fixed.byteLengths[i];
1566+
}
1567+
continue;
1568+
}
1569+
}
15491570
const entry = createBatchEntry(batch);
15501571
if (hasWritevSync && batch.length > 1) {
15511572
const accepted = writer.writevSync(validateBatchEntry(entry));
@@ -1672,6 +1693,21 @@ async function pipeTo(source, ...args) {
16721693
// Write a batch using try-fallback: sync first, async if needed.
16731694
// Returns undefined on sync success, or a Promise when async fallback
16741695
// is required. Callers must check: const p = writeBatch(b); if (p) await p;
1696+
// Write a FixedBatch with writeSync(), like the loop at the end of
1697+
// writeBatch(), falling back to the async path for the rest of the batch.
1698+
function writeFixedBatch(fixed) {
1699+
const { chunks } = fixed;
1700+
for (let i = 0; i < chunks.length; i++) {
1701+
const chunk = checkFixedBatchChunk(fixed, i);
1702+
if (!writer.writeSync(chunk)) {
1703+
checkFixedBatchChunk(fixed, i);
1704+
return writeBatchAsyncFallback(fixedBatchToEntry(fixed), i);
1705+
}
1706+
checkFixedBatchChunk(fixed, i);
1707+
totalBytes += fixed.byteLengths[i];
1708+
}
1709+
}
1710+
16751711
function writeBatch(batch) {
16761712
// Single chunk, the common case: check the view around writeSync()
16771713
// without allocating a batch entry, and create one only to fall back to
@@ -1684,6 +1720,10 @@ async function pipeTo(source, ...args) {
16841720
}
16851721
return writeBatchAsyncFallback(createBatchEntry(batch), 0);
16861722
}
1723+
if (!hasWritev && hasWriteSync) {
1724+
const fixed = snapshotFixedBatch(batch);
1725+
if (fixed !== null) return writeFixedBatch(fixed);
1726+
}
16871727
const entry = createBatchEntry(batch);
16881728
if (hasWritev && batch.length > 1) {
16891729
if (!hasWritevSync ||

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

Lines changed: 79 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -468,6 +468,20 @@ function validateByteView(snapshot) {
468468
*/
469469
function callWithByteView(value, method, receiver) {
470470
const buffer = TypedArrayPrototypeGetBuffer(value);
471+
if (!isSharedArrayBuffer(buffer) && !ArrayBufferPrototypeGetResizable(buffer)) {
472+
// A non-empty view of a fixed-length, non-shared ArrayBuffer can only
473+
// change by the buffer being detached, which makes its byteLength 0 (see
474+
// FixedByteView), so checking its byteLength is enough.
475+
const byteLength = TypedArrayPrototypeGetByteLength(value);
476+
if (byteLength !== 0) {
477+
const result = FunctionPrototypeCall(method, receiver, value);
478+
if (TypedArrayPrototypeGetByteLength(value) !== byteLength) {
479+
throw new ERR_INVALID_STATE.TypeError(
480+
'Byte view was resized or detached after being accepted');
481+
}
482+
return result;
483+
}
484+
}
471485
if (isSharedArrayBuffer(buffer)) {
472486
const snapshot = snapshotByteView(value);
473487
const result = FunctionPrototypeCall(method, receiver, value);
@@ -607,6 +621,68 @@ function validateRecordedChunks(chunks, checks) {
607621
return chunks;
608622
}
609623

624+
// A batch accepted for writing one chunk at a time: its chunks, and their
625+
// byteLengths when accepted (see snapshotFixedBatch()).
626+
function FixedBatch(chunks, byteLengths, byteLength) {
627+
this.chunks = chunks;
628+
this.byteLengths = byteLengths;
629+
this.byteLength = byteLength;
630+
}
631+
FixedBatch.prototype = ObjectFreeze({ __proto__: null });
632+
633+
/**
634+
* Snapshot a batch of Uint8Arrays like createBatchEntry(), without an object
635+
* for every chunk, when every chunk is a non-empty view of a fixed-length,
636+
* non-shared ArrayBuffer: such views can only change by being detached,
637+
* which makes their byteLength 0 (see FixedByteView), so their byteLengths
638+
* are all that needs to be recorded. Check a chunk with checkFixedBatchChunk()
639+
* before and after using it.
640+
* @param {Uint8Array[]} chunks
641+
* @returns {FixedBatch|null} null if a chunk needs a full snapshot.
642+
*/
643+
function snapshotFixedBatch(chunks) {
644+
const count = chunks.length;
645+
const byteLengths = new Array(count);
646+
let byteLength = 0;
647+
let checkedBuffer;
648+
for (let i = 0; i < count; i++) {
649+
const view = chunks[i];
650+
const buffer = TypedArrayPrototypeGetBuffer(view);
651+
// Chunks often share a buffer: check each buffer once.
652+
if (buffer !== checkedBuffer) {
653+
if (isSharedArrayBuffer(buffer) ||
654+
ArrayBufferPrototypeGetResizable(buffer)) {
655+
return null;
656+
}
657+
checkedBuffer = buffer;
658+
}
659+
const length = TypedArrayPrototypeGetByteLength(view);
660+
if (length === 0) return null;
661+
byteLengths[i] = length;
662+
byteLength += length;
663+
}
664+
return new FixedBatch(ArrayPrototypeSlice(chunks), byteLengths, byteLength);
665+
}
666+
667+
function checkFixedBatchChunk(batch, index) {
668+
const chunk = batch.chunks[index];
669+
if (TypedArrayPrototypeGetByteLength(chunk) !== batch.byteLengths[index]) {
670+
throw new ERR_INVALID_STATE.TypeError(
671+
'Byte view was resized or detached after being accepted');
672+
}
673+
return chunk;
674+
}
675+
676+
// The batch entry of a FixedBatch, to continue writing it asynchronously.
677+
function fixedBatchToEntry(batch) {
678+
const { chunks, byteLengths } = batch;
679+
const views = new Array(chunks.length);
680+
for (let i = 0; i < chunks.length; i++) {
681+
views[i] = new FixedByteView(chunks[i], byteLengths[i]);
682+
}
683+
return new BatchEntry(views, batch.byteLength);
684+
}
685+
610686
function createBatchEntry(chunks) {
611687
const views = new Array(chunks.length);
612688
let byteLength = 0;
@@ -870,7 +946,10 @@ module.exports = {
870946
concatBytes,
871947
createOperationQueue,
872948
convertChunks,
949+
checkFixedBatchChunk,
873950
createBatchEntry,
951+
fixedBatchToEntry,
952+
snapshotFixedBatch,
874953
recordChunk,
875954
splitBatchEntry,
876955
getProtocolMethod,

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

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -210,6 +210,53 @@ async function testPipeRejectsSyncWriteDetachOrResize() {
210210
assert.deepStrictEqual(written, [chunk]);
211211
}
212212

213+
// Batches of several chunks written one at a time are checked against the
214+
// byteLengths recorded when the batch was accepted, before and after each
215+
// writeSync(). Detaching a later chunk, or the chunk being written, must be
216+
// rejected.
217+
async function testPipeRejectsDetachInMultiChunkBatch() {
218+
for (const detachIndex of [1, 0]) {
219+
const asyncBuffers = [new ArrayBuffer(2), new ArrayBuffer(2)];
220+
const asyncWritten = [];
221+
await assert.rejects(pipeTo([asyncBuffers.map((b) => new Uint8Array(b))], {
222+
writeSync(chunk) {
223+
asyncWritten.push(chunk.byteLength);
224+
if (asyncWritten.length === 1) asyncBuffers[detachIndex].transfer();
225+
return true;
226+
},
227+
write: common.mustNotCall(),
228+
fail: common.mustCall(),
229+
}), kResizeError);
230+
assert.deepStrictEqual(asyncWritten, [2]);
231+
232+
const syncBuffers = [new ArrayBuffer(2), new ArrayBuffer(2)];
233+
const syncWritten = [];
234+
assert.throws(() => pipeToSync([syncBuffers.map((b) => new Uint8Array(b))], {
235+
writeSync(chunk) {
236+
syncWritten.push(chunk.byteLength);
237+
if (syncWritten.length === 1) syncBuffers[detachIndex].transfer();
238+
return true;
239+
},
240+
fail: common.mustCall(),
241+
}, { preventClose: true }), kResizeError);
242+
assert.deepStrictEqual(syncWritten, [2]);
243+
}
244+
245+
// When writeSync() declines a chunk, pipeTo() writes the rest of the batch
246+
// with write().
247+
const chunks = [Uint8Array.of(1), Uint8Array.of(2, 3), Uint8Array.of(4)];
248+
const written = [];
249+
assert.strictEqual(await pipeTo([chunks], {
250+
writeSync(chunk) {
251+
if (chunk === chunks[1]) return false;
252+
written.push(chunk);
253+
return true;
254+
},
255+
write(chunk) { written.push(chunk); },
256+
}, { preventClose: true }), 4);
257+
assert.deepStrictEqual(written, chunks);
258+
}
259+
213260
async function testConsumersRejectDetachedViews() {
214261
// Views of fixed-length buffers are tracked without a full snapshot; they
215262
// must still be rejected when detached after being accepted.
@@ -249,4 +296,5 @@ Promise.all([
249296
testConsumersRejectDetachedViews(),
250297
testPipeRejectsWriterResize(),
251298
testPipeRejectsSyncWriteDetachOrResize(),
299+
testPipeRejectsDetachInMultiChunkBatch(),
252300
]).then(common.mustCall());

0 commit comments

Comments
 (0)