From e472ced05f1a803ceb0be1b9d198b23df9cfc0c3 Mon Sep 17 00:00:00 2001
From: Dunqing <29533304+Dunqing@users.noreply.github.com>
Date: Sun, 26 Jan 2025 10:53:48 +0000
Subject: [PATCH] perf(mangler): optimize handling of collecting lived scope
 ids (#8724)

Just some low-hanging fruit optimization.

I initially want to dedupe for reference scopes, so that we can avoid calling `scope_tree.ancestors(used_scope_id).take_while(|s_id| *s_id != scope_id)` for duplicate `used_scope_id` but the overhead of calling `unique` or `dedup` is over the improvement.
---
 crates/oxc_mangler/src/lib.rs                 | 33 +++++++------------
 crates/oxc_minifier/tests/mangler/mod.rs      |  2 ++
 .../tests/mangler/snapshots/mangler.snap      | 31 +++++++++++++++++
 3 files changed, 45 insertions(+), 21 deletions(-)

diff --git a/crates/oxc_mangler/src/lib.rs b/crates/oxc_mangler/src/lib.rs
index e20d61f30..f060d1fdc 100644
--- a/crates/oxc_mangler/src/lib.rs
+++ b/crates/oxc_mangler/src/lib.rs
@@ -1,4 +1,3 @@
-use std::iter;
 use std::ops::Deref;
 
 use fixedbitset::FixedBitSet;
@@ -193,10 +192,7 @@ impl Mangler {
         // Walk down the scope tree and assign a slot number for each symbol.
         // It is possible to do this in a loop over the symbol list,
         // but walking down the scope tree seems to generate a better code.
-        for scope_id in iter::once(scope_tree.root_scope_id())
-            .chain(scope_tree.iter_all_child_ids(scope_tree.root_scope_id()))
-        {
-            let bindings = scope_tree.get_bindings(scope_id);
+        for (scope_id, bindings) in scope_tree.iter_bindings() {
             if bindings.is_empty() {
                 continue;
             }
@@ -227,33 +223,28 @@ impl Mangler {
             tmp_bindings.clear();
             tmp_bindings.extend(bindings.values().copied());
             tmp_bindings.sort_unstable();
-            for (symbol_id, assigned_slot) in
+            for (&symbol_id, assigned_slot) in
                 tmp_bindings.iter().zip(reusable_slots.iter().copied())
             {
                 slots[symbol_id.index()] = assigned_slot;
 
+                // If the symbol is declared by `var`, then it can be hoisted to
+                // parent, so we need to include the scope where it is declared.
+                // (for cases like `function foo() { { var x; let y; } }`)
                 let declared_scope_id =
-                    ast_nodes.get_node(symbol_table.get_declaration(*symbol_id)).scope_id();
+                    ast_nodes.get_node(symbol_table.get_declaration(symbol_id)).scope_id();
 
                 // Calculate the scope ids that this symbol is alive in.
                 let lived_scope_ids = symbol_table
-                    .get_resolved_references(*symbol_id)
-                    .flat_map(|reference| {
-                        let used_scope_id = ast_nodes.get_node(reference.node_id()).scope_id();
+                    .get_resolved_references(symbol_id)
+                    .map(|reference| ast_nodes.get_node(reference.node_id()).scope_id())
+                    .chain([scope_id, declared_scope_id])
+                    .flat_map(|used_scope_id| {
                         scope_tree.ancestors(used_scope_id).take_while(|s_id| *s_id != scope_id)
-                    })
-                    // also include scopes that this symbol was declared (for cases like `function foo() { { var x; let y; } }`)
-                    .chain(
-                        scope_tree
-                            .ancestors(declared_scope_id)
-                            .take_while(|s_id| *s_id != scope_id),
-                    )
-                    .chain(iter::once(scope_id));
+                    });
 
                 // Since the slot is now assigned to this symbol, it is alive in all the scopes that this symbol is alive in.
-                for scope_id in lived_scope_ids {
-                    slot_liveness[assigned_slot].insert(scope_id.index());
-                }
+                slot_liveness[assigned_slot].extend(lived_scope_ids.map(oxc_index::Idx::index));
             }
         }
 
diff --git a/crates/oxc_minifier/tests/mangler/mod.rs b/crates/oxc_minifier/tests/mangler/mod.rs
index a99e7ccd6..d97947513 100644
--- a/crates/oxc_minifier/tests/mangler/mod.rs
+++ b/crates/oxc_minifier/tests/mangler/mod.rs
@@ -44,6 +44,8 @@ fn mangler() {
         "function _() { var x; let y; }", // x and y should have different names
         "function _() { { var x; var y; } }", // x and y should have different names
         "function _() { { var x; let y; } }", // x and y should have different names
+        "function _() { let a; { let b; { let c; { let d; var x; } } } }",
+        "function _() { let a; { let b; { let c; { console.log(a); let d; var x; } } } }",
     ];
     let top_level_cases = [
         "function foo(a) {a}",
diff --git a/crates/oxc_minifier/tests/mangler/snapshots/mangler.snap b/crates/oxc_minifier/tests/mangler/snapshots/mangler.snap
index e54ece1cb..3cc8414ab 100644
--- a/crates/oxc_minifier/tests/mangler/snapshots/mangler.snap
+++ b/crates/oxc_minifier/tests/mangler/snapshots/mangler.snap
@@ -181,6 +181,37 @@ function _() {
 	}
 }
 
+function _() { let a; { let b; { let c; { let d; var x; } } } }
+function _() {
+	let a;
+	{
+		let a;
+		{
+			let a;
+			{
+				let a;
+				var b;
+			}
+		}
+	}
+}
+
+function _() { let a; { let b; { let c; { console.log(a); let d; var x; } } } }
+function _() {
+	let a;
+	{
+		let c;
+		{
+			let c;
+			{
+				console.log(a);
+				let c;
+				var b;
+			}
+		}
+	}
+}
+
 function foo(a) {a}
 function a(a) {
 	a;