diff --git a/src/ast/symbol.rs b/src/ast/symbol.rs index 0dbe7cb2b56c..95efffd74e2b 100644 --- a/src/ast/symbol.rs +++ b/src/ast/symbol.rs @@ -413,6 +413,23 @@ pub type List<'a> = bun_alloc::ArenaVec<'a, Symbol>; /// clones are owned for the link lifetime — global allocator, no arena tag. pub type NestedList = Vec>; +/// [`Map::follow`] (path compression included) for the parser's flat per-file symbol table. +pub fn follow_symbols(symbols: &[Symbol], ref_: Ref) -> Ref { + let mut root = ref_; + loop { + let link = symbols[root.inner_index() as usize].link.get(); + if !link.is_valid() { + break; + } + root = link; + } + let mut current = ref_; + while current != root { + current = symbols[current.inner_index() as usize].link.replace(root); + } + root +} + impl Symbol { pub(crate) fn merge_contents_with(&mut self, old: &mut Symbol) { self.use_count_estimate += old.use_count_estimate; diff --git a/src/js_parser/p.rs b/src/js_parser/p.rs index 31ffcc1c6a07..7c8fb7ebf142 100644 --- a/src/js_parser/p.rs +++ b/src/js_parser/p.rs @@ -2121,15 +2121,8 @@ impl<'a, const TYPESCRIPT: bool, const SCAN_ONLY: bool> P<'a, TYPESCRIPT, SCAN_O /// returns. A block `var n` hoisted onto a parameter `n` is a linked ref /// of the same variable. pub(crate) fn record_assignment(&mut self, ref_: Ref) { - let mut ref_ = ref_; - loop { - let symbol = &mut self.symbols[ref_.inner_index() as usize]; - if !symbol.has_link() { - symbol.set_has_been_assigned_to(true); - return; - } - ref_ = symbol.link.get(); - } + let root = js_ast::symbol::follow_symbols(&self.symbols, ref_); + self.symbols[root.inner_index() as usize].set_has_been_assigned_to(true); } pub(crate) fn log_arrow_arg_errors(&mut self, errors: &mut DeferredArrowArgErrors) { @@ -5576,12 +5569,8 @@ impl<'a, const TYPESCRIPT: bool, const SCAN_ONLY: bool> P<'a, TYPESCRIPT, SCAN_O for i in 0..self.relocated_top_level_vars.len() { // Follow links because "var" declarations may be merged due to hoisting let mut local = self.relocated_top_level_vars[i]; - while !local.ref_.is_empty() { - let symbol = &self.symbols[local.ref_.inner_index() as usize]; - if !symbol.has_link() { - break; - } - local.ref_ = symbol.link.get(); + if !local.ref_.is_empty() { + local.ref_ = js_ast::symbol::follow_symbols(&self.symbols, local.ref_); } self.relocated_top_level_vars[i] = local; let Some(ref_) = local.ref_.to_nullable() else { @@ -6913,17 +6902,9 @@ impl<'a, const TYPESCRIPT: bool, const SCAN_ONLY: bool> P<'a, TYPESCRIPT, SCAN_O stmts_inside_closure: &'a mut [Stmt], all_values_are_pure: bool, ) -> Result<(), crate::Error> { - let mut name_ref = original_name_ref; - // Follow the link chain in case symbols were merged - let mut symbol = &self.symbols[name_ref.inner_index() as usize]; - while symbol.has_link() { - let link = symbol.link.get(); - name_ref = link; - symbol = &self.symbols[name_ref.inner_index() as usize]; - } - let symbol_kind = symbol.kind; - let _ = symbol; + let name_ref = js_ast::symbol::follow_symbols(&self.symbols, original_name_ref); + let symbol_kind = self.symbols[name_ref.inner_index() as usize].kind; let arena = self.arena; // Make sure to only emit a variable once for a given namespace, since there @@ -9263,12 +9244,7 @@ impl<'a, const TYPESCRIPT: bool, const SCAN_ONLY: bool> P<'a, TYPESCRIPT, SCAN_O // If this symbol was merged, use the symbol at the end of the // linked list in the map. This is the case for multiple "var" // declarations with the same name, for example. - let mut r#ref = input; - let mut symbol_ref = &ctx.symbols[r#ref.inner_index() as usize]; - while symbol_ref.has_link() { - r#ref = symbol_ref.link.get(); - symbol_ref = &ctx.symbols[r#ref.inner_index() as usize]; - } + let r#ref = js_ast::symbol::follow_symbols(ctx.symbols, input); ctx.top_level .entry(r#ref) diff --git a/src/js_parser/scan/scan_imports.rs b/src/js_parser/scan/scan_imports.rs index a368c81b476d..0517247434ee 100644 --- a/src/js_parser/scan/scan_imports.rs +++ b/src/js_parser/scan/scan_imports.rs @@ -327,18 +327,11 @@ impl<'a> ImportScanner<'a> { // member, while generated symbols (e.g. JSX runtime imports) // link the other way. let symbol = &p.symbols[name_ref.inner_index() as usize]; - let mut link = symbol.link.get(); - if !link.is_valid() { + if !symbol.has_link() { continue; } // Follow the chain of replacements to the live symbol. - loop { - let next = p.symbols[link.inner_index() as usize].link.get(); - if !next.is_valid() { - break; - } - link = next; - } + let link = js_ast::symbol::follow_symbols(&p.symbols, name_ref); // SAFETY: arena-owned slice valid for 'p. let name = symbol.original_name.slice(); let member = p diff --git a/test/js/bun/transpiler/transpiler-redeclared-symbol-chain-hang.test.ts b/test/js/bun/transpiler/transpiler-redeclared-symbol-chain-hang.test.ts new file mode 100644 index 000000000000..a8a65fb6bf6f --- /dev/null +++ b/test/js/bun/transpiler/transpiler-redeclared-symbol-chain-hang.test.ts @@ -0,0 +1,81 @@ +// bun-fuzz: every re-declaration of a name that merges with the previous one +// (`enum E {} enum E {}`, top-level `var x; var x;`) links the old symbol to +// the new one, so n re-declarations form a link chain of length n. The parser +// walked that chain from the start for every declaration without shortening +// it, which made n merged `enum` blocks (transpile) and n top-level `var` +// re-declarations (bundle) O(n^2): 720 KB of `enum E{…}` blocks took 14 s. +// +// Each case is timed against the same number of distinctly-named declarations, +// which does the same work minus the chain, so the check does not depend on +// how fast the machine is. +import { expect, test } from "bun:test"; +import { tempDir } from "harness"; +import { join } from "node:path"; + +/** + * Best-of timing. Repeats only while a run is cheap, so a release build takes + * several samples and a debug build takes one. + */ +async function bench(run: () => T | Promise): Promise<{ ms: number; result: T }> { + let ms = Infinity; + let spent = 0; + let result!: T; + for (let i = 0; i < 5 && spent < 400; i++) { + const start = performance.now(); + result = await run(); + const elapsed = performance.now() - start; + ms = Math.min(ms, elapsed); + spent += elapsed; + } + return { ms, result }; +} + +/** + * `prefix + 0 + suffix + prefix + 1 + suffix + …`. join() formats the numbers + * natively; building 16k strings in a JS loop takes half a second in a debug + * build. + */ +function numbered(n: number, prefix: string, suffix: string): string { + return prefix + [...Array(n).keys()].join(suffix + prefix) + suffix; +} + +test("merged enum declarations transpile in linear time", async () => { + const n = 16_384; + const merged = Buffer.alloc(n * 9, "enum E{}\n").toString(); + const distinct = numbered(n, "enum E", "{}\n"); + const transpiler = new Bun.Transpiler({ loader: "ts" }); + transpiler.transformSync("enum Warmup {}"); + + const baseline = await bench(() => transpiler.transformSync(distinct)); + const { ms, result: out } = await bench(() => transpiler.transformSync(merged)); + + // One `var E;` for the whole chain, then one closure per block. + expect(out).toStartWith("var E;\n((E) => {})(E ||= {});\n((E) => {})(E ||= {});\n"); + expect(out.indexOf("var E", 1)).toBe(-1); + expect(out.length).toBe("var E;\n".length + n * "((E) => {})(E ||= {});\n".length); + + // ~0.8x with the fix; 10x (debug) to 30x (release) before it. + expect(ms / baseline.ms).toBeLessThan(3); +}); + +test("top-level var re-declarations bundle in linear time", async () => { + const n = 16_384; + using dir = tempDir("redeclared-var-chain", { + "same.ts": Buffer.alloc(n * 11, "var x = 1;\n").toString() + "export { x };\n", + "distinct.ts": numbered(n, "var x", " = 1;\n") + "export { x0 };\n", + }); + const build = async (file: string) => { + const result = await Bun.build({ entrypoints: [join(String(dir), file)] }); + expect(result.success).toBeTrue(); + return result.outputs[0].text(); + }; + + const baseline = await bench(() => build("distinct.ts")); + const { ms, result: out } = await bench(() => build("same.ts")); + + expect(out).toContain("var x = 1;\nvar x = 1;\n"); + expect(out).toContain("export {\n x\n};"); + + // ~1x with the fix; 10x (debug) to 20x (release) before it. + expect(ms / baseline.ms).toBeLessThan(4); +});