Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

perf(mangler): optimize handling of collecting lived scope ids #8724

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 12 additions & 21 deletions crates/oxc_mangler/src/lib.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
use std::iter;
use std::ops::Deref;

use fixedbitset::FixedBitSet;
Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -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));
}
}

Expand Down
2 changes: 2 additions & 0 deletions crates/oxc_minifier/tests/mangler/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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}",
Expand Down
31 changes: 31 additions & 0 deletions crates/oxc_minifier/tests/mangler/snapshots/mangler.snap
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
Loading