perf(semantic): remove span field from Reference (#4464)

Remove `span` field from `Reference`. Span can instead be got via looking up span on `AstKind` via `reference.node_id`. This shrinks `Reference` from 20 bytes to 12 bytes. It does make getting span of a `Reference` a bit slower, as there's an extra table lookup involved, but the only places this is needed is in linter, and almost always when generating a diagnostic (i.e. cold path).
oxc-project · Jul 26, 2024 · 348c1ad · 348c1ad
1 parent 6a9f4db
commit 348c1ad
Show file tree

Hide file tree

Showing 14 changed files with 36 additions and 28 deletions.
diff --git a/crates/oxc_linter/src/rules/eslint/no_class_assign.rs b/crates/oxc_linter/src/rules/eslint/no_class_assign.rs
@@ -42,7 +42,7 @@ impl Rule for NoClassAssign {
                     ctx.diagnostic(no_class_assign_diagnostic(
                         symbol_table.get_name(symbol_id),
                         symbol_table.get_span(symbol_id),
-                        reference.span(),
+                        ctx.semantic().reference_span(reference),
                     ));
                 }
             }

diff --git a/crates/oxc_linter/src/rules/eslint/no_const_assign.rs b/crates/oxc_linter/src/rules/eslint/no_const_assign.rs
@@ -41,7 +41,7 @@ impl Rule for NoConstAssign {
                     ctx.diagnostic(no_const_assign_diagnostic(
                         symbol_table.get_name(symbol_id),
                         symbol_table.get_span(symbol_id),
-                        reference.span(),
+                        ctx.semantic().reference_span(reference),
                     ));
                 }
             }

diff --git a/crates/oxc_linter/src/rules/eslint/no_ex_assign.rs b/crates/oxc_linter/src/rules/eslint/no_ex_assign.rs
@@ -41,7 +41,9 @@ impl Rule for NoExAssign {
         if symbol_table.get_flag(symbol_id).is_catch_variable() {
             for reference in symbol_table.get_resolved_references(symbol_id) {
                 if reference.is_write() {
-                    ctx.diagnostic(no_ex_assign_diagnostic(reference.span()));
+                    ctx.diagnostic(no_ex_assign_diagnostic(
+                        ctx.semantic().reference_span(reference),
+                    ));
                 }
             }
         }

diff --git a/crates/oxc_linter/src/rules/eslint/no_func_assign.rs b/crates/oxc_linter/src/rules/eslint/no_func_assign.rs
@@ -40,7 +40,7 @@ impl Rule for NoFuncAssign {
                 if reference.is_write() {
                     ctx.diagnostic(no_func_assign_diagnostic(
                         symbol_table.get_name(symbol_id),
-                        reference.span(),
+                        ctx.semantic().reference_span(reference),
                     ));
                 }
             }

diff --git a/crates/oxc_linter/src/rules/eslint/no_global_assign.rs b/crates/oxc_linter/src/rules/eslint/no_global_assign.rs
@@ -67,7 +67,10 @@ impl Rule for NoGlobalAssign {
                     if !self.excludes.contains(&CompactStr::from(name))
                         && ctx.env_contains_var(name)
                     {
-                        ctx.diagnostic(no_global_assign_diagnostic(name, reference.span()));
+                        ctx.diagnostic(no_global_assign_diagnostic(
+                            name,
+                            ctx.semantic().reference_span(reference),
+                        ));
                     }
                 }
             }

