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(semantic): use Atom<'a> for References #3972

Merged
merged 3 commits into from
Jun 29, 2024
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
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
Loading