From 43310e6480cb12fa0fe9711ac2bdb7f9ebbcad83 Mon Sep 17 00:00:00 2001
From: overlookmotel <557937+overlookmotel@users.noreply.github.com>
Date: Sun, 21 Jul 2024 12:42:19 +0000
Subject: [PATCH] perf(semantic): remove a branch from `add_scope` (#4384)

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.
---
 crates/oxc_semantic/src/builder.rs         |  7 +++--
 crates/oxc_semantic/src/scope.rs           | 30 +++++++++++++++++-----
 crates/oxc_traverse/src/context/scoping.rs |  2 +-
 3 files changed, 28 insertions(+), 11 deletions(-)

diff --git a/crates/oxc_semantic/src/builder.rs b/crates/oxc_semantic/src/builder.rs
index ec173b308022ee..4880ccaf265a6d 100644
--- a/crates/oxc_semantic/src/builder.rs
+++ b/crates/oxc_semantic/src/builder.rs
@@ -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);
@@ -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() {
@@ -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 {
diff --git a/crates/oxc_semantic/src/scope.rs b/crates/oxc_semantic/src/scope.rs
index 41b61b53e2c1b1..c5e9cc77649fa0 100644
--- a/crates/oxc_semantic/src/scope.rs
+++ b/crates/oxc_semantic/src/scope.rs
@@ -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,
@@ -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
     }
 
diff --git a/crates/oxc_traverse/src/context/scoping.rs b/crates/oxc_traverse/src/context/scoping.rs
index ace86b7b238791..26aa7e50213a23 100644
--- a/crates/oxc_traverse/src/context/scoping.rs
+++ b/crates/oxc_traverse/src/context/scoping.rs
@@ -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.