diff --git a/src/bun_core/string/mod.rs b/src/bun_core/string/mod.rs index 522a11a2a279..486f16a43699 100644 --- a/src/bun_core/string/mod.rs +++ b/src/bun_core/string/mod.rs @@ -560,6 +560,16 @@ impl String { core::ptr::null_mut() } } + /// Address of the JSC atom behind `self`: equal for equal text of live atoms on one thread. + #[inline] + pub fn atom_address(&self) -> Option { + if self.tag != Tag::WTFStringImpl { + return None; + } + let atom = self.as_wtf(); + debug_assert!(atom.is_atom()); + core::num::NonZeroUsize::new(core::ptr::from_ref(atom).addr()) + } /// An isolated copy of a WTF-backed impl (+1, `clone()` for other tags), /// for handing the value to one other thread; not for sharing one impl /// between VMs. diff --git a/src/jsc/ConsoleObject.rs b/src/jsc/ConsoleObject.rs index e0a876975832..996b6f43c740 100644 --- a/src/jsc/ConsoleObject.rs +++ b/src/jsc/ConsoleObject.rs @@ -590,6 +590,51 @@ struct Column { width: u32, } +/// The column of a property name, by [`column_key`]. +type ColumnIndex = HashMap; + +/// Comparing this many keys costs about as much as one index lookup. +const MAX_SCANNED_COLUMNS: usize = 8; + +/// The identity of a name from `JSPropertyIterator`: its atom's address, 0 for the empty name. +#[inline(always)] +fn column_key(name: &BunString) -> usize { + match name.atom_address() { + Some(address) => address.get(), + None => { + debug_assert!(name.is_empty()); + 0 + } + } +} + +/// Finds or adds the column of `name` through the index, which the first call builds. +#[inline(never)] +fn indexed_column( + columns: &mut Vec, + index: &mut Option, + name: &BunString, +) -> usize { + let index = index.get_or_insert_with(|| { + let mut index = ColumnIndex::with_capacity(columns.len()); + for (i, column) in columns.iter().enumerate().skip(1) { + index + .put_no_clobber(column_key(&column.name), i) + .expect("unreachable"); + } + index + }); + let entry = index.get_or_put(column_key(name)).expect("unreachable"); + if !entry.found_existing { + *entry.value_ptr = columns.len(); + columns.push(Column { + name: name.clone(), + width: 1, + }); + } + *entry.value_ptr +} + enum RowKey { /// Property-name UTF-8 bytes + visible width (plain-object tabular data). Str { @@ -714,6 +759,7 @@ impl<'a> TablePrinter<'a> { &mut self, cell_text: &mut Vec, columns: &mut Vec, + column_index: &mut Option, row_key: RowKey, row_value: JSValue, ) -> JsResult { @@ -768,21 +814,25 @@ impl<'a> TablePrinter<'a> { }, )?; + // One object's names are distinct: a cell can only match an earlier row's column. + let row_start = columns.len(); while let Some((col_key, value)) = cols_iter.next()? { // find or create the column for the property - let col_idx: usize = 'brk: { - // reshaped for borrowck — split find/append. - if let Some(idx) = - columns[1..].iter().position(|col| col.name.eql(&col_key)) - { - break 'brk 1 + idx; + let col_idx: usize = if row_start > 1 + MAX_SCANNED_COLUMNS { + indexed_column(columns, column_index, &col_key) + } else { + let key = column_key(&col_key); + let earlier = &columns[1..row_start]; + match earlier.iter().position(|col| column_key(&col.name) == key) { + Some(idx) => 1 + idx, + None => { + columns.push(Column { + name: (*col_key).clone(), + width: 1, + }); + columns.len() - 1 + } } - - columns.push(Column { - name: (*col_key).clone(), - width: 1, - }); - break 'brk columns.len() - 1; }; let cell = self.format_cell::(cell_text, value)?; @@ -906,11 +956,13 @@ impl<'a> TablePrinter<'a> { let mut cell_text: Vec = Vec::new(); let mut rows: Vec = Vec::new(); { + let mut column_index: Option = None; if self.is_iterable { struct Ctx<'c, 'a> { this: &'c mut TablePrinter<'a>, cell_text: &'c mut Vec, columns: &'c mut Vec, + column_index: &'c mut Option, rows: &'c mut Vec, idx: u32, err: Option, @@ -921,6 +973,7 @@ impl<'a> TablePrinter<'a> { this: self, cell_text: &mut cell_text, columns: &mut columns, + column_index: &mut column_index, rows: &mut rows, idx: 0, err: None, @@ -942,6 +995,7 @@ impl<'a> TablePrinter<'a> { match ctx.this.collect_row::( ctx.cell_text, ctx.columns, + ctx.column_index, RowKey::Num(ctx.idx), value, ) { @@ -974,6 +1028,7 @@ impl<'a> TablePrinter<'a> { let row = self.collect_row::( &mut cell_text, &mut columns, + &mut column_index, key, value, )?; diff --git a/src/jsc/JSPropertyIterator.rs b/src/jsc/JSPropertyIterator.rs index b6c6ad38faff..4aa37fae3f50 100644 --- a/src/jsc/JSPropertyIterator.rs +++ b/src/jsc/JSPropertyIterator.rs @@ -167,6 +167,7 @@ impl<'a> JSPropertyIterator<'a> { /// The name borrows the C++ iterator's storage (no ref) and is valid until /// `self` drops, hence `&self`: a yielded name may be held across later /// calls. The value is `JSValue::ZERO` unless `options.include_value`. + /// Without `options.include_symbols` a name is empty or a JSC atom (`String::atom_address`). pub fn next(&self) -> JsResult, JSValue)>> { loop { let i: usize = self.iter_i.get() as usize; @@ -222,6 +223,9 @@ impl<'a> JSPropertyIterator<'a> { if name.is_dead() { continue; } + debug_assert!( + self.options.include_symbols || name.is_empty() || name.atom_address().is_some() + ); if self.options.skip_empty_name && name.is_empty() { continue; diff --git a/test/js/bun/console/console-table.test.ts b/test/js/bun/console/console-table.test.ts index 416c9c27d6d8..ddb069214fc6 100644 --- a/test/js/bun/console/console-table.test.ts +++ b/test/js/bun/console/console-table.test.ts @@ -1,6 +1,7 @@ import { spawnSync } from "bun"; +import { jscDescribe } from "bun:jsc"; import { describe, expect, test } from "bun:test"; -import { bunEnv, bunExe } from "harness"; +import { bunEnv, bunExe, isASAN, isDebug, isWindows, withoutAggressiveGC } from "harness"; // `console.table` and `Bun.inspect.table` share the same native TablePrinter, // so we can render in-process instead of spawning a subprocess per case. @@ -363,3 +364,306 @@ console.log("calls=" + calls);`, expect({ stdout, stderr, exitCode }).toEqual({ stdout: box("1") + "calls=1\n", stderr: "", exitCode: 0 }); }); }); + +// A cell's column is found among the columns that earlier rows made, by which +// property name it is and not by the text of that name: compared with each of +// them while they are few, through an index once they are not +// (MAX_SCANNED_COLUMNS in ConsoleObject.rs). Both have to agree on which names +// are the same column, and neither may hand out the index or Values column. +describe("console.table column lookup", () => { + // Two rows with the same 7-character keys, each cell holding its own key, so + // the header and both rows render as the same line of 7-wide cells and the + // whole table can be spelled out. The second row lists the keys in the + // opposite order: each of its cells has to be found in the column the first + // row created, not in the column at the same position. + function wideTable(columns: number) { + const keys = Array.from({ length: columns }, (_, i) => "c" + (100000 + i)); + const first: Record = {}; + for (const key of keys) first[key] = key; + const second: Record = {}; + for (let i = keys.length - 1; i >= 0; i--) second[keys[i]] = keys[i]; + + const rule = (left: string, mid: string, right: string) => { + const segment = mid + "─────────"; + return left + "───" + Buffer.alloc(Buffer.byteLength(segment) * columns, segment).toString() + right + "\n"; + }; + const body = "│ " + keys.join(" │ ") + " │\n"; + const expected = + rule("┌", "┬", "┐") + "│ " + body + rule("├", "┼", "┤") + "│ 0 " + body + "│ 1 " + body + rule("└", "┴", "┘"); + return { rows: [first, second], expected }; + } + + test("cells land in the column their name was first seen in", () => { + const { rows, expected } = wideTable(300); + expect(Bun.inspect.table(rows)).toBe(expected); + }); + + // Names with a text that defeats a lookup which hashes it: all of them have + // one `Bun.hash.wyhash`. For a key of 16 bytes that hash no longer depends + // on bytes 4..8 and 12..16 once the other eight are its second secret. + const hex = (i: number) => i.toString(16).padStart(4, "0"); + const sameHashName = (i: number) => "\xd1\x7e\x03\xe7" + hex(i >>> 16) + "\xdb\x28\xb4\xa0" + hex(i & 0xffff); + const plainName = (i: number) => "c" + (100000 + i); + + test("the names made to share one hash share it", () => { + const hashes = Array.from({ length: 100 }, (_, i) => Bun.hash.wyhash(Buffer.from(sameHashName(i), "latin1"))); + expect(new Set(hashes).size).toBe(1); + }); + + // A table of up to three rows: `columns` names, the same names backwards, + // as many new names. The first row makes its columns without a lookup, the + // second finds each of them through the index, the third adds to the + // index. `wider` is how many times as wide the wide table is on a release + // build: wide enough there for each of these to fail when its part of the + // lookup is taken out, narrow enough for main to fail in under 5 seconds. + test.each([ + { shape: "one row of plain names", rows: 1, name: plainName, wider: 256 }, + { shape: "three rows of plain names", rows: 3, name: plainName, wider: 128 }, + { shape: "three rows of names that share one hash", rows: 3, name: sameHashName, wider: 128 }, + ])("rendering is linear in the number of columns: $shape", ({ rows, name, wider }) => { + function table(columns: number) { + const indexes = Array.from({ length: columns }, (_, i) => i); + const row = (indexes: number[]) => Object.fromEntries(indexes.map(i => [name(i), i])); + return [row(indexes), row(indexes.toReversed()), row(indexes.map(i => columns + i))].slice(0, rows); + } + + // Whatever the names look like: no name made two columns, and the first + // two rows have the same cells. + function check(out: string, columns: number) { + const lines = out.split("\n"); + expect({ + columns: lines[0].split("┬").length - 1, + sameCells: rows < 2 || lines[3].slice("│ 0".length) === lines[4].slice("│ 1".length), + }).toEqual({ columns: rows < 3 ? columns : 2 * columns, sameCells: true }); + } + + // CPU time leaves out the time that other processes run. On Windows it + // only advances with the clock interrupt, about every 16 ms. + const now = isWindows + ? () => performance.now() + : () => { + const { user, system } = process.cpuUsage(); + return (user + system) / 1000; + }; + + function render(data: object[], times: number) { + let out = ""; + const start = now(); + for (let i = 0; i < times; i++) out = Bun.inspect.table(data); + const ms = now() - start; + check(out, Object.keys(data[0]).length); + return ms; + } + + // `widthRatio` narrow tables have as many cells as one wide table, so the + // two take about as long when the cost of a cell does not depend on the + // width. The fastest of several runs has the least other work in it. A + // build that fails takes long for the wide table, and does not repeat it. + const widthRatio = isDebug || isASAN ? 64 : wider; + const narrowColumns = isDebug || isASAN ? 32 : 125; + const narrow = table(narrowColumns); + const wide = table(narrowColumns * widthRatio); + withoutAggressiveGC(() => { + // A collection that starts during a measurement would be most of it. + Bun.gc(true); + let narrowMs = Infinity; + for (let run = 0; run < 3; run++) narrowMs = Math.min(narrowMs, render(narrow, widthRatio)); + + const deadline = performance.now() + 1_500; + let wideMs = Infinity; + do { + Bun.gc(true); + wideMs = Math.min(wideMs, render(wide, 1)); + } while (wideMs >= 4 * narrowMs && performance.now() < deadline); + + expect(wideMs / narrowMs).toBeLessThan(4); + }); + }); + + // The column names and the cells of each row, trimmed, without the row-index + // column. + function shape(table: string) { + const cellsOf = (line: string) => + line + .split("│") + .slice(2, -1) + .map(cell => cell.trim()); + const [, header, , ...lines] = table.split("\n"); + return { columns: cellsOf(header), cells: lines.slice(0, -2).map(cellsOf) }; + } + + // Enough columns to leave the scan behind. Keep it above MAX_SCANNED_COLUMNS. + const fillerNames = Array.from({ length: 10 }, (_, i) => "f" + i); + const filler = Object.fromEntries(fillerNames.map(name => [name, 0])); + + // `rows` on their own are narrow enough to be scanned. Behind a row of + // filler columns, every one of their names goes through the index. + function expectColumns(rows: unknown[], columns: string[], cells: string[][]) { + expect(shape(Bun.inspect.table(rows))).toEqual({ columns, cells }); + + const blank = (names: unknown[]) => names.map(() => ""); + expect(shape(Bun.inspect.table([filler, ...rows]))).toEqual({ + columns: [...fillerNames, ...columns], + cells: [[...fillerNames.map(() => "0"), ...blank(columns)], ...cells.map(row => [...blank(fillerNames), ...row])], + }); + } + + // What the table has to show: columns in first-seen order, a cell where the + // row has that key. + function expectedShape(rows: Record[]) { + const columns: string[] = []; + for (const row of rows) for (const key of Object.keys(row)) if (!columns.includes(key)) columns.push(key); + return { columns, cells: rows.map(row => columns.map(key => (key in row ? String(row[key]) : ""))) }; + } + + test("rows that each share some columns with earlier rows and add their own", () => { + // Row r has the keys k(2r) to k(2r+5), listed backwards on odd rows, so it + // finds four columns of earlier rows and makes two. The columns grow past + // the scan limit at the fourth row. Later rows find indexed columns and + // columns that were added after the index was built. + const rows = Array.from({ length: 12 }, (_, r) => { + const keys = Array.from({ length: 6 }, (_, i) => "k" + (2 * r + i)); + if (r % 2) keys.reverse(); + return Object.fromEntries(keys.map(key => [key, r])); + }); + expect(shape(Bun.inspect.table(rows))).toEqual(expectedShape(rows)); + }); + + test("narrow rows after a wide first row", () => { + const wide = Object.fromEntries(Array.from({ length: 20 }, (_, i) => ["w" + i, i])); + const rows = [wide, { w19: "a", w0: "b" }, {}, { n0: 1, w7: 2 }, { n0: 3 }, wide]; + expect(shape(Bun.inspect.table(rows))).toEqual(expectedShape(rows)); + }); + + test("names with non-ASCII characters are matched across rows", () => { + // 日本 is stored as UTF-16, é as Latin-1. + expectColumns( + [ + { 日本: 1, é: 2 }, + { é: 3, 日本: 4 }, + ], + ["日本", "é"], + [ + ["1", "2"], + ["4", "3"], + ], + ); + }); + + test("the empty name is a column like any other", () => { + expectColumns( + [ + { "": 1, "a": 2 }, + { "a": 3, "": 4 }, + ], + ["", "a"], + [ + ["1", "2"], + ["4", "3"], + ], + ); + }); + + test("a property named like the index column gets its own column", () => { + // The name " " trims to "" here. The point is the extra column. + expectColumns([{ " ": 1 }, { " ": 2 }], [""], [["1"], ["2"]]); + }); + + test("a property named Values is separate from the Values column", () => { + expectColumns( + [{ Values: 1 }, 2, { Values: 3 }], + ["Values", "Values"], + [ + ["1", ""], + ["", "2"], + ["3", ""], + ], + ); + }); + + test("one name spelled by different strings is one column", () => { + // No string in this file spells these names, so the first row that has + // one is where its text becomes a property name. + const unique = Math.random().toString(36).slice(2); + const narrow = "n" + unique; + const late = "l" + unique; + const wide = "日" + unique; + const respell = (name: string) => name.slice(0, 1) + name.slice(1); + // A flat 16-bit string keeps its storage when it becomes a property name. + const in16Bit = (name: string) => new TextDecoder("utf-16le").decode(Buffer.from(name, "utf16le")); + const rows = [ + { [in16Bit(narrow)]: 1, [respell(wide)]: 2, [respell(late)]: 9 }, + { [respell(narrow)]: 3, [respell(wide)]: 4, [in16Bit(late)]: 10 }, + JSON.parse(`{${JSON.stringify(wide)}:5,${JSON.stringify(narrow)}:6}`), + JSON.parse(`{${JSON.stringify(narrow)}:7}`), + new Proxy( + {}, + { + ownKeys: () => [respell(narrow), respell(wide)], + getOwnPropertyDescriptor: () => ({ value: 8, enumerable: true, configurable: true }), + get: () => 8, + }, + ), + ]; + // `narrow` is 16-bit although its text fits in 8 bits, and `late` is 8-bit + // when its 16-bit spelling arrives. + const stored = Object.fromEntries(Object.keys(rows[0]).map(key => [key, /8Bit:\(\d\)/.exec(jscDescribe(key))![0]])); + expect(stored).toEqual({ [narrow]: "8Bit:(0)", [wide]: "8Bit:(0)", [late]: "8Bit:(1)" }); + expectColumns( + rows, + [narrow, wide, late], + [ + ["1", "2", "9"], + ["3", "4", "10"], + ["6", "5", ""], + ["7", "", ""], + ["8", "8", ""], + ], + ); + }); + + test("an index is one column for array rows and object rows", () => { + expectColumns( + [["a", "b"], { 1: "c", 0: "d" }, { "0": "e" }, new Uint8Array([7, 8])], + ["0", "1"], + [ + ["a", "b"], + ["d", "c"], + ["e", ""], + ["7", "8"], + ], + ); + }); + + test("a column keeps its name after the rows that had it are collected", () => { + // Rows alternate between two sets of names, so the last row that had the + // names of a new row is two collections back. + const unique = Math.random().toString(36).slice(2); + const families = ["gc", "zz"].map(prefix => Array.from({ length: 12 }, (_, i) => prefix + unique + i)); + const body = (row: number) => "{" + families[row % 2].map(name => JSON.stringify(name) + ":" + row).join(",") + "}"; + const collected: boolean[] = []; + function* rows() { + const refs: WeakRef[] = []; + for (let row = 0; row < 6; row++) { + { + const value = JSON.parse(body(row)); + refs.push(new WeakRef(value)); + yield value; + } + // The table can still hold the row it just read. The row before it + // is garbage, and only the table still uses its names. + Bun.gc(true); + if (row > 0) collected.push(refs[row - 1].deref() === undefined); + } + } + const blank = families[0].map(() => ""); + expect(shape(Bun.inspect.table(rows()))).toEqual({ + columns: families.flat(), + cells: [0, 1, 2, 3, 4, 5].map(row => { + const cells = families[0].map(() => String(row)); + return row % 2 ? [...blank, ...cells] : [...cells, ...blank]; + }), + }); + expect(collected).toEqual([true, true, true, true, true]); + }); +});