Skip to content

util: improve Weak(Map|Set) support #19259

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 32 additions & 5 deletions doc/api/util.md
Original file line number Diff line number Diff line change
Expand Up @@ -345,6 +345,9 @@ stream.write('With ES6');
<!-- YAML
added: v0.3.0
changes:
- version: REPLACEME
pr-url: https://github.com/nodejs/node/pull/REPLACEME
description: WeakMap and WeakSet entries can now be inspected as well.
- version: REPLACEME
pr-url: https://github.com/nodejs/node/pull/17907
description: The `depth` default changed to Infinity.
Expand All @@ -369,7 +372,8 @@ changes:
* `object` {any} Any JavaScript primitive or Object.
* `options` {Object}
* `showHidden` {boolean} If `true`, the `object`'s non-enumerable symbols and
properties will be included in the formatted result. Defaults to `false`.
properties will be included in the formatted result as well as [`WeakMap`][]
and [`WeakSet`][] entries. Defaults to `false`.
* `colors` {boolean} If `true`, the output will be styled with ANSI color
codes. Defaults to `false`. Colors are customizable, see
[Customizing `util.inspect` colors][].
Expand All @@ -378,10 +382,14 @@ changes:
* `showProxy` {boolean} If `true`, then objects and functions that are
`Proxy` objects will be introspected to show their `target` and `handler`
objects. Defaults to `false`.
* `maxArrayLength` {number} Specifies the maximum number of array and
`TypedArray` elements to include when formatting. Defaults to `100`. Set to
`null` or `Infinity` to show all array elements. Set to `0` or negative to
show no array elements.
<!--
TODO(BridgeAR): Deprecate `maxArrayLength` and replace it with
`maxEntries`.
-->
* `maxArrayLength` {number} Specifies the maximum number of `Array`,
[`TypedArray`][], [`WeakMap`][] and [`WeakSet`][] elements to include when
formatting. Defaults to `100`. Set to `null` or `Infinity` to show all
elements. Set to `0` or negative to show no elements.
* `breakLength` {number} The length at which an object's keys are split
across multiple lines. Set to `Infinity` to format an object as a single
line. Defaults to 60 for legacy compatibility.
Expand Down Expand Up @@ -501,6 +509,25 @@ console.log(util.inspect(o, { compact: false, breakLength: 80 }));
// chunks.
```

Using the `showHidden` option allows to inspect [`WeakMap`][] and [`WeakSet`][]
entries. If there are more entries than `maxArrayLength`, there is no guarantee
which entries are displayed. That means retrieving the same ['WeakSet'][]
entries twice might actually result in a different output. Besides this any item
might be collected at any point of time by the garbage collector if there is no
strong reference left to that object. Therefore there is no guarantee to get a
reliable output.

```js
const { inspect } = require('util');

const obj = { a: 1 };
const obj2 = { b: 2 };
const weakSet = new WeakSet([obj, obj2]);

