Skip to content

Commit c90c676

Browse files
addaleaxaddaleax
committed
console,util: avoid pair array generation in C++
Use a plain `[key, value, key, value]`-style list instead of an array of pairs for inspecting collections. This also fixes a bug with `console.table()` where inspecting a non-key-value `MapIterator` would have led to odd results. Refs: #20719 (comment)
1 parent 70cc5da commit c90c676

5 files changed

Lines changed: 65 additions & 37 deletions

File tree

‎lib/console.js‎

Lines changed: 17 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -344,17 +344,26 @@ Console.prototype.table = function(tabularData, properties) {
344344
const getIndexArray = (length) => ArrayFrom({ length }, (_, i) => inspect(i));
345345

346346
const mapIter = isMapIterator(tabularData);
347+
let isKeyValue = false;
347348
if (mapIter)
348-
tabularData = previewEntries(tabularData);
349+
[ tabularData, isKeyValue ] = previewEntries(tabularData);
349350

350-
if (mapIter || isMap(tabularData)) {
351+
if (isKeyValue || isMap(tabularData)) {
351352
const keys = [];
352353
const values = [];
353354
let length = 0;
354-
for (const [k, v] of tabularData) {
355-
keys.push(inspect(k));
356-
values.push(inspect(v));
357-
length++;
355+
if (mapIter) {
356+
for (let i = 0; i < tabularData.length / 2; ++i) {
357+
keys.push(inspect(tabularData[i * 2]));
358+
values.push(inspect(tabularData[i * 2 + 1]));
359+
length++;
360+
}
361+
} else {
362+
for (const [k, v] of tabularData) {
363+
keys.push(inspect(k));
364+
values.push(inspect(v));
365+
length++;
366+
}
358367
}
359368
return final([
360369
iterKey, keyKey, valuesKey
@@ -367,9 +376,9 @@ Console.prototype.table = function(tabularData, properties) {
367376

368377
const setIter = isSetIterator(tabularData);
369378
if (setIter)
370-
tabularData = previewEntries(tabularData);
379+
[ tabularData ] = previewEntries(tabularData);
371380

372-
const setlike = setIter || isSet(tabularData);
381+
const setlike = setIter || (mapIter && !isKeyValue) || isSet(tabularData);
373382
if (setlike) {
374383
const values = [];
375384
let length = 0;

‎lib/util.js‎

Lines changed: 20 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -906,7 +906,7 @@ function formatMap(ctx, value, recurseTimes, keys) {
906906

907907
function formatWeakSet(ctx, value, recurseTimes, keys) {
908908
const maxArrayLength = Math.max(ctx.maxArrayLength, 0);
909-
const entries = previewEntries(value).slice(0, maxArrayLength + 1);
909+
const [ entries ] = previewEntries(value).slice(0, maxArrayLength + 1);
910910
const maxLength = Math.min(maxArrayLength, entries.length);
911911
let output = new Array(maxLength);
912912
for (var i = 0; i < maxLength; ++i)
@@ -923,14 +923,16 @@ function formatWeakSet(ctx, value, recurseTimes, keys) {
923923

924924
function formatWeakMap(ctx, value, recurseTimes, keys) {
925925
const maxArrayLength = Math.max(ctx.maxArrayLength, 0);
926-
const entries = previewEntries(value).slice(0, maxArrayLength + 1);
927-
const remainder = entries.length > maxArrayLength;
928-
const len = entries.length - (remainder ? 1 : 0);
926+
const [ entries ] = previewEntries(value).slice(0, (maxArrayLength + 1) * 2);
927+
// Entries exist as [key1, val1, key2, val2, ...]
928+
const remainder = entries.length / 2 > maxArrayLength;
929+
const len = entries.length / 2 - (remainder ? 1 : 0);
929930
const maxLength = Math.min(maxArrayLength, len);
930931
let output = new Array(maxLength);
931-
for (var i = 0; i < len; i++) {
932-
output[i] = `${formatValue(ctx, entries[i][0], recurseTimes)} => ` +
933-
formatValue(ctx, entries[i][1], recurseTimes);
932+
for (var i = 0; i < maxLength; i++) {
933+
const pos = i * 2;
934+
output[i] = `${formatValue(ctx, entries[pos], recurseTimes)} => ` +
935+
formatValue(ctx, entries[pos + 1], recurseTimes);
934936
}
935937
// Sort all entries to have a halfway reliable output (if more entries than
936938
// retrieved ones exist, we can not reliably return the same output).
@@ -942,9 +944,19 @@ function formatWeakMap(ctx, value, recurseTimes, keys) {
942944
return output;
943945
}
944946

947+
function zip2(list) {
948+
const ret = Array(list.length / 2);
949+
for (let i = 0; i < ret.length; ++i)
950+
ret[i] = [list[2 * i], list[2 * i + 1]];
951+
return ret;
952+
}
953+
945954
function formatCollectionIterator(ctx, value, recurseTimes, keys) {
946955
const output = [];
947-
for (const entry of previewEntries(value)) {
956+
let [ entries, isKeyValue ] = previewEntries(value);
957+
if (isKeyValue)
958+
entries = zip2(entries);
959+
for (const entry of entries) {
948960
if (ctx.maxArrayLength === output.length) {
949961
output.push('... more items');
950962
break;

‎src/node_util.cc‎

Lines changed: 6 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -53,29 +53,16 @@ static void PreviewEntries(const FunctionCallbackInfo<Value>& args) {
5353
if (!args[0]->IsObject())
5454
return;
5555

56+
Environment* env = Environment::GetCurrent(args);
5657
bool is_key_value;
5758
Local<Array> entries;
5859
if (!args[0].As<Object>()->PreviewEntries(&is_key_value).ToLocal(&entries))
5960
return;
60-
if (!is_key_value)
61-
return args.GetReturnValue().Set(entries);
62-
63-
uint32_t length = entries->Length();
64-
CHECK_EQ(length % 2, 0);
65-
66-
Environment* env = Environment::GetCurrent(args);
67-
Local<Context> context = env->context();
68-
69-
Local<Array> pairs = Array::New(env->isolate(), length / 2);
70-
for (uint32_t i = 0; i < length / 2; i++) {
71-
Local<Array> pair = Array::New(env->isolate(), 2);
72-
pair->Set(context, 0, entries->Get(context, i * 2).ToLocalChecked())
73-
.FromJust();
74-
pair->Set(context, 1, entries->Get(context, i * 2 + 1).ToLocalChecked())
75-
.FromJust();
76-
pairs->Set(context, i, pair).FromJust();
77-
}
78-
args.GetReturnValue().Set(pairs);
61+
Local<Array> ret = Array::New(env->isolate(), 2);
62+
ret->Set(env->context(), 0, entries).FromJust();
63+
ret->Set(env->context(), 1, v8::Boolean::New(env->isolate(), is_key_value))
64+
.FromJust();
65+
return args.GetReturnValue().Set(ret);
7966
}
8067

8168
// Side effect-free stringification that will never throw exceptions.

‎test/parallel/test-console-table.js‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -117,6 +117,26 @@ test(new Map([[1, 1], [2, 2], [3, 3]]).entries(), `
117117
└───────────────────┴─────┴────────┘
118118
`);
119119

120+
test(new Map([[1, 1], [2, 2], [3, 3]]).values(), `
121+
┌───────────────────┬────────┐
122+
│ (iteration index) │ Values │
123+
├───────────────────┼────────┤
124+
│ 0 │ 1 │
125+
│ 1 │ 2 │
126+
│ 2 │ 3 │
127+
└───────────────────┴────────┘
128+
`);
129+
130+
test(new Map([[1, 1], [2, 2], [3, 3]]).keys(), `
131+
┌───────────────────┬────────┐
132+
│ (iteration index) │ Values │
133+
├───────────────────┼────────┤
134+
│ 0 │ 1 │
135+
│ 1 │ 2 │
136+
│ 2 │ 3 │
137+
└───────────────────┴────────┘
138+
`);
139+
120140
test(new Set([1, 2, 3]).values(), `
121141
┌───────────────────┬────────┐
122142
│ (iteration index) │ Values │

‎test/parallel/test-util-inspect.js‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -447,13 +447,13 @@ assert.strictEqual(util.inspect(-5e-324), '-5e-324');
447447
{
448448
const map = new Map();
449449
map.set(1, 2);
450-
const vals = previewEntries(map.entries());
450+
const [ vals ] = previewEntries(map.entries());
451451
const valsOutput = [];
452452
for (const o of vals) {
453453
valsOutput.push(o);
454454
}
455455

456-
assert.strictEqual(util.inspect(valsOutput), '[ [ 1, 2 ] ]');
456+
assert.strictEqual(util.inspect(valsOutput), '[ 1, 2 ]');
457457
}
458458

459459
// Test for other constructors in different context.

0 commit comments

Comments
 (0)