From 0fbd4b1d021ed5fcd95210047a9e1d2addefe51a Mon Sep 17 00:00:00 2001 From: Ruben Bridgewater Date: Fri, 9 Mar 2018 14:57:19 +0100 Subject: [PATCH] util: improve iterator inspect output 1) So far extra keys on an (Set|Map)Iterator were ignored. Those will now be visible. 2) Improve the performance of showing (Set|Map)Iterator by using the cloned iterator instead of copying all entries first. 3) So far the output was strictly limited to up to 100 entries. The limit will now depend on `maxArrayLength` instead (that default is set to 100 as well) and the output indicates that more entries exist than visible. PR-URL: https://github.com/nodejs/node/pull/19259 Reviewed-By: Yosuke Furukawa Reviewed-By: Matteo Collina Reviewed-By: Tiancheng "Timothy" Gu Reviewed-By: James M Snell --- lib/internal/bootstrap/node.js | 4 ++-- lib/internal/v8.js | 32 ++++++++++++++++-------------- lib/util.js | 29 +++++++++++++++++---------- test/parallel/test-util-inspect.js | 10 +++++++++- 4 files changed, 46 insertions(+), 29 deletions(-) diff --git a/lib/internal/bootstrap/node.js b/lib/internal/bootstrap/node.js index 7d7ca03d36f..9055e8c4198 100644 --- a/lib/internal/bootstrap/node.js +++ b/lib/internal/bootstrap/node.js @@ -471,8 +471,8 @@ // functions lazily (unless --nolazy is set) so we need to do this // before we turn off --allow_natives_syntax again. const v8 = NativeModule.require('internal/v8'); - v8.previewMapIterator(new Map().entries(), 1); - v8.previewSetIterator(new Set().entries(), 1); + v8.previewMapIterator(new Map().entries()); + v8.previewSetIterator(new Set().entries()); // Disable --allow_natives_syntax again unless it was explicitly // specified on the command line. const re = /^--allow[-_]natives[-_]syntax$/; diff --git a/lib/internal/v8.js b/lib/internal/v8.js index b80e3de33d6..92d5beb7c4c 100644 --- a/lib/internal/v8.js +++ b/lib/internal/v8.js @@ -1,21 +1,23 @@ 'use strict'; -function take(it, n) { - const result = []; - for (const e of it) { - if (--n < 0) - break; - result.push(e); - } - return result; +// This file provides access to some of V8's native runtime functions. See +// https://github.com/v8/v8/wiki/Built-in-functions for further information +// about their implementation. +// They have to be loaded before anything else to make sure we deactivate them +// before executing any other code. Gaining access is achieved by using a +// specific flag that is used internally in the startup phase. + +// Clone the provided Map Iterator. +function previewMapIterator(it) { + return %MapIteratorClone(it); } -function previewMapIterator(it, n) { - return take(%MapIteratorClone(it), n); +// Clone the provided Set Iterator. +function previewSetIterator(it) { + return %SetIteratorClone(it); } -function previewSetIterator(it, n) { - return take(%SetIteratorClone(it), n); -} - -module.exports = { previewMapIterator, previewSetIterator }; +module.exports = { + previewMapIterator, + previewSetIterator +}; diff --git a/lib/util.js b/lib/util.js index 2cf79bb5f08..774b185095c 100644 --- a/lib/util.js +++ b/lib/util.js @@ -30,7 +30,10 @@ const { const { TextDecoder, TextEncoder } = require('internal/encoding'); const { isBuffer } = require('buffer').Buffer; -const { previewMapIterator, previewSetIterator } = require('internal/v8'); +const { + previewMapIterator, + previewSetIterator +} = require('internal/v8'); const { getPromiseDetails, @@ -836,25 +839,29 @@ function formatMap(ctx, value, recurseTimes, keys) { return output; } -function formatCollectionIterator(preview, ctx, value, recurseTimes, - visibleKeys, keys) { - const nextRecurseTimes = recurseTimes === null ? null : recurseTimes - 1; - const vals = preview(value, 100); +function formatCollectionIterator(preview, ctx, value, recurseTimes, keys) { const output = []; - for (const o of vals) { - output.push(formatValue(ctx, o, nextRecurseTimes)); + for (const entry of preview(value)) { + if (ctx.maxArrayLength === output.length) { + output.push('... more items'); + break; + } + output.push(formatValue(ctx, entry, recurseTimes)); + } + for (var n = 0; n < keys.length; n++) { + output.push(formatProperty(ctx, value, recurseTimes, keys[n], 0)); } return output; } -function formatMapIterator(ctx, value, recurseTimes, visibleKeys, keys) { +function formatMapIterator(ctx, value, recurseTimes, keys) { return formatCollectionIterator(previewMapIterator, ctx, value, recurseTimes, - visibleKeys, keys); + keys); } -function formatSetIterator(ctx, value, recurseTimes, visibleKeys, keys) { +function formatSetIterator(ctx, value, recurseTimes, keys) { return formatCollectionIterator(previewSetIterator, ctx, value, recurseTimes, - visibleKeys, keys); + keys); } function formatPromise(ctx, value, recurseTimes, keys) { diff --git a/test/parallel/test-util-inspect.js b/test/parallel/test-util-inspect.js index 0dd1ad74c00..ab27e69c183 100644 --- a/test/parallel/test-util-inspect.js +++ b/test/parallel/test-util-inspect.js @@ -448,7 +448,7 @@ assert.strictEqual(util.inspect(-5e-324), '-5e-324'); { const map = new Map(); map.set(1, 2); - const vals = previewMapIterator(map.entries(), 100); + const vals = previewMapIterator(map.entries()); const valsOutput = []; for (const o of vals) { valsOutput.push(o); @@ -924,6 +924,10 @@ if (typeof Symbol !== 'undefined') { const keys = map.keys(); assert.strictEqual(util.inspect(keys), '[Map Iterator] { \'foo\' }'); assert.strictEqual(util.inspect(keys), '[Map Iterator] { \'foo\' }'); + keys.extra = true; + assert.strictEqual( + util.inspect(keys, { maxArrayLength: 0 }), + '[Map Iterator] { ... more items, extra: true }'); } // Test Set iterators. @@ -937,6 +941,10 @@ if (typeof Symbol !== 'undefined') { const keys = aSet.keys(); assert.strictEqual(util.inspect(keys), '[Set Iterator] { 1, 3 }'); assert.strictEqual(util.inspect(keys), '[Set Iterator] { 1, 3 }'); + keys.extra = true; + assert.strictEqual( + util.inspect(keys, { maxArrayLength: 1 }), + '[Set Iterator] { 1, ... more items, extra: true }'); } // Test alignment of items in container.