diff --git a/crates/oxc_linter/src/rules/eslint/no_import_assign.rs b/crates/oxc_linter/src/rules/eslint/no_import_assign.rs
@@ -74,7 +74,7 @@ impl Rule for NoImportAssign {
                             || matches!(parent_parent_kind, AstKind::ChainExpression(_) if ctx.nodes().parent_kind(parent_parent_node.id()).is_some_and(is_unary_expression_with_delete_operator))
                         {
                             if let Some((span, _)) = expr.static_property_info() {
-                                if span != reference.span() {
+                                if span != ctx.semantic().reference_span(reference) {
                                     return ctx
                                         .diagnostic(no_import_assign_diagnostic(expr.span()));
                                 }
@@ -87,7 +87,9 @@ impl Rule for NoImportAssign {
                     || (is_namespace_specifier
                         && is_argument_of_well_known_mutation_function(reference.node_id(), ctx))
                 {
-                    ctx.diagnostic(no_import_assign_diagnostic(reference.span()));
+                    ctx.diagnostic(no_import_assign_diagnostic(
+                        ctx.semantic().reference_span(reference),
+                    ));
                 }
             }
         }

diff --git a/crates/oxc_linter/src/rules/eslint/no_undef.rs b/crates/oxc_linter/src/rules/eslint/no_undef.rs
@@ -1,7 +1,7 @@
 use oxc_ast::AstKind;
 use oxc_diagnostics::OxcDiagnostic;
 use oxc_macros::declare_oxc_lint;
-use oxc_span::Span;
+use oxc_span::{GetSpan, Span};
 use oxc_syntax::operator::UnaryOperator;
 
 use crate::{context::LintContext, rule::Rule, AstNode};
@@ -67,7 +67,7 @@ impl Rule for NoUndef {
                     continue;
                 }
 
-                ctx.diagnostic(no_undef_diagnostic(name, reference.span()));
+                ctx.diagnostic(no_undef_diagnostic(name, node.kind().span()));
             }
         }
     }

diff --git a/crates/oxc_linter/src/rules/jest/no_jasmine_globals.rs b/crates/oxc_linter/src/rules/jest/no_jasmine_globals.rs
@@ -54,7 +54,11 @@ impl Rule for NoJasmineGlobals {
             for &(reference_id, _) in reference_ids {
                 let reference = symbol_table.get_reference(reference_id);
                 if let Some((error, help)) = get_non_jasmine_property_messages(name) {
-                    ctx.diagnostic(no_jasmine_globals_diagnostic(error, help, reference.span()));
+                    ctx.diagnostic(no_jasmine_globals_diagnostic(
+                        error,
+                        help,
+                        ctx.semantic().reference_span(reference),
+                    ));
                 }
             }
         }

diff --git a/crates/oxc_linter/src/rules/nextjs/no_duplicate_head.rs b/crates/oxc_linter/src/rules/nextjs/no_duplicate_head.rs
@@ -1,7 +1,6 @@
 use oxc_ast::AstKind;
 use oxc_diagnostics::{LabeledSpan, OxcDiagnostic};
 use oxc_macros::declare_oxc_lint;
-use oxc_semantic::Reference;
 
 use crate::{context::LintContext, rule::Rule};
 
@@ -65,7 +64,7 @@ impl Rule for NoDuplicateHead {
                 let kind = nodes.ancestors(r.node_id()).nth(2).map(|node_id| nodes.kind(node_id));
                 matches!(kind, Some(AstKind::JSXOpeningElement(_)))
             })
-            .map(Reference::span)
+            .map(|reference| ctx.semantic().reference_span(reference))
             .map(LabeledSpan::underline)
             .collect::<Vec<_>>();
 

