From 8a21c2e66815e804944ce17cb5071b78b066c021 Mon Sep 17 00:00:00 2001 From: Raz Luvaton <16746759+rluvaton@users.noreply.github.com> Date: Fri, 15 Sep 2023 14:32:31 +0300 Subject: [PATCH 1/5] stream: improve webstream readable async iterator performance --- lib/internal/webstreams/readablestream.js | 75 ++++++++++++++--------- 1 file changed, 46 insertions(+), 29 deletions(-) diff --git a/lib/internal/webstreams/readablestream.js b/lib/internal/webstreams/readablestream.js index 28e985875c5ddb..b47c810232c632 100644 --- a/lib/internal/webstreams/readablestream.js +++ b/lib/internal/webstreams/readablestream.js @@ -241,6 +241,33 @@ const getNonWritablePropertyDescriptor = (value) => { * }} UnderlyingSource */ +class ReadRequestData { + constructor(reader, state, promise) { + this.reader = reader; + this.state = state; + this.promise = promise; + } + + [kChunk](chunk) { + this.state.current = undefined; + this.promise.resolve({value: chunk, done: false}); + } + + [kClose]() { + this.state.current = undefined; + this.state.done = true; + readableStreamReaderGenericRelease(this.reader); + this.promise.resolve({ done: true, value: undefined }); + } + + [kError](error) { + this.state.current = undefined; + this.state.done = true; + readableStreamReaderGenericRelease(this.reader); + this.promise.reject(error); + } +} + class ReadableStream { [kType] = 'ReadableStream'; @@ -477,9 +504,14 @@ class ReadableStream { // eslint-disable-next-line no-use-before-define const reader = new ReadableStreamDefaultReader(this); - let done = false; + + const state = { + done: false, + current: undefined, + }; + // let done = false; let started = false; - let current; + // let current; // The nextSteps function is not an async function in order // to make it more efficient. Because nextSteps explicitly @@ -488,7 +520,7 @@ class ReadableStream { // unnecessary Promise allocations to occur, which just add // cost. function nextSteps() { - if (done) + if (state.done) return PromiseResolve({ done: true, value: undefined }); if (reader[kState].stream === undefined) { @@ -498,31 +530,16 @@ class ReadableStream { } const promise = createDeferredPromise(); - readableStreamDefaultReaderRead(reader, { - [kChunk](chunk) { - current = undefined; - promise.resolve({ value: chunk, done: false }); - }, - [kClose]() { - current = undefined; - done = true; - readableStreamReaderGenericRelease(reader); - promise.resolve({ done: true, value: undefined }); - }, - [kError](error) { - current = undefined; - done = true; - readableStreamReaderGenericRelease(reader); - promise.reject(error); - }, - }); + const readRequest = new ReadRequestData(reader, state, promise); + + readableStreamDefaultReaderRead(reader, readRequest); return promise.promise; } async function returnSteps(value) { - if (done) + if (state.done) return { done: true, value }; // eslint-disable-line node-core/avoid-prototype-pollution - done = true; + state.done = true; if (reader[kState].stream === undefined) { throw new ERR_INVALID_STATE.TypeError( @@ -559,19 +576,19 @@ class ReadableStream { // need to investigate if it's a bug in our impl or // the spec. if (!started) { - current = PromiseResolve(); + state.current = PromiseResolve(); started = true; } - current = current !== undefined ? - PromisePrototypeThen(current, nextSteps, nextSteps) : + state.current = state.current !== undefined ? + PromisePrototypeThen(state.current, nextSteps, nextSteps) : nextSteps(); - return current; + return state.current; }, return(error) { - return current ? + return state.current ? PromisePrototypeThen( - current, + state.current, () => returnSteps(error), () => returnSteps(error)) : returnSteps(error); From 2c62bf6e593e4132bf90aacafa2e3d8a516e3dae Mon Sep 17 00:00:00 2001 From: Raz Luvaton <16746759+rluvaton@users.noreply.github.com> Date: Fri, 15 Sep 2023 14:39:30 +0300 Subject: [PATCH 2/5] stream: rename, fix lint issues and cleanup --- lib/internal/webstreams/readablestream.js | 65 +++++++++++------------ 1 file changed, 32 insertions(+), 33 deletions(-) diff --git a/lib/internal/webstreams/readablestream.js b/lib/internal/webstreams/readablestream.js index b47c810232c632..551e053d90a287 100644 --- a/lib/internal/webstreams/readablestream.js +++ b/lib/internal/webstreams/readablestream.js @@ -241,33 +241,6 @@ const getNonWritablePropertyDescriptor = (value) => { * }} UnderlyingSource */ -class ReadRequestData { - constructor(reader, state, promise) { - this.reader = reader; - this.state = state; - this.promise = promise; - } - - [kChunk](chunk) { - this.state.current = undefined; - this.promise.resolve({value: chunk, done: false}); - } - - [kClose]() { - this.state.current = undefined; - this.state.done = true; - readableStreamReaderGenericRelease(this.reader); - this.promise.resolve({ done: true, value: undefined }); - } - - [kError](error) { - this.state.current = undefined; - this.state.done = true; - readableStreamReaderGenericRelease(this.reader); - this.promise.reject(error); - } -} - class ReadableStream { [kType] = 'ReadableStream'; @@ -506,12 +479,10 @@ class ReadableStream { const reader = new ReadableStreamDefaultReader(this); const state = { - done: false, - current: undefined, + done: false, + current: undefined, }; - // let done = false; let started = false; - // let current; // The nextSteps function is not an async function in order // to make it more efficient. Because nextSteps explicitly @@ -530,7 +501,8 @@ class ReadableStream { } const promise = createDeferredPromise(); - const readRequest = new ReadRequestData(reader, state, promise); + // eslint-disable-next-line no-use-before-define + const readRequest = new ReadableStreamAsyncIteratorReadRequest(reader, state, promise); readableStreamDefaultReaderRead(reader, readRequest); return promise.promise; @@ -588,7 +560,7 @@ class ReadableStream { return(error) { return state.current ? PromisePrototypeThen( - state.current, + state.current, () => returnSteps(error), () => returnSteps(error)) : returnSteps(error); @@ -790,6 +762,33 @@ function createReadableStreamBYOBRequest(controller, view) { return stream; } +class ReadableStreamAsyncIteratorReadRequest { + constructor(reader, state, promise) { + this.reader = reader; + this.state = state; + this.promise = promise; + } + + [kChunk](chunk) { + this.state.current = undefined; + this.promise.resolve({ value: chunk, done: false }); + } + + [kClose]() { + this.state.current = undefined; + this.state.done = true; + readableStreamReaderGenericRelease(this.reader); + this.promise.resolve({ done: true, value: undefined }); + } + + [kError](error) { + this.state.current = undefined; + this.state.done = true; + readableStreamReaderGenericRelease(this.reader); + this.promise.reject(error); + } +} + class DefaultReadRequest { constructor() { this[kState] = createDeferredPromise(); From 066330b2d63122d7663084a6f9a7f148d09115db Mon Sep 17 00:00:00 2001 From: Raz Luvaton <16746759+rluvaton@users.noreply.github.com> Date: Sun, 17 Sep 2023 19:38:11 +0300 Subject: [PATCH 3/5] stream: add proto Co-authored-by: Benjamin Gruenbaum --- lib/internal/webstreams/readablestream.js | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/internal/webstreams/readablestream.js b/lib/internal/webstreams/readablestream.js index 551e053d90a287..cbc0e911c07a32 100644 --- a/lib/internal/webstreams/readablestream.js +++ b/lib/internal/webstreams/readablestream.js @@ -481,6 +481,7 @@ class ReadableStream { const state = { done: false, current: undefined, + __proto__: null, }; let started = false; From 7f423eb47735159c7fd94b53490182ca9b9e6f08 Mon Sep 17 00:00:00 2001 From: Raz Luvaton <16746759+rluvaton@users.noreply.github.com> Date: Sun, 17 Sep 2023 22:58:17 +0300 Subject: [PATCH 4/5] stream: inline var --- lib/internal/webstreams/readablestream.js | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/lib/internal/webstreams/readablestream.js b/lib/internal/webstreams/readablestream.js index cbc0e911c07a32..1f37a4d1253251 100644 --- a/lib/internal/webstreams/readablestream.js +++ b/lib/internal/webstreams/readablestream.js @@ -503,9 +503,7 @@ class ReadableStream { const promise = createDeferredPromise(); // eslint-disable-next-line no-use-before-define - const readRequest = new ReadableStreamAsyncIteratorReadRequest(reader, state, promise); - - readableStreamDefaultReaderRead(reader, readRequest); + readableStreamDefaultReaderRead(reader, new ReadableStreamAsyncIteratorReadRequest(reader, state, promise)); return promise.promise; } From 824abd622d65d1902986399512bf8c59d38e31fc Mon Sep 17 00:00:00 2001 From: Raz Luvaton <16746759+rluvaton@users.noreply.github.com> Date: Mon, 18 Sep 2023 10:20:13 +0300 Subject: [PATCH 5/5] stream: remove `__proto__: null` due to performance hit from 140% improvement to 120% --- lib/internal/webstreams/readablestream.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/internal/webstreams/readablestream.js b/lib/internal/webstreams/readablestream.js index 1f37a4d1253251..a2727d94b6da4b 100644 --- a/lib/internal/webstreams/readablestream.js +++ b/lib/internal/webstreams/readablestream.js @@ -478,10 +478,10 @@ class ReadableStream { // eslint-disable-next-line no-use-before-define const reader = new ReadableStreamDefaultReader(this); + // No __proto__ here to avoid the performance hit. const state = { done: false, current: undefined, - __proto__: null, }; let started = false;