From 1a3f4a87b68eb5a906564b05f3774ea768fc510c Mon Sep 17 00:00:00 2001 From: Ben Noordhuis Date: Thu, 20 Aug 2015 22:19:46 +0200 Subject: [PATCH 1/3] stream: fix off-by-factor-16 error in comment The high watermark is capped at 8 MB, not 128 MB like the comment in lib/_stream_readable.js said. PR-URL: https://github.com/nodejs/node/pull/2479 Reviewed-By: Chris Dickinson Reviewed-By: Sakthipriyan Vairamani --- lib/_stream_readable.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/_stream_readable.js b/lib/_stream_readable.js index 11069985a4d40a..644c4ffa67b04f 100644 --- a/lib/_stream_readable.js +++ b/lib/_stream_readable.js @@ -190,7 +190,7 @@ Readable.prototype.setEncoding = function(enc) { return this; }; -// Don't raise the hwm > 128MB +// Don't raise the hwm > 8MB const MAX_HWM = 0x800000; function roundUpToNextPowerOf2(n) { if (n >= MAX_HWM) { From 3af8e451f272f1398acf7336f9eff0c64340ffe3 Mon Sep 17 00:00:00 2001 From: Ben Noordhuis Date: Thu, 20 Aug 2015 22:29:57 +0200 Subject: [PATCH 2/3] stream: micro-optimize high water mark calculation Don't iterate over all 32 bits, use some hacker's delight bit twiddling to compute the next power of two. The logic can be reduced to `n = 1 << 32 - Math.clz32(n)` but then it can't easily be backported to v2.x; Math.clz32() was added in V8 4.3. PR-URL: https://github.com/nodejs/node/pull/2479 Reviewed-By: Chris Dickinson Reviewed-By: Sakthipriyan Vairamani --- lib/_stream_readable.js | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/lib/_stream_readable.js b/lib/_stream_readable.js index 644c4ffa67b04f..766577af580ca4 100644 --- a/lib/_stream_readable.js +++ b/lib/_stream_readable.js @@ -198,7 +198,11 @@ function roundUpToNextPowerOf2(n) { } else { // Get the next highest power of 2 n--; - for (var p = 1; p < 32; p <<= 1) n |= n >> p; + n |= n >>> 1; + n |= n >>> 2; + n |= n >>> 4; + n |= n >>> 8; + n |= n >>> 16; n++; } return n; From caa0d0c0a761cdcc516cd9b7a214e5f05526bb9e Mon Sep 17 00:00:00 2001 From: Ben Noordhuis Date: Thu, 20 Aug 2015 22:33:12 +0200 Subject: [PATCH 3/3] stream: rename poorly named function roundUpToNextPowerOf2() does more than just rounding up to the next power of two. Rename it to computeNewHighWaterMark(). PR-URL: https://github.com/nodejs/node/pull/2479 Reviewed-By: Chris Dickinson Reviewed-By: Sakthipriyan Vairamani --- lib/_stream_readable.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/_stream_readable.js b/lib/_stream_readable.js index 766577af580ca4..681c3eaec245be 100644 --- a/lib/_stream_readable.js +++ b/lib/_stream_readable.js @@ -192,7 +192,7 @@ Readable.prototype.setEncoding = function(enc) { // Don't raise the hwm > 8MB const MAX_HWM = 0x800000; -function roundUpToNextPowerOf2(n) { +function computeNewHighWaterMark(n) { if (n >= MAX_HWM) { n = MAX_HWM; } else { @@ -231,7 +231,7 @@ function howMuchToRead(n, state) { // power of 2, to prevent increasing it excessively in tiny // amounts. if (n > state.highWaterMark) - state.highWaterMark = roundUpToNextPowerOf2(n); + state.highWaterMark = computeNewHighWaterMark(n); // don't have that much. return null, unless we've ended. if (n > state.length) {