From ddd4de4b84ed9c0efce8c657e0eb3675a5a6ad88 Mon Sep 17 00:00:00 2001 From: Ruben Bridgewater Date: Tue, 25 Jul 2017 23:21:12 -0300 Subject: [PATCH 1/6] util: use for in instead of while hasOwnProperty This is a huge performance improvement in case of sparse arrays when using util.inspect as the hole will simple be skipped. --- lib/util.js | 33 ++++++++++++++++++++++----------- 1 file changed, 22 insertions(+), 11 deletions(-) diff --git a/lib/util.js b/lib/util.js index 86be8612b97131..eca5b76dbe0c66 100644 --- a/lib/util.js +++ b/lib/util.js @@ -672,22 +672,33 @@ function formatArray(ctx, value, recurseTimes, visibleKeys, keys) { var output = []; let visibleLength = 0; let index = 0; - while (index < value.length && visibleLength < ctx.maxArrayLength) { - let emptyItems = 0; - while (index < value.length && !hasOwnProperty(value, String(index))) { - emptyItems++; - index++; - } - if (emptyItems > 0) { + for (const elem in value) { + if (visibleLength === ctx.maxArrayLength) + break; + const i = +elem; + if (index !== i) { + if (i !== i) // Fast NaN check + continue; + const emptyItems = i - index; const ending = emptyItems > 1 ? 's' : ''; const message = `<${emptyItems} empty item${ending}>`; output.push(ctx.stylize(message, 'undefined')); - } else { - output.push(formatProperty(ctx, value, recurseTimes, visibleKeys, - String(index), true)); - index++; + index = i; + visibleLength++; + if (visibleLength === ctx.maxArrayLength) + break; } + output.push(formatProperty(ctx, value, recurseTimes, visibleKeys, + elem, true)); visibleLength++; + index++; + } + if (index < value.length && visibleLength !== ctx.maxArrayLength) { + const len = value.length - index; + const ending = len > 1 ? 's' : ''; + const message = `<${len} empty item${ending}>`; + output.push(ctx.stylize(message, 'undefined')); + index = value.length; } var remaining = value.length - index; if (remaining > 0) { From b73a399b4add2b8576a0d0cd84dd49b9613eac33 Mon Sep 17 00:00:00 2001 From: Ruben Bridgewater Date: Wed, 26 Jul 2017 00:58:12 -0300 Subject: [PATCH 2/6] util: use faster visibleKeys property lookup --- lib/util.js | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/lib/util.js b/lib/util.js index eca5b76dbe0c66..00042777a595f3 100644 --- a/lib/util.js +++ b/lib/util.js @@ -64,7 +64,6 @@ const inspectDefaultOptions = Object.seal({ const numbersOnlyRE = /^\d+$/; -const objectHasOwnProperty = Object.prototype.hasOwnProperty; const propertyIsEnumerable = Object.prototype.propertyIsEnumerable; const regExpToString = RegExp.prototype.toString; const dateToISOString = Date.prototype.toISOString; @@ -814,7 +813,7 @@ function formatProperty(ctx, value, recurseTimes, visibleKeys, key, array) { str = ctx.stylize('[Setter]', 'special'); } } - if (!hasOwnProperty(visibleKeys, key)) { + if (visibleKeys[key] === undefined) { if (typeof key === 'symbol') { name = `[${ctx.stylize(key.toString(), 'symbol')}]`; } else { @@ -993,11 +992,6 @@ function _extend(target, source) { return target; } -function hasOwnProperty(obj, prop) { - return objectHasOwnProperty.call(obj, prop); -} - - // Deprecated old stuff. function print(...args) { From ab497dfd4b7104c28e632ccd620121d01fa6755d Mon Sep 17 00:00:00 2001 From: Ruben Bridgewater Date: Wed, 26 Jul 2017 11:35:05 -0300 Subject: [PATCH 3/6] fixup --- lib/util.js | 2 +- test/parallel/test-util-inspect.js | 10 +++++++++- 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/lib/util.js b/lib/util.js index 00042777a595f3..de73f480dcb6e4 100644 --- a/lib/util.js +++ b/lib/util.js @@ -676,7 +676,7 @@ function formatArray(ctx, value, recurseTimes, visibleKeys, keys) { break; const i = +elem; if (index !== i) { - if (i !== i) // Fast NaN check + if (i > 0 === false) continue; const emptyItems = i - index; const ending = emptyItems > 1 ? 's' : ''; diff --git a/test/parallel/test-util-inspect.js b/test/parallel/test-util-inspect.js index 7fc5fb4eac0495..d8b24d98318421 100644 --- a/test/parallel/test-util-inspect.js +++ b/test/parallel/test-util-inspect.js @@ -298,8 +298,16 @@ assert.strictEqual( get: function() { this.push(true); return this.length; } } ); + Object.defineProperty( + value, + '-1', + { + enumerable: true, + value: -1 + } + ); assert.strictEqual(util.inspect(value), - '[ 1, 2, 3, growingLength: [Getter] ]'); + '[ 1, 2, 3, growingLength: [Getter], \'-1\': -1 ]'); } // Function with properties From 61119e4ae46f93394e3cb2b8569aeb441530c58f Mon Sep 17 00:00:00 2001 From: Ruben Bridgewater Date: Thu, 27 Jul 2017 00:28:23 -0300 Subject: [PATCH 4/6] fixup comments commit 1 --- lib/util.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/util.js b/lib/util.js index de73f480dcb6e4..0ab18543e8bab7 100644 --- a/lib/util.js +++ b/lib/util.js @@ -676,6 +676,7 @@ function formatArray(ctx, value, recurseTimes, visibleKeys, keys) { break; const i = +elem; if (index !== i) { + // Skip zero and negative numbers as well as non numbers if (i > 0 === false) continue; const emptyItems = i - index; @@ -683,8 +684,7 @@ function formatArray(ctx, value, recurseTimes, visibleKeys, keys) { const message = `<${emptyItems} empty item${ending}>`; output.push(ctx.stylize(message, 'undefined')); index = i; - visibleLength++; - if (visibleLength === ctx.maxArrayLength) + if (++visibleLength === ctx.maxArrayLength) break; } output.push(formatProperty(ctx, value, recurseTimes, visibleKeys, From a2574f0bc5571997134532723bc2ee43bf349c0f Mon Sep 17 00:00:00 2001 From: Ruben Bridgewater Date: Fri, 28 Jul 2017 10:08:26 -0300 Subject: [PATCH 5/6] fixup commit 1 --- lib/util.js | 5 ++++- test/parallel/test-util-inspect.js | 8 ++++++++ 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/lib/util.js b/lib/util.js index 0ab18543e8bab7..30984f1d591f33 100644 --- a/lib/util.js +++ b/lib/util.js @@ -671,9 +671,12 @@ function formatArray(ctx, value, recurseTimes, visibleKeys, keys) { var output = []; let visibleLength = 0; let index = 0; - for (const elem in value) { + for (const elem of keys) { if (visibleLength === ctx.maxArrayLength) break; + // Symbols might have been added to the keys + if (typeof elem !== 'string') + continue; const i = +elem; if (index !== i) { // Skip zero and negative numbers as well as non numbers diff --git a/test/parallel/test-util-inspect.js b/test/parallel/test-util-inspect.js index d8b24d98318421..28771bfab1ed0f 100644 --- a/test/parallel/test-util-inspect.js +++ b/test/parallel/test-util-inspect.js @@ -310,6 +310,14 @@ assert.strictEqual( '[ 1, 2, 3, growingLength: [Getter], \'-1\': -1 ]'); } +// Array with inherited number properties +{ + class CustomArray extends Array {}; + CustomArray.prototype[5] = 'foo'; + const arr = new CustomArray(50); + assert.strictEqual(util.inspect(arr), 'CustomArray [ <50 empty items> ]'); +} + // Function with properties { const value = () => {}; From 4aed280ee9a27df1eff5ae874715f635407e66ad Mon Sep 17 00:00:00 2001 From: Ruben Bridgewater Date: Fri, 28 Jul 2017 10:09:09 -0300 Subject: [PATCH 6/6] benchmarks: add inspect-array benchmark --- benchmark/util/inspect-array.js | 39 +++++++++++++++++++++++++++++++++ 1 file changed, 39 insertions(+) create mode 100644 benchmark/util/inspect-array.js diff --git a/benchmark/util/inspect-array.js b/benchmark/util/inspect-array.js new file mode 100644 index 00000000000000..44067b8b8f81da --- /dev/null +++ b/benchmark/util/inspect-array.js @@ -0,0 +1,39 @@ +'use strict'; + +const common = require('../common'); +const util = require('util'); + +const bench = common.createBenchmark(main, { + n: [1e2], + len: [1e5], + type: [ + 'denseArray', + 'sparseArray', + 'mixedArray' + ] +}); + +function main(conf) { + const { n, len, type } = conf; + var arr = Array(len); + var i; + + switch (type) { + case 'denseArray': + arr = arr.fill(0); + break; + case 'sparseArray': + break; + case 'mixedArray': + for (i = 0; i < n; i += 2) + arr[i] = i; + break; + default: + throw new Error(`Unsupported type ${type}`); + } + bench.start(); + for (i = 0; i < n; i++) { + util.inspect(arr); + } + bench.end(n); +}