From b73cb7bf9cd8825acda0d378a9afa1c3b1062f51 Mon Sep 17 00:00:00 2001 From: Marcos Casagrande Date: Mon, 26 Sep 2022 08:55:22 +0200 Subject: [PATCH] perf(ext/console): break on iterableLimit & better sparse array handling (#15935) --- cli/tests/unit/console_test.ts | 52 +++++++++++++- ext/console/02_console.js | 126 +++++++++++++++++++++++---------- 2 files changed, 136 insertions(+), 42 deletions(-) diff --git a/cli/tests/unit/console_test.ts b/cli/tests/unit/console_test.ts index ad7c0caa7e..0ddbe278ce 100644 --- a/cli/tests/unit/console_test.ts +++ b/cli/tests/unit/console_test.ts @@ -759,7 +759,54 @@ Deno.test(function consoleTestStringifyIterable() { `[ <4 empty items>, 0, 0, <4 empty items> ]`, ); - /* TODO(ry) Fix this test + const emptyArray = Array(5000); + assertEquals( + stringify(emptyArray), + `[ <5000 empty items> ]`, + ); + + assertEquals( + stringify(Array(1)), + `[ <1 empty item> ]`, + ); + + const withEmptyElAndMoreItems = Array(500); + withEmptyElAndMoreItems.fill(0, 50, 80); + withEmptyElAndMoreItems.fill(2, 100, 120); + withEmptyElAndMoreItems.fill(3, 140, 160); + withEmptyElAndMoreItems.fill(4, 180); + assertEquals( + stringify(withEmptyElAndMoreItems), + `[ + <50 empty items>, 0, 0, 0, + 0, 0, 0, 0, + 0, 0, 0, 0, + 0, 0, 0, 0, + 0, 0, 0, 0, + 0, 0, 0, 0, + 0, 0, 0, 0, + 0, 0, 0, <20 empty items>, + 2, 2, 2, 2, + 2, 2, 2, 2, + 2, 2, 2, 2, + 2, 2, 2, 2, + 2, 2, 2, 2, + <20 empty items>, 3, 3, 3, + 3, 3, 3, 3, + 3, 3, 3, 3, + 3, 3, 3, 3, + 3, 3, 3, 3, + 3, <20 empty items>, 4, 4, + 4, 4, 4, 4, + 4, 4, 4, 4, + 4, 4, 4, 4, + 4, 4, 4, 4, + 4, 4, 4, 4, + 4, 4, 4, 4, + ... 294 more items +]`, + ); + const lWithEmptyEl = Array(200); lWithEmptyEl.fill(0, 50, 80); assertEquals( @@ -776,9 +823,8 @@ Deno.test(function consoleTestStringifyIterable() { 0, 0, 0, 0, 0, 0, 0, <120 empty items> -]` +]`, ); - */ }); Deno.test(function consoleTestStringifyIterableWhenGrouped() { diff --git a/ext/console/02_console.js b/ext/console/02_console.js index 24918ea1a1..5b65774d94 100644 --- a/ext/console/02_console.js +++ b/ext/console/02_console.js @@ -59,6 +59,7 @@ SafeSet, SetPrototype, SetPrototypeEntries, + SetPrototypeGetSize, Symbol, SymbolPrototype, SymbolPrototypeToString, @@ -90,6 +91,7 @@ MapPrototypeDelete, MapPrototypeEntries, MapPrototypeForEach, + MapPrototypeGetSize, Error, ErrorPrototype, ErrorCaptureStackTrace, @@ -110,6 +112,7 @@ ReflectGetOwnPropertyDescriptor, ReflectGetPrototypeOf, ReflectHas, + TypedArrayPrototypeGetLength, WeakMapPrototype, WeakSetPrototype, } = window.__bootstrap.primordials; @@ -390,19 +393,23 @@ const entries = []; let iter; let valueIsTypedArray = false; + let entriesLength; switch (options.typeName) { case "Map": iter = MapPrototypeEntries(value); + entriesLength = MapPrototypeGetSize(value); break; case "Set": iter = SetPrototypeEntries(value); + entriesLength = SetPrototypeGetSize(value); break; case "Array": - iter = ArrayPrototypeEntries(value); + entriesLength = value.length; break; default: if (isTypedArray(value)) { + entriesLength = TypedArrayPrototypeGetLength(value); iter = ArrayPrototypeEntries(value); valueIsTypedArray = true; } else { @@ -410,41 +417,61 @@ } } - let entriesLength = 0; - const next = () => { - return iter.next(); - }; - while (true) { - let el; - try { - const res = iter.next(); - if (res.done) { - break; - } - el = res.value; - } catch (err) { - if (valueIsTypedArray) { - // TypedArray.prototype.entries doesn't throw, unless the ArrayBuffer - // is detached. We don't want to show the exception in that case, so - // we catch it here and pretend the ArrayBuffer has no entries (like - // Chrome DevTools does). - break; - } - throw err; - } - if (entriesLength < inspectOptions.iterableLimit) { + if (options.typeName === "Array") { + for ( + let i = 0, j = 0; + i < entriesLength && j < inspectOptions.iterableLimit; + i++, j++ + ) { inspectOptions.indentLevel++; - ArrayPrototypePush( - entries, - options.entryHandler( - el, - inspectOptions, - FunctionPrototypeBind(next, iter), - ), + const { entry, skipTo } = options.entryHandler( + [i, value[i]], + inspectOptions, ); + ArrayPrototypePush(entries, entry); inspectOptions.indentLevel--; + + if (skipTo) { + // subtract skipped (empty) items + entriesLength -= skipTo - i; + i = skipTo; + } + } + } else { + let i = 0; + while (true) { + let el; + try { + const res = iter.next(); + if (res.done) { + break; + } + el = res.value; + } catch (err) { + if (valueIsTypedArray) { + // TypedArray.prototype.entries doesn't throw, unless the ArrayBuffer + // is detached. We don't want to show the exception in that case, so + // we catch it here and pretend the ArrayBuffer has no entries (like + // Chrome DevTools does). + break; + } + throw err; + } + if (i < inspectOptions.iterableLimit) { + inspectOptions.indentLevel++; + ArrayPrototypePush( + entries, + options.entryHandler( + el, + inspectOptions, + ), + ); + inspectOptions.indentLevel--; + } else { + break; + } + i++; } - entriesLength++; } if (options.sort) { @@ -805,24 +832,45 @@ inspectOptions, ) { const gray = maybeColor(colors.gray, inspectOptions); + let lastValidIndex = 0; + let keys; const options = { typeName: "Array", displayName: "", delims: ["[", "]"], - entryHandler: (entry, inspectOptions, next) => { + entryHandler: (entry, inspectOptions) => { const [index, val] = entry; let i = index; + lastValidIndex = index; if (!ObjectPrototypeHasOwnProperty(value, i)) { - i++; - while (!ObjectPrototypeHasOwnProperty(value, i) && i < value.length) { - next(); - i++; + let skipTo; + keys = keys || ObjectKeys(value); + i = value.length; + if (keys.length === 0) { + // fast path, all items are empty + skipTo = i; + } else { + // Not all indexes are empty or there's a non-index property + // Find first non-empty array index + while (keys.length) { + const key = ArrayPrototypeShift(keys); + // check if it's a valid array index + if (key > lastValidIndex && key < 2 ** 32 - 1) { + i = Number(key); + break; + } + } + + skipTo = i - 1; } const emptyItems = i - index; const ending = emptyItems > 1 ? "s" : ""; - return gray(`<${emptyItems} empty item${ending}>`); + return { + entry: gray(`<${emptyItems} empty item${ending}>`), + skipTo, + }; } else { - return inspectValueWithQuotes(val, inspectOptions); + return { entry: inspectValueWithQuotes(val, inspectOptions) }; } }, group: inspectOptions.compact,