util: improve util.inspect performance

* improve util.inspect performance This is a huge performance improvement in case of sparse arrays when using util.inspect as the hole will simple be skipped. * use faster visibleKeys property lookup * add inspect-array benchmark PR-URL: #14492 Fixes: #14487 Reviewed-By: Alexey Orlenko <[email protected]> Reviewed-By: Refael Ackermann <[email protected]>
nodejs · Aug 13, 2017 · 1168410 · 1168410
1 parent 1c00875
commit 1168410
Show file tree

Hide file tree

Showing 3 changed files with 82 additions and 19 deletions.
diff --git a/benchmark/util/inspect-array.js 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);
+}
diff --git 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;
@@ -683,22 +682,36 @@ 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 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
+      if (i > 0 === false)
+        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;
+      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) {
@@ -814,7 +827,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 {
@@ -996,11 +1009,6 @@ function _extend(target, source) {
   return target;
 }
 
-function hasOwnProperty(obj, prop) {
-  return objectHasOwnProperty.call(obj, prop);
-}
-
-
 // Deprecated old stuff.
 
 function print(...args) {

diff --git a/test/parallel/test-util-inspect.js b/test/parallel/test-util-inspect.js
@@ -298,8 +298,24 @@ 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 ]');
+}
+
+// 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