Skip to content

Commit

Permalink
perf(semantic): use Atom<'a> for References (#3972)
Browse files Browse the repository at this point in the history
Relates to [this
issue](oxc-project/backlog#31) on the backlog.
  • Loading branch information
DonIsaac authored Jun 29, 2024
1 parent 0c81fbe commit 1eac3d2
Show file tree
Hide file tree
Showing 12 changed files with 88 additions and 88 deletions.
4 changes: 3 additions & 1 deletion crates/oxc_linter/src/rules/eslint/no_global_assign.rs
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,9 @@ impl Rule for NoGlobalAssign {
let reference = symbol_table.get_reference(reference_id);
if reference.is_write() {
let name = reference.name();
if !self.excludes.contains(name) && ctx.env_contains_var(name) {
// Vec::contains isn't working here, but this has the same
// effect and time complexity.
if !self.excludes.iter().any(|e| e == name) && ctx.env_contains_var(name) {
ctx.diagnostic(no_global_assign_diagnostic(name, reference.span()));
}
}
Expand Down
6 changes: 3 additions & 3 deletions crates/oxc_minifier/src/mangler/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,11 @@ use oxc_span::CompactStr;
type Slot = usize;

#[derive(Debug)]
pub struct Mangler {
symbol_table: SymbolTable,
pub struct Mangler<'a> {
symbol_table: SymbolTable<'a>,
}

impl Mangler {
impl<'a> Mangler<'a> {
pub fn get_symbol_name(&self, symbol_id: SymbolId) -> &str {
self.symbol_table.get_name(symbol_id)
}
Expand Down
20 changes: 10 additions & 10 deletions crates/oxc_semantic/src/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ use oxc_cfg::{
IterationInstructionKind, ReturnInstructionKind,
};
use oxc_diagnostics::OxcDiagnostic;
use oxc_span::{CompactStr, SourceType, Span};
use oxc_span::{Atom, CompactStr, SourceType, Span};
use oxc_syntax::{module_record::ModuleRecord, operator::AssignmentOperator};

use crate::{
Expand Down Expand Up @@ -63,8 +63,8 @@ pub struct SemanticBuilder<'a> {

// builders
pub nodes: AstNodes<'a>,
pub scope: ScopeTree,
pub symbols: SymbolTable,
pub scope: ScopeTree<'a>,
pub symbols: SymbolTable<'a>,

pub(crate) module_record: Arc<ModuleRecord>,

Expand Down Expand Up @@ -315,7 +315,7 @@ impl<'a> SemanticBuilder<'a> {

pub fn declare_reference(
&mut self,
reference: Reference,
reference: Reference<'a>,
add_unresolved_reference: bool,
) -> ReferenceId {
let reference_name = reference.name().clone();
Expand All @@ -327,7 +327,7 @@ impl<'a> SemanticBuilder<'a> {
reference_id,
);
} else {
self.resolve_reference_ids(reference_name.clone(), vec![reference_id]);
self.resolve_reference_ids(reference_name, vec![reference_id]);
}
reference_id
}
Expand Down Expand Up @@ -361,7 +361,7 @@ impl<'a> SemanticBuilder<'a> {
}
}

fn resolve_reference_ids(&mut self, name: CompactStr, reference_ids: Vec<ReferenceId>) {
fn resolve_reference_ids(&mut self, name: Atom<'a>, reference_ids: Vec<ReferenceId>) {
let parent_scope_id =
self.scope.get_parent_id(self.current_scope_id).unwrap_or(self.current_scope_id);

Expand Down Expand Up @@ -1884,9 +1884,9 @@ impl<'a> SemanticBuilder<'a> {
}
}

fn reference_identifier(&mut self, ident: &IdentifierReference) {
fn reference_identifier(&mut self, ident: &IdentifierReference<'a>) {
let flag = self.resolve_reference_usages();
let name = ident.name.to_compact_str();
let name = ident.name.clone();
let reference = Reference::new(ident.span, name, self.current_node_id, flag);
// `function foo({bar: identifier_reference}) {}`
// ^^^^^^^^^^^^^^^^^^^^ Parameter initializer must be resolved immediately
Expand All @@ -1905,7 +1905,7 @@ impl<'a> SemanticBuilder<'a> {
}
}

fn reference_jsx_identifier(&mut self, ident: &JSXIdentifier) {
fn reference_jsx_identifier(&mut self, ident: &JSXIdentifier<'a>) {
match self.nodes.parent_kind(self.current_node_id) {
Some(AstKind::JSXElementName(_)) => {
if !ident.name.chars().next().is_some_and(char::is_uppercase) {
Expand All @@ -1917,7 +1917,7 @@ impl<'a> SemanticBuilder<'a> {
}
let reference = Reference::new(
ident.span,
ident.name.to_compact_str(),
ident.name.clone(),
self.current_node_id,
ReferenceFlag::read(),
);
Expand Down
8 changes: 4 additions & 4 deletions crates/oxc_semantic/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -42,9 +42,9 @@ pub struct Semantic<'a> {

nodes: AstNodes<'a>,

scopes: ScopeTree,
scopes: ScopeTree<'a>,

symbols: SymbolTable,
symbols: SymbolTable<'a>,

classes: ClassTable,

Expand All @@ -60,7 +60,7 @@ pub struct Semantic<'a> {
}

impl<'a> Semantic<'a> {
pub fn into_symbol_table_and_scope_tree(self) -> (SymbolTable, ScopeTree) {
pub fn into_symbol_table_and_scope_tree(self) -> (SymbolTable<'a>, ScopeTree<'a>) {
(self.symbols, self.scopes)
}

Expand All @@ -84,7 +84,7 @@ impl<'a> Semantic<'a> {
&self.classes
}

pub fn scopes_mut(&mut self) -> &mut ScopeTree {
pub fn scopes_mut(&mut self) -> &mut ScopeTree<'a> {
&mut self.scopes
}

Expand Down
14 changes: 7 additions & 7 deletions crates/oxc_semantic/src/reference.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
// Silence erroneous warnings from Rust Analyser for `#[derive(Tsify)]`
#![allow(non_snake_case)]

use oxc_span::{CompactStr, Span};
use oxc_span::{Atom, Span};
pub use oxc_syntax::reference::{ReferenceFlag, ReferenceId};
#[cfg(feature = "serialize")]
use serde::Serialize;
Expand All @@ -13,25 +13,25 @@ use crate::{symbol::SymbolId, AstNodeId};
#[derive(Debug, Clone)]
#[cfg_attr(feature = "serialize", derive(Serialize, Tsify))]
#[cfg_attr(feature = "serialize", serde(rename_all = "camelCase"))]
pub struct Reference {
pub struct Reference<'a> {
span: Span,
/// The name of the identifier that was referred to
name: CompactStr,
name: Atom<'a>,
node_id: AstNodeId,
symbol_id: Option<SymbolId>,
/// Describes how this referenced is used by other AST nodes. References can
/// be reads, writes, or both.
flag: ReferenceFlag,
}

impl Reference {
pub fn new(span: Span, name: CompactStr, node_id: AstNodeId, flag: ReferenceFlag) -> Self {
impl<'a> Reference<'a> {
pub fn new(span: Span, name: Atom<'a>, node_id: AstNodeId, flag: ReferenceFlag) -> Self {
Self { span, name, node_id, symbol_id: None, flag }
}

pub fn new_with_symbol_id(
span: Span,
name: CompactStr,
name: Atom<'a>,
node_id: AstNodeId,
symbol_id: SymbolId,
flag: ReferenceFlag,
Expand All @@ -43,7 +43,7 @@ impl Reference {
self.span
}

pub fn name(&self) -> &CompactStr {
pub fn name(&self) -> &Atom<'a> {
&self.name
}

Expand Down
18 changes: 9 additions & 9 deletions crates/oxc_semantic/src/scope.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ use std::hash::BuildHasherDefault;

use indexmap::IndexMap;
use oxc_index::IndexVec;
use oxc_span::CompactStr;
use oxc_span::{Atom, CompactStr};
pub use oxc_syntax::scope::{ScopeFlags, ScopeId};
use rustc_hash::{FxHashMap, FxHasher};

Expand All @@ -11,13 +11,13 @@ use crate::{reference::ReferenceId, symbol::SymbolId, AstNodeId};
type FxIndexMap<K, V> = IndexMap<K, V, BuildHasherDefault<FxHasher>>;

type Bindings = FxIndexMap<CompactStr, SymbolId>;
type UnresolvedReferences = FxHashMap<CompactStr, Vec<ReferenceId>>;
type UnresolvedReferences<'a> = FxHashMap<Atom<'a>, Vec<ReferenceId>>;

/// Scope Tree
///
/// `SoA` (Struct of Arrays) for memory efficiency.
#[derive(Debug, Default)]
pub struct ScopeTree {
pub struct ScopeTree<'a> {
/// Maps a scope to the parent scope it belongs in
parent_ids: IndexVec<ScopeId, Option<ScopeId>>,

Expand All @@ -27,10 +27,10 @@ pub struct ScopeTree {
node_ids: FxHashMap<ScopeId, AstNodeId>,
flags: IndexVec<ScopeId, ScopeFlags>,
bindings: IndexVec<ScopeId, Bindings>,
unresolved_references: IndexVec<ScopeId, UnresolvedReferences>,
unresolved_references: IndexVec<ScopeId, UnresolvedReferences<'a>>,
}

impl ScopeTree {
impl<'a> ScopeTree<'a> {
pub fn len(&self) -> usize {
self.parent_ids.len()
}
Expand Down Expand Up @@ -141,7 +141,7 @@ impl ScopeTree {
self.get_binding(self.root_scope_id(), name)
}

pub fn add_root_unresolved_reference(&mut self, name: CompactStr, reference_id: ReferenceId) {
pub fn add_root_unresolved_reference(&mut self, name: Atom<'a>, reference_id: ReferenceId) {
self.add_unresolved_reference(self.root_scope_id(), name, reference_id);
}

Expand Down Expand Up @@ -208,7 +208,7 @@ impl ScopeTree {
pub(crate) fn add_unresolved_reference(
&mut self,
scope_id: ScopeId,
name: CompactStr,
name: Atom<'a>,
reference_id: ReferenceId,
) {
self.unresolved_references[scope_id].entry(name).or_default().push(reference_id);
Expand All @@ -217,7 +217,7 @@ impl ScopeTree {
pub(crate) fn extend_unresolved_reference(
&mut self,
scope_id: ScopeId,
name: CompactStr,
name: Atom<'a>,
reference_ids: Vec<ReferenceId>,
) {
self.unresolved_references[scope_id].entry(name).or_default().extend(reference_ids);
Expand All @@ -226,7 +226,7 @@ impl ScopeTree {
pub(crate) fn unresolved_references_mut(
&mut self,
scope_id: ScopeId,
) -> &mut UnresolvedReferences {
) -> &mut UnresolvedReferences<'a> {
&mut self.unresolved_references[scope_id]
}
}
12 changes: 6 additions & 6 deletions crates/oxc_semantic/src/symbol.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,19 +28,19 @@ export type IndexVec<I, T> = Array<T>;
/// `SoA` (Struct of Arrays) for memory efficiency.
#[derive(Debug, Default)]
#[cfg_attr(feature = "serialize", derive(Serialize, Tsify), serde(rename_all = "camelCase"))]
pub struct SymbolTable {
pub struct SymbolTable<'a> {
pub spans: IndexVec<SymbolId, Span>,
pub names: IndexVec<SymbolId, CompactStr>,
pub flags: IndexVec<SymbolId, SymbolFlags>,
pub scope_ids: IndexVec<SymbolId, ScopeId>,
/// Pointer to the AST Node where this symbol is declared
pub declarations: IndexVec<SymbolId, AstNodeId>,
pub resolved_references: IndexVec<SymbolId, Vec<ReferenceId>>,
pub references: IndexVec<ReferenceId, Reference>,
pub references: IndexVec<ReferenceId, Reference<'a>>,
pub redeclare_variables: IndexVec<SymbolId, Vec<Span>>,
}

impl SymbolTable {
impl<'a> SymbolTable<'a> {
pub fn len(&self) -> usize {
self.spans.len()
}
Expand Down Expand Up @@ -136,15 +136,15 @@ impl SymbolTable {
self.redeclare_variables[symbol_id].push(span);
}

pub fn create_reference(&mut self, reference: Reference) -> ReferenceId {
pub fn create_reference(&mut self, reference: Reference<'a>) -> ReferenceId {
self.references.push(reference)
}

pub fn get_reference(&self, reference_id: ReferenceId) -> &Reference {
pub fn get_reference(&self, reference_id: ReferenceId) -> &Reference<'a> {
&self.references[reference_id]
}

pub fn get_reference_mut(&mut self, reference_id: ReferenceId) -> &mut Reference {
pub fn get_reference_mut(&mut self, reference_id: ReferenceId) -> &mut Reference<'a> {
&mut self.references[reference_id]
}

Expand Down
10 changes: 8 additions & 2 deletions crates/oxc_semantic/tests/integration/util/symbol_tester.rs
Original file line number Diff line number Diff line change
Expand Up @@ -110,18 +110,24 @@ impl<'a> SymbolTester<'a> {
self
}

// Note: can't use `Reference::is_read()` due to error warning about overly
// generic FnMut impl.

#[must_use]
pub fn has_number_of_reads(self, ref_count: usize) -> Self {
self.has_number_of_references_where(ref_count, Reference::is_read)
#[allow(clippy::redundant_closure_for_method_calls)]
self.has_number_of_references_where(ref_count, |r| r.is_read())
}

#[must_use]
pub fn has_number_of_writes(self, ref_count: usize) -> Self {
self.has_number_of_references_where(ref_count, Reference::is_write)
#[allow(clippy::redundant_closure_for_method_calls)]
self.has_number_of_references_where(ref_count, |r| r.is_write())
}

#[must_use]
pub fn has_number_of_references(self, ref_count: usize) -> Self {
#[allow(clippy::redundant_closure_for_method_calls)]
self.has_number_of_references_where(ref_count, |_| true)
}

Expand Down
3 changes: 1 addition & 2 deletions crates/oxc_transformer/src/react/jsx.rs
Original file line number Diff line number Diff line change
Expand Up @@ -986,8 +986,7 @@ fn get_read_identifier_reference<'a>(
name: Atom<'a>,
ctx: &mut TraverseCtx<'a>,
) -> IdentifierReference<'a> {
let reference_id =
ctx.create_reference_in_current_scope(name.to_compact_str(), ReferenceFlag::Read);
let reference_id = ctx.create_reference_in_current_scope(name.clone(), ReferenceFlag::Read);
IdentifierReference::new_read(span, name, Some(reference_id))
}

Expand Down
7 changes: 2 additions & 5 deletions crates/oxc_transformer/src/typescript/annotations.rs
Original file line number Diff line number Diff line change
Expand Up @@ -540,11 +540,8 @@ struct Assignment<'a> {
impl<'a> Assignment<'a> {
// Creates `this.name = name`
fn create_this_property_assignment(&self, ctx: &mut TraverseCtx<'a>) -> Statement<'a> {
let reference_id = ctx.create_bound_reference(
self.name.to_compact_str(),
self.symbol_id,
ReferenceFlag::Read,
);
let reference_id =
ctx.create_bound_reference(self.name.clone(), self.symbol_id, ReferenceFlag::Read);
let id = IdentifierReference::new_read(self.span, self.name.clone(), Some(reference_id));

ctx.ast.expression_statement(
Expand Down
Loading

0 comments on commit 1eac3d2

Please sign in to comment.