Skip to content

Commit

Permalink
util: improve iterator inspect output
Browse files Browse the repository at this point in the history
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: #19259
Reviewed-By: Yosuke Furukawa <yosuke.furukawa@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Tiancheng "Timothy" Gu <timothygu99@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
  • Loading branch information
BridgeAR committed Mar 25, 2018
1 parent a101631 commit 0fbd4b1
Show file tree
Hide file tree
Showing 4 changed files with 45 additions and 28 deletions.
4 changes: 2 additions & 2 deletions lib/internal/bootstrap/node.js
Original file line number Diff line number Diff line change
Expand Up @@ -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$/;
Expand Down
30 changes: 16 additions & 14 deletions lib/internal/v8.js
Original file line number Diff line number Diff line change
@@ -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.

function previewMapIterator(it, n) {
return take(%MapIteratorClone(it), n);
// Clone the provided Map Iterator.
function previewMapIterator(it) {
return %MapIteratorClone(it);
}

function previewSetIterator(it, n) {
return take(%SetIteratorClone(it), n);
// Clone the provided Set Iterator.
function previewSetIterator(it) {
return %SetIteratorClone(it);
}

module.exports = { previewMapIterator, previewSetIterator };
module.exports = {
previewMapIterator,
previewSetIterator
};
29 changes: 18 additions & 11 deletions lib/util.js
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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) {
Expand Down
10 changes: 9 additions & 1 deletion test/parallel/test-util-inspect.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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.
Expand All @@ -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.
Expand Down

0 comments on commit 0fbd4b1

Please sign in to comment.