Skip to content

Commit

Permalink
stream: resolve perf regression introduced by V8 7.3
Browse files Browse the repository at this point in the history
This commit contains two fixes:
1. use instanceof instead of Object.getPrototypeOf, as checking an
   object prototype with Object.getPrototypeOf is slower
   than an instanceof check.
2. avoid parseInt(undefined, 10) to get NaN as it regressed.

PR-URL: #28842
Fixes: #28586
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Anto Aravinth <anto.aravinth.cse@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
  • Loading branch information
mcollina authored and Trott committed Jul 27, 2019
1 parent db1c4a7 commit 147b9d9
Show file tree
Hide file tree
Showing 2 changed files with 11 additions and 3 deletions.
11 changes: 9 additions & 2 deletions lib/_stream_readable.js
Original file line number Diff line number Diff line change
Expand Up @@ -256,7 +256,8 @@ function readableAddChunk(stream, chunk, encoding, addToFront) {
} else if (state.objectMode || chunk && chunk.length > 0) {
if (typeof chunk !== 'string' &&
!state.objectMode &&
Object.getPrototypeOf(chunk) !== Buffer.prototype) {
// Do not use Object.getPrototypeOf as it is slower since V8 7.3.
!(chunk instanceof Buffer)) {
chunk = Stream._uint8ArrayToBuffer(chunk);
}

Expand Down Expand Up @@ -399,7 +400,13 @@ function howMuchToRead(n, state) {
// You can override either this method, or the async _read(n) below.
Readable.prototype.read = function(n) {
debug('read', n);
n = parseInt(n, 10);
// Same as parseInt(undefined, 10), however V8 7.3 performance regressed
// in this scenario, so we are doing it manually.
if (n === undefined) {
n = NaN;
} else if (!Number.isInteger(n)) {
n = parseInt(n, 10);
}
const state = this._readableState;
const nOrig = n;

Expand Down
3 changes: 2 additions & 1 deletion lib/_stream_writable.js
Original file line number Diff line number Diff line change
Expand Up @@ -277,7 +277,8 @@ Writable.prototype.write = function(chunk, encoding, cb) {
var ret = false;
const isBuf = !state.objectMode && Stream._isUint8Array(chunk);

if (isBuf && Object.getPrototypeOf(chunk) !== Buffer.prototype) {
// Do not use Object.getPrototypeOf as it is slower since V8 7.3.
if (isBuf && !(chunk instanceof Buffer)) {
chunk = Stream._uint8ArrayToBuffer(chunk);
}

Expand Down

0 comments on commit 147b9d9

Please sign in to comment.