console.log(inspect(weakSet, { showHidden: true }));
// WeakSet { { a: 1 }, { b: 2 } }
```

Please note that `util.inspect()` is a synchronous method that is mainly
intended as a debugging tool. Some input values can have a significant
performance overhead that can block the event loop. Use this function
Expand Down
6 changes: 4 additions & 2 deletions lib/internal/bootstrap/node.js
Original file line number Diff line number Diff line change
Expand Up @@ -472,8 +472,10 @@
// 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());
v8.previewWeakMap(new WeakMap(), 1);
v8.previewWeakSet(new WeakSet(), 1);
// Disable --allow_natives_syntax again unless it was explicitly
// specified on the command line.
const re = /^--allow[-_]natives[-_]syntax$/;
Expand Down
44 changes: 31 additions & 13 deletions lib/internal/v8.js
Original file line number Diff line number Diff line change
@@ -1,21 +1,39 @@
'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);
}

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

function previewMapIterator(it, n) {
return take(%MapIteratorClone(it), n);
// Retrieve all WeakMap instance key / value pairs up to `max`. `max` limits the
// number of key / value pairs returned. Make sure it is a positive number,
// otherwise V8 aborts. Passing through `0` returns all elements.
function previewWeakMap(weakMap, max) {
return %GetWeakMapEntries(weakMap, max);
}

function previewSetIterator(it, n) {
return take(%SetIteratorClone(it), n);
// Retrieve all WeakSet instance values up to `max`. `max` limits the
// number of key / value pairs returned. Make sure it is a positive number,
// otherwise V8 aborts. Passing through `0` returns all elements.
function previewWeakSet(weakSet, max) {
return %GetWeakSetValues(weakSet, max);
}

module.exports = { previewMapIterator, previewSetIterator };
module.exports = {
previewMapIterator,
previewSetIterator,
previewWeakMap,
previewWeakSet
};
95 changes: 84 additions & 11 deletions lib/util.js
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,12 @@ const {
const { TextDecoder, TextEncoder } = require('internal/encoding');
const { isBuffer } = require('buffer').Buffer;

const { previewMapIterator, previewSetIterator } = require('internal/v8');
const {
previewMapIterator,
previewSetIterator,
previewWeakMap,
previewWeakSet
} = require('internal/v8');

const {
getPromiseDetails,
Expand All @@ -51,6 +56,8 @@ const {
isPromise,
isSet,
isSetIterator,
isWeakMap,
isWeakSet,
isRegExp,
isDate,
isTypedArray
Expand Down Expand Up @@ -288,6 +295,8 @@ function inspect(value, opts) {
colors: inspectDefaultOptions.colors,
customInspect: inspectDefaultOptions.customInspect,
showProxy: inspectDefaultOptions.showProxy,
// TODO(BridgeAR): Deprecate `maxArrayLength` and replace it with
// `maxEntries`.
maxArrayLength: inspectDefaultOptions.maxArrayLength,
breakLength: inspectDefaultOptions.breakLength,
indentationLvl: 0,
Expand Down Expand Up @@ -325,6 +334,8 @@ Object.defineProperty(inspect, 'defaultOptions', {
if (options === null || typeof options !== 'object') {
throw new ERR_INVALID_ARG_TYPE('options', 'Object');
}
// TODO(BridgeAR): Add input validation and make sure `defaultOptions` are
// not configurable.
return _extend(inspectDefaultOptions, options);
}
});
Expand Down Expand Up @@ -462,6 +473,7 @@ function formatValue(ctx, value, recurseTimes, ln) {
let braces;
let noIterator = true;
let raw;
let extra;

// Iterators and the rest are split to reduce checks
if (value[Symbol.iterator]) {
Expand Down Expand Up @@ -559,6 +571,20 @@ function formatValue(ctx, value, recurseTimes, ln) {
} else if (isPromise(value)) {
braces[0] = `${prefix}{`;
formatter = formatPromise;
} else if (isWeakSet(value)) {
braces[0] = `${prefix}{`;
if (ctx.showHidden) {
formatter = formatWeakSet;
} else {
extra = '[items unknown]';
}
} else if (isWeakMap(value)) {
braces[0] = `${prefix}{`;
if (ctx.showHidden) {
formatter = formatWeakMap;
} else {
extra = '[items unknown]';
}
} else {
// Check boxed primitives other than string with valueOf()
// NOTE: `Date` has to be checked first!
Expand Down Expand Up @@ -613,6 +639,9 @@ function formatValue(ctx, value, recurseTimes, ln) {
ctx.seen.push(value);
const output = formatter(ctx, value, recurseTimes, keys);

if (extra !== undefined)
output.unshift(extra);

for (var i = 0; i < symbols.length; i++) {
output.push(formatProperty(ctx, value, recurseTimes, symbols[i], 0));
}
Expand Down Expand Up @@ -836,25 +865,69 @@ 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 formatWeakSet(ctx, value, recurseTimes, keys) {
const maxArrayLength = Math.max(ctx.maxArrayLength, 0);
const entries = previewWeakSet(value, maxArrayLength + 1);
const maxLength = Math.min(maxArrayLength, entries.length);
let output = new Array(maxLength);
for (var i = 0; i < maxLength; ++i)
output[i] = formatValue(ctx, entries[i], recurseTimes);
// Sort all entries to have a halfway reliable output (if more entries than
// retrieved ones exist, we can not reliably return the same output).
output = output.sort();
if (entries.length > maxArrayLength)
output.push('... more items');
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: the corresponding code in formatArray() is:

node/lib/util.js

Lines 771 to 772 in 982e3bd

if (remaining > 0)
output[i++] = `... ${remaining} more item${remaining > 1 ? 's' : ''}`;

We could do that instead.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I do not see how that is possible. We have to explicitly enter a maximum number of entries we want. This is a upper bound and to be able to show that there are more entries, I actually get one more than requested with the regular maximum. We would have to get all entries each time (by passing through e.g. the maximum number of entries a Array may have) but that would be a significant overhead.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I could get two more items to show "1 more item" and "more items".

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah I misread it then. LGTM!

for (i = 0; i < keys.length; i++)
output.push(formatProperty(ctx, value, recurseTimes, keys[i], 0));
return output;
}

function formatWeakMap(ctx, value, recurseTimes, keys) {
const maxArrayLength = Math.max(ctx.maxArrayLength, 0);
const entries = previewWeakMap(value, maxArrayLength + 1);
// Entries exist as [key1, val1, key2, val2, ...]
const remainder = entries.length / 2 > maxArrayLength;
const len = entries.length / 2 - (remainder ? 1 : 0);
const maxLength = Math.min(maxArrayLength, len);
let output = new Array(maxLength);
for (var i = 0; i < len; i++) {
const pos = i * 2;
output[i] = `${formatValue(ctx, entries[pos], recurseTimes)} => ` +
formatValue(ctx, entries[pos + 1], recurseTimes);
}
// Sort all entries to have a halfway reliable output (if more entries than
// retrieved ones exist, we can not reliably return the same output).
output = output.sort();
if (remainder > 0)
output.push('... more items');
for (i = 0; i < keys.length; i++)
output.push(formatProperty(ctx, value, recurseTimes, keys[i], 0));
return output;
}

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
58 changes: 57 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 Expand Up @@ -1341,3 +1349,51 @@ util.inspect(process);
expect = '{\n a: \'12 45 78 01 34 \' +\n \'67 90 23\'\n}';
assert.strictEqual(out, expect);
}

{ // Test WeakMap
const obj = {};
const arr = [];
const weakMap = new WeakMap([[obj, arr], [arr, obj]]);
let out = util.inspect(weakMap, { showHidden: true });
let expect = 'WeakMap { [ [length]: 0 ] => {}, {} => [ [length]: 0 ] }';
assert.strictEqual(out, expect);

out = util.inspect(weakMap);
expect = 'WeakMap { [items unknown] }';
assert.strictEqual(out, expect);

out = util.inspect(weakMap, { maxArrayLength: 0, showHidden: true });
expect = 'WeakMap { ... more items }';
assert.strictEqual(out, expect);

weakMap.extra = true;
out = util.inspect(weakMap, { maxArrayLength: 1, showHidden: true });
// It is not possible to determine the output reliable.
expect = 'WeakMap { [ [length]: 0 ] => {}, ... more items, extra: true }';
const expectAlt = 'WeakMap { {} => [ [length]: 0 ], ... more items, ' +
'extra: true }';
assert(out === expect || out === expectAlt);
}

{ // Test WeakSet
const weakSet = new WeakSet([{}, [1]]);
let out = util.inspect(weakSet, { showHidden: true });
let expect = 'WeakSet { [ 1, [length]: 1 ], {} }';
assert.strictEqual(out, expect);

out = util.inspect(weakSet);
expect = 'WeakSet { [items unknown] }';
assert.strictEqual(out, expect);

out = util.inspect(weakSet, { maxArrayLength: -2, showHidden: true });
expect = 'WeakSet { ... more items }';
assert.strictEqual(out, expect);

weakSet.extra = true;
out = util.inspect(weakSet, { maxArrayLength: 1, showHidden: true });
// It is not possible to determine the output reliable.
expect = 'WeakSet { {}, ... more items, extra: true }';
const expectAlt = 'WeakSet { [ 1, [length]: 1 ], ... more items, ' +
'extra: true }';
assert(out === expect || out === expectAlt);
}