diff --git a/crates/oxc_linter/src/rules/typescript/no_non_null_asserted_nullish_coalescing.rs b/crates/oxc_linter/src/rules/typescript/no_non_null_asserted_nullish_coalescing.rs
@@ -60,7 +60,7 @@ fn has_assignment_before_node(
     let symbol_table = ctx.semantic().symbols();
 
     for reference in symbol_table.get_resolved_references(symbol_id) {
-        if reference.is_write() && reference.span().end < parent_span_end {
+        if reference.is_write() && ctx.semantic().reference_span(reference).end < parent_span_end {
             return true;
         }
     }

diff --git a/crates/oxc_semantic/src/builder.rs b/crates/oxc_semantic/src/builder.rs
@@ -1932,7 +1932,7 @@ impl<'a> SemanticBuilder<'a> {
 
     fn reference_identifier(&mut self, ident: &IdentifierReference<'a>) {
         let flag = self.resolve_reference_usages();
-        let reference = Reference::new(ident.span, self.current_node_id, flag);
+        let reference = Reference::new(self.current_node_id, flag);
         let reference_id = self.declare_reference(ident.name.clone(), reference);
         ident.reference_id.set(Some(reference_id));
     }
@@ -1956,7 +1956,7 @@ impl<'a> SemanticBuilder<'a> {
             Some(AstKind::JSXMemberExpressionObject(_)) => {}
             _ => return,
         }
-        let reference = Reference::new(ident.span, self.current_node_id, ReferenceFlag::read());
+        let reference = Reference::new(self.current_node_id, ReferenceFlag::read());
         self.declare_reference(ident.name.clone(), reference);
     }
 

diff --git a/crates/oxc_semantic/src/lib.rs b/crates/oxc_semantic/src/lib.rs
@@ -23,7 +23,7 @@ pub use jsdoc::{JSDoc, JSDocFinder, JSDocTag};
 pub use node::{AstNode, AstNodeId, AstNodes};
 use oxc_ast::{ast::IdentifierReference, AstKind, Trivias};
 use oxc_cfg::ControlFlowGraph;
-use oxc_span::SourceType;
+use oxc_span::{GetSpan, SourceType, Span};
 pub use oxc_syntax::{
     module_record::ModuleRecord,
     scope::{ScopeFlags, ScopeId},
@@ -148,6 +148,11 @@ impl<'a> Semantic<'a> {
             _ => unreachable!(),
         }
     }
+
+    pub fn reference_span(&self, reference: &Reference) -> Span {
+        let node = self.nodes.get_node(reference.node_id());
+        node.kind().span()
+    }
 }
 
 #[cfg(test)]

diff --git a/crates/oxc_semantic/src/reference.rs b/crates/oxc_semantic/src/reference.rs
@@ -1,7 +1,6 @@
 // Silence erroneous warnings from Rust Analyser for `#[derive(Tsify)]`
 #![allow(non_snake_case)]
 
-use oxc_span::Span;
 pub use oxc_syntax::reference::{ReferenceFlag, ReferenceId};
 #[cfg(feature = "serialize")]
 use serde::Serialize;
@@ -14,7 +13,6 @@ use crate::{symbol::SymbolId, AstNodeId};
 #[cfg_attr(feature = "serialize", derive(Serialize, Tsify))]
 #[cfg_attr(feature = "serialize", serde(rename_all = "camelCase"))]
 pub struct Reference {
-    span: Span,
     node_id: AstNodeId,
     symbol_id: Option<SymbolId>,
     /// Describes how this referenced is used by other AST nodes. References can
@@ -23,21 +21,16 @@ pub struct Reference {
 }
 
 impl Reference {
-    pub fn new(span: Span, node_id: AstNodeId, flag: ReferenceFlag) -> Self {
-        Self { span, node_id, symbol_id: None, flag }
+    pub fn new(node_id: AstNodeId, flag: ReferenceFlag) -> Self {
+        Self { node_id, symbol_id: None, flag }
     }
 
     pub fn new_with_symbol_id(
-        span: Span,
         node_id: AstNodeId,
         symbol_id: SymbolId,
         flag: ReferenceFlag,
     ) -> Self {
-        Self { span, node_id, symbol_id: Some(symbol_id), flag }
-    }
-
-    pub fn span(&self) -> Span {
-        self.span
+        Self { node_id, symbol_id: Some(symbol_id), flag }
     }
 
     pub fn node_id(&self) -> AstNodeId {

diff --git a/crates/oxc_traverse/src/context/scoping.rs b/crates/oxc_traverse/src/context/scoping.rs
@@ -267,7 +267,7 @@ impl TraverseScoping {
         symbol_id: SymbolId,
         flag: ReferenceFlag,
     ) -> ReferenceId {
-        let reference = Reference::new_with_symbol_id(SPAN, AstNodeId::DUMMY, symbol_id, flag);
+        let reference = Reference::new_with_symbol_id(AstNodeId::DUMMY, symbol_id, flag);
         let reference_id = self.symbols.create_reference(reference);
         self.symbols.resolved_references[symbol_id].push(reference_id);
         reference_id
@@ -296,7 +296,7 @@ impl TraverseScoping {
         name: CompactStr,
         flag: ReferenceFlag,
     ) -> ReferenceId {
-        let reference = Reference::new(SPAN, AstNodeId::DUMMY, flag);
+        let reference = Reference::new(AstNodeId::DUMMY, flag);
         let reference_id = self.symbols.create_reference(reference);
         self.scopes.add_root_unresolved_reference(name, (reference_id, flag));
         reference_id