Skip to content

Commit

Permalink
perf(semantic): remove a branch from add_scope (#4384)
Browse files Browse the repository at this point in the history
Similar to #4361.

`ScopeTree::add_scope` had a branch specifically to handle `Program`. Remove that by inlining the special logic for `Program` into `visit_program`.

Probably won't have much effect on benchmarks as the branch is easy to predict, but still removes a few instructions and makes `add_scope` easier for compiler to inline.
  • Loading branch information
overlookmotel committed Jul 21, 2024
1 parent 97c7678 commit e70c67b
Show file tree
Hide file tree
Showing 3 changed files with 28 additions and 11 deletions.
7 changes: 3 additions & 4 deletions crates/oxc_semantic/src/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -189,7 +189,7 @@ impl<'a> SemanticBuilder<'a> {
/// # Panics
pub fn build(mut self, program: &Program<'a>) -> SemanticBuilderReturn<'a> {
if self.source_type.is_typescript_definition() {
let scope_id = self.scope.add_scope(None, AstNodeId::DUMMY, ScopeFlags::Top);
let scope_id = self.scope.add_root_scope(AstNodeId::DUMMY, ScopeFlags::Top);
program.scope_id.set(Some(scope_id));
} else {
self.visit_program(program);
Expand Down Expand Up @@ -471,8 +471,7 @@ impl<'a> Visit<'a> for SemanticBuilder<'a> {
};
flags = self.scope.get_new_scope_flags(flags, parent_scope_id);

self.current_scope_id =
self.scope.add_scope(Some(parent_scope_id), self.current_node_id, flags);
self.current_scope_id = self.scope.add_scope(parent_scope_id, self.current_node_id, flags);
scope_id.set(Some(self.current_scope_id));

if self.scope.get_flags(parent_scope_id).is_catch_clause() {
Expand Down Expand Up @@ -553,7 +552,7 @@ impl<'a> Visit<'a> for SemanticBuilder<'a> {
if program.is_strict() {
flags |= ScopeFlags::StrictMode;
}
self.current_scope_id = self.scope.add_scope(None, self.current_node_id, flags);
self.current_scope_id = self.scope.add_root_scope(self.current_node_id, flags);
program.scope_id.set(Some(self.current_scope_id));

if let Some(hashbang) = &program.hashbang {
Expand Down
30 changes: 24 additions & 6 deletions crates/oxc_semantic/src/scope.rs
Original file line number Diff line number Diff line change
Expand Up @@ -191,7 +191,31 @@ impl ScopeTree {
&mut self.bindings[scope_id]
}

/// Create scope.
/// For root (`Program`) scope, use `add_root_scope`.
pub fn add_scope(
&mut self,
parent_id: ScopeId,
node_id: AstNodeId,
flags: ScopeFlags,
) -> ScopeId {
let scope_id = self.add_scope_impl(Some(parent_id), node_id, flags);

// Set this scope as child of parent scope
self.child_ids[parent_id].push(scope_id);

scope_id
}

/// Create root (`Program`) scope.
pub fn add_root_scope(&mut self, node_id: AstNodeId, flags: ScopeFlags) -> ScopeId {
self.add_scope_impl(None, node_id, flags)
}

// `#[inline]` because almost always called from `add_scope` and want to avoid
// overhead of a function call there.
#[inline]
fn add_scope_impl(
&mut self,
parent_id: Option<ScopeId>,
node_id: AstNodeId,
Expand All @@ -202,12 +226,6 @@ impl ScopeTree {
self.flags.push(flags);
self.bindings.push(Bindings::default());
self.node_ids.push(node_id);

// Set this scope as child of parent scope.
if let Some(parent_id) = parent_id {
self.child_ids[parent_id].push(scope_id);
}

scope_id
}

Expand Down
2 changes: 1 addition & 1 deletion crates/oxc_traverse/src/context/scoping.rs
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@ impl TraverseScoping {
/// `flags` provided are amended to inherit from parent scope's flags.
pub fn create_scope_child_of_current(&mut self, flags: ScopeFlags) -> ScopeId {
let flags = self.scopes.get_new_scope_flags(flags, self.current_scope_id);
self.scopes.add_scope(Some(self.current_scope_id), AstNodeId::DUMMY, flags)
self.scopes.add_scope(self.current_scope_id, AstNodeId::DUMMY, flags)
}

/// Insert a scope into scope tree below a statement.
Expand Down

0 comments on commit e70c67b

Please sign in to comment.