From 38d9b7fd2697492738c3a39d5d3d6fcbb6283706 Mon Sep 17 00:00:00 2001 From: Robert Nagy Date: Fri, 17 Apr 2020 08:56:25 +0200 Subject: [PATCH 1/4] stream: removed unecessary writing property writecb casted to boolean has the same value as writing. --- lib/_stream_writable.js | 23 +++++++++++++---------- 1 file changed, 13 insertions(+), 10 deletions(-) diff --git a/lib/_stream_writable.js b/lib/_stream_writable.js index 25235166d9fa7d..7ae67bd8b1d1e1 100644 --- a/lib/_stream_writable.js +++ b/lib/_stream_writable.js @@ -120,9 +120,6 @@ function WritableState(options, stream, isDuplex) { // socket or file. this.length = 0; - // A flag to see when we're in the middle of a write. - this.writing = false; - // When true all writes will be buffered until .uncork() call this.corked = 0; @@ -188,6 +185,14 @@ function WritableState(options, stream, isDuplex) { this.corkedRequestsFree = corkReq; } +ObjectDefineProperties(WritableState.prototype, { + writing: { + get() { + return !!this.writecb; + } + } +}); + WritableState.prototype.getBuffer = function getBuffer() { let current = this.bufferedRequest; const out = []; @@ -318,7 +323,7 @@ Writable.prototype.uncork = function() { if (state.corked) { state.corked--; - if (!state.writing && + if (!state.writecb && !state.corked && !state.bufferProcessing && state.bufferedRequest) @@ -349,7 +354,7 @@ function writeOrBuffer(stream, state, chunk, encoding, cb) { if (!ret) state.needDrain = true; - if (state.writing || state.corked || state.errored) { + if (state.writecb || state.corked || state.errored) { const last = state.lastBufferedRequest; state.lastBufferedRequest = { chunk, @@ -375,7 +380,6 @@ function writeOrBuffer(stream, state, chunk, encoding, cb) { function doWrite(stream, state, writev, len, chunk, encoding, cb) { state.writelen = len; state.writecb = cb; - state.writing = true; state.sync = true; if (state.destroyed) state.onwrite(new ERR_STREAM_DESTROYED('write')); @@ -409,7 +413,6 @@ function onwrite(stream, er) { return; } - state.writing = false; state.writecb = null; state.length -= state.writelen; state.writelen = 0; @@ -484,7 +487,7 @@ function afterWrite(stream, state, count, cb) { // If there's something in the buffer waiting, then invoke callbacks. function errorBuffer(state, err) { - if (state.writing || !state.bufferedRequest) { + if (state.writecb || !state.bufferedRequest) { return; } @@ -551,7 +554,7 @@ function clearBuffer(stream, state) { // it means that we need to wait until it does. // also, that means that the chunk and cb are currently // being processed, so move the buffer counter past them. - if (state.writing) { + if (state.writecb) { break; } } @@ -621,7 +624,7 @@ function needFinish(state) { !state.errored && state.bufferedRequest === null && !state.finished && - !state.writing); + !state.writecb); } function callFinal(stream, state) { From 4ac87beb6d0f8b6693fc8792229f814f64ef92bd Mon Sep 17 00:00:00 2001 From: Robert Nagy Date: Fri, 17 Apr 2020 09:11:24 +0200 Subject: [PATCH 2/4] stream: simplify clearBuffer condition Was doing some unecessary checks. --- lib/_stream_writable.js | 22 ++++++++++------------ 1 file changed, 10 insertions(+), 12 deletions(-) diff --git a/lib/_stream_writable.js b/lib/_stream_writable.js index 7ae67bd8b1d1e1..dde0abee2d19f1 100644 --- a/lib/_stream_writable.js +++ b/lib/_stream_writable.js @@ -186,6 +186,7 @@ function WritableState(options, stream, isDuplex) { } ObjectDefineProperties(WritableState.prototype, { + // Backwards compat. writing: { get() { return !!this.writecb; @@ -323,11 +324,7 @@ Writable.prototype.uncork = function() { if (state.corked) { state.corked--; - if (!state.writecb && - !state.corked && - !state.bufferProcessing && - state.bufferedRequest) - clearBuffer(this, state); + clearBuffer(this, state); } }; @@ -432,13 +429,7 @@ function onwrite(stream, er) { onwriteError(stream, state, er, cb); } } else { - // Check if we're actually ready to finish, but don't emit yet - const finished = needFinish(state) || stream.destroyed; - - if (!finished && - !state.corked && - !state.bufferProcessing && - state.bufferedRequest) { + if (state.bufferedRequest) { clearBuffer(stream, state); } @@ -503,6 +494,13 @@ function errorBuffer(state, err) { // If there's something in the buffer waiting, then process it function clearBuffer(stream, state) { + if (state.writecb || + state.corked || + state.bufferProcessing || + !state.bufferedRequest) { + return; + } + state.bufferProcessing = true; let entry = state.bufferedRequest; From 206f62b64986f1987b8fcdba945e7f616c26b43d Mon Sep 17 00:00:00 2001 From: Robert Nagy Date: Fri, 17 Apr 2020 18:31:52 +0200 Subject: [PATCH 3/4] fixup: try perf --- lib/_stream_writable.js | 9 --------- 1 file changed, 9 deletions(-) diff --git a/lib/_stream_writable.js b/lib/_stream_writable.js index dde0abee2d19f1..66b1f9c3151379 100644 --- a/lib/_stream_writable.js +++ b/lib/_stream_writable.js @@ -185,15 +185,6 @@ function WritableState(options, stream, isDuplex) { this.corkedRequestsFree = corkReq; } -ObjectDefineProperties(WritableState.prototype, { - // Backwards compat. - writing: { - get() { - return !!this.writecb; - } - } -}); - WritableState.prototype.getBuffer = function getBuffer() { let current = this.bufferedRequest; const out = []; From 4331da7d0b0b2724f9c2b23ee1ad8ebb351657d4 Mon Sep 17 00:00:00 2001 From: Robert Nagy Date: Fri, 17 Apr 2020 19:47:41 +0200 Subject: [PATCH 4/4] Revert "fixup: try perf" This reverts commit 206f62b64986f1987b8fcdba945e7f616c26b43d. --- lib/_stream_writable.js | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/lib/_stream_writable.js b/lib/_stream_writable.js index 66b1f9c3151379..dde0abee2d19f1 100644 --- a/lib/_stream_writable.js +++ b/lib/_stream_writable.js @@ -185,6 +185,15 @@ function WritableState(options, stream, isDuplex) { this.corkedRequestsFree = corkReq; } +ObjectDefineProperties(WritableState.prototype, { + // Backwards compat. + writing: { + get() { + return !!this.writecb; + } + } +}); + WritableState.prototype.getBuffer = function getBuffer() { let current = this.bufferedRequest; const out = [];