Skip to content

Commit

Permalink
perf: replace some CompactStr usages with Cows (#4377)
Browse files Browse the repository at this point in the history
Reduce memory allocations in semantic and linter by using `Cow<'a, str>` over `CompactStr`
  • Loading branch information
DonIsaac committed Jul 20, 2024
1 parent 7a3e925 commit a207923
Show file tree
Hide file tree
Showing 11 changed files with 96 additions and 55 deletions.
30 changes: 15 additions & 15 deletions crates/oxc_ast/src/ast_impl/js.rs
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
use crate::ast::*;

use std::{cell::Cell, fmt, hash::Hash};
use std::{borrow::Cow, cell::Cell, fmt, hash::Hash};

use oxc_allocator::{Box, FromIn, Vec};
use oxc_span::{Atom, CompactStr, GetSpan, SourceType, Span};
use oxc_span::{Atom, GetSpan, SourceType, Span};
use oxc_syntax::{
operator::UnaryOperator,
reference::{ReferenceFlag, ReferenceId},
Expand Down Expand Up @@ -332,20 +332,20 @@ impl<'a> ObjectExpression<'a> {
}

impl<'a> PropertyKey<'a> {
pub fn static_name(&self) -> Option<CompactStr> {
pub fn static_name(&self) -> Option<Cow<'a, str>> {
match self {
Self::StaticIdentifier(ident) => Some(ident.name.to_compact_str()),
Self::StringLiteral(lit) => Some(lit.value.to_compact_str()),
Self::RegExpLiteral(lit) => Some(lit.regex.to_string().into()),
Self::NumericLiteral(lit) => Some(lit.value.to_string().into()),
Self::BigIntLiteral(lit) => Some(lit.raw.to_compact_str()),
Self::NullLiteral(_) => Some("null".into()),
Self::StaticIdentifier(ident) => Some(Cow::Borrowed(ident.name.as_str())),
Self::StringLiteral(lit) => Some(Cow::Borrowed(lit.value.as_str())),
Self::RegExpLiteral(lit) => Some(Cow::Owned(lit.regex.to_string())),
Self::NumericLiteral(lit) => Some(Cow::Owned(lit.value.to_string())),
Self::BigIntLiteral(lit) => Some(Cow::Borrowed(lit.raw.as_str())),
Self::NullLiteral(_) => Some(Cow::Borrowed("null")),
Self::TemplateLiteral(lit) => lit
.expressions
.is_empty()
.then(|| lit.quasi())
.flatten()
.map(|quasi| quasi.to_compact_str()),
.map(std::convert::Into::into),
_ => None,
}
}
Expand All @@ -369,9 +369,9 @@ impl<'a> PropertyKey<'a> {
}
}

pub fn name(&self) -> Option<CompactStr> {
pub fn name(&self) -> Option<Cow<'a, str>> {
if self.is_private_identifier() {
self.private_name().map(|name| name.to_compact_str())
self.private_name().map(|name| Cow::Borrowed(name.as_str()))
} else {
self.static_name()
}
Expand Down Expand Up @@ -1237,7 +1237,7 @@ impl<'a> ClassElement<'a> {
}
}

pub fn static_name(&self) -> Option<CompactStr> {
pub fn static_name(&self) -> Option<Cow<'a, str>> {
match self {
Self::TSIndexSignature(_) | Self::StaticBlock(_) => None,
Self::MethodDefinition(def) => def.key.static_name(),
Expand Down Expand Up @@ -1424,8 +1424,8 @@ impl<'a> ImportDeclarationSpecifier<'a> {
ImportDeclarationSpecifier::ImportDefaultSpecifier(specifier) => &specifier.local,
}
}
pub fn name(&self) -> CompactStr {
self.local().name.to_compact_str()
pub fn name(&self) -> Cow<'a, str> {
Cow::Borrowed(self.local().name.as_str())
}
}

Expand Down
2 changes: 1 addition & 1 deletion crates/oxc_isolated_declarations/src/module.rs
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ impl<'a> IsolatedDeclarations<'a> {
self.scope.has_reference(&specifier.local.name)
}
ImportDeclarationSpecifier::ImportNamespaceSpecifier(_) => {
self.scope.has_reference(specifier.name().as_str())
self.scope.has_reference(&specifier.name())
}
});
if specifiers.is_empty() {
Expand Down
11 changes: 5 additions & 6 deletions crates/oxc_linter/src/rules/eslint/sort_imports.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
use std::{
borrow::Cow,
fmt::{Display, Write},
str::FromStr,
};
Expand All @@ -10,7 +11,7 @@ use oxc_ast::{
};
use oxc_diagnostics::OxcDiagnostic;
use oxc_macros::declare_oxc_lint;
use oxc_span::{CompactStr, Span};
use oxc_span::Span;

use crate::{context::LintContext, rule::Rule};

Expand Down Expand Up @@ -208,9 +209,9 @@ impl SortImports {

if self.ignore_case {
current_local_member_name =
current_local_member_name.map(|name| name.to_lowercase()).map(CompactStr::from);
current_local_member_name.map(|name| name.to_lowercase().into());
previous_local_member_name =
previous_local_member_name.map(|name| name.to_lowercase()).map(CompactStr::from);
previous_local_member_name.map(|name| name.to_lowercase().into());
}

// "memberSyntaxSortOrder": ["none", "all", "multiple", "single"]
Expand Down Expand Up @@ -240,8 +241,6 @@ impl SortImports {
if let Some((current_name, previous_name)) =
current_local_member_name.zip(previous_local_member_name)
{
let current_name = current_name.as_str();
let previous_name = previous_name.as_str();
if current_name < previous_name {
ctx.diagnostic(sort_imports_alphabetically_diagnostic(current.span));
}
Expand Down Expand Up @@ -443,7 +442,7 @@ impl Display for ImportKind {
}
}

fn get_first_local_member_name(decl: &ImportDeclaration) -> Option<CompactStr> {
fn get_first_local_member_name<'a>(decl: &ImportDeclaration<'a>) -> Option<Cow<'a, str>> {
let specifiers = decl.specifiers.as_ref()?;
specifiers.first().map(ImportDeclarationSpecifier::name)
}
Expand Down
2 changes: 1 addition & 1 deletion crates/oxc_linter/src/rules/react/no_danger.rs
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ impl Rule for NoDanger {
for prop in &obj_expr.properties {
if let ObjectPropertyKind::ObjectProperty(obj_prop) = prop {
if let Some(prop_name) = obj_prop.key.static_name() {
if prop_name.as_str() == "dangerouslySetInnerHTML" {
if prop_name == "dangerouslySetInnerHTML" {
ctx.diagnostic(no_danger_diagnostic(obj_prop.key.span()));
}
}
Expand Down
18 changes: 10 additions & 8 deletions crates/oxc_linter/src/rules/react/rules_of_hooks.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
use std::borrow::Cow;

use oxc_ast::{
ast::{ArrowFunctionExpression, Function},
AstKind,
Expand All @@ -8,7 +10,6 @@ use oxc_cfg::{
};
use oxc_macros::declare_oxc_lint;
use oxc_semantic::{AstNodeId, AstNodes};
use oxc_span::CompactStr;
use oxc_syntax::operator::AssignmentOperator;

use crate::{
Expand Down Expand Up @@ -191,7 +192,7 @@ impl Rule for RulesOfHooks {
// useState(0);
// }
// }
if ident.is_some_and(|name| !is_react_component_or_hook_name(name.as_str()))
if ident.is_some_and(|name| !is_react_component_or_hook_name(&name))
|| is_export_default(nodes, parent_func.id())
{
return ctx.diagnostic(diagnostics::function_error(
Expand Down Expand Up @@ -352,7 +353,9 @@ fn is_somewhere_inside_component_or_hook(nodes: &AstNodes, node_id: AstNodeId) -
(
node.id(),
match node.kind() {
AstKind::Function(func) => func.id.as_ref().map(|it| it.name.to_compact_str()),
AstKind::Function(func) => {
func.id.as_ref().map(|it| Cow::Borrowed(it.name.as_str()))
}
AstKind::ArrowFunctionExpression(_) => {
get_declaration_identifier(nodes, node.id())
}
Expand All @@ -362,21 +365,20 @@ fn is_somewhere_inside_component_or_hook(nodes: &AstNodes, node_id: AstNodeId) -
})
.any(|(id, ident)| {
ident.is_some_and(|name| {
is_react_component_or_hook_name(name.as_str())
|| is_memo_or_forward_ref_callback(nodes, id)
is_react_component_or_hook_name(&name) || is_memo_or_forward_ref_callback(nodes, id)
})
})
}

fn get_declaration_identifier<'a>(
nodes: &'a AstNodes<'a>,
node_id: AstNodeId,
) -> Option<CompactStr> {
) -> Option<Cow<'a, str>> {
nodes.ancestors(node_id).map(|id| nodes.kind(id)).find_map(|kind| {
match kind {
// const useHook = () => {};
AstKind::VariableDeclaration(decl) if decl.declarations.len() == 1 => {
decl.declarations[0].id.get_identifier().map(|id| id.to_compact_str())
decl.declarations[0].id.get_identifier().map(|id| Cow::Borrowed(id.as_str()))
}
// useHook = () => {};
AstKind::AssignmentExpression(expr)
Expand All @@ -387,7 +389,7 @@ fn get_declaration_identifier<'a>(
// const {useHook = () => {}} = {};
// ({useHook = () => {}} = {});
AstKind::AssignmentPattern(patt) => {
patt.left.get_identifier().map(|id| id.to_compact_str())
patt.left.get_identifier().map(|id| Cow::Borrowed(id.as_str()))
}
// { useHook: () => {} }
// { useHook() {} }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,7 @@ impl GetMethod for ClassElement<'_> {
fn get_method(&self) -> Option<Method> {
match self {
ClassElement::MethodDefinition(def) => def.key.static_name().map(|name| Method {
name,
name: name.into(),
r#static: def.r#static,
call_signature: false,
kind: get_kind_from_key(&def.key),
Expand All @@ -142,7 +142,7 @@ impl GetMethod for TSSignature<'_> {
fn get_method(&self) -> Option<Method> {
match self {
TSSignature::TSMethodSignature(sig) => sig.key.static_name().map(|name| Method {
name: name.clone(),
name: name.into(),
r#static: false,
call_signature: false,
kind: get_kind_from_key(&sig.key),
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use std::{error::Error, ops::Deref};
use std::{borrow::Cow, error::Error, ops::Deref};

use itertools::Itertools;
use oxc_ast::{
Expand All @@ -11,7 +11,7 @@ use oxc_ast::{
use oxc_diagnostics::OxcDiagnostic;
use oxc_macros::declare_oxc_lint;
use oxc_semantic::{Reference, SymbolId};
use oxc_span::{CompactStr, GetSpan, Span};
use oxc_span::{GetSpan, Span};

use crate::{
context::LintContext,
Expand Down Expand Up @@ -231,7 +231,7 @@ impl Rule for ConsistentTypeImports {

// ['foo', 'bar', 'baz' ] => "foo, bar, and baz".
let type_imports = format_word_list(&type_names);
let type_names = type_names.iter().map(CompactStr::as_str).collect::<Vec<_>>();
let type_names = type_names.iter().map(std::convert::AsRef::as_ref).collect::<Vec<_>>();

let fixer_fn = |fixer: RuleFixer<'_, 'a>| {
let fix_options = FixOptions {
Expand Down Expand Up @@ -272,21 +272,21 @@ impl Rule for ConsistentTypeImports {
// the `and` clause inserted before the last item.
//
// Example: ['foo', 'bar', 'baz' ] returns the string "foo, bar, and baz".
fn format_word_list(words: &[CompactStr]) -> String {
fn format_word_list<'a>(words: &[Cow<'a, str>]) -> Cow<'a, str> {
match words.len() {
0 => String::new(),
1 => words[0].to_string(),
2 => format!("{} and {}", words[0], words[1]),
0 => Cow::Borrowed(""),
1 => words[0].clone(),
2 => Cow::Owned(format!("{} and {}", words[0], words[1])),
_ => {
let mut result = String::new();
let mut result = String::with_capacity(words.len() * 2);
for (i, word) in words.iter().enumerate() {
if i == words.len() - 1 {
result.push_str(&format!("and {word}"));
} else {
result.push_str(&format!("{word}, "));
}
}
result
Cow::Owned(result)
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -316,19 +316,19 @@ impl ExplicitFunctionReturnType {
let Some(name) = def.key.name() else { return false };
def.key.is_identifier()
&& !def.computed
&& self.allowed_names.contains(name.as_str())
&& self.allowed_names.contains(name.as_ref())
}
AstKind::PropertyDefinition(def) => {
let Some(name) = def.key.name() else { return false };
def.key.is_identifier()
&& !def.computed
&& self.allowed_names.contains(name.as_str())
&& self.allowed_names.contains(name.as_ref())
}
AstKind::ObjectProperty(prop) => {
let Some(name) = prop.key.name() else { return false };
prop.key.is_identifier()
&& !prop.computed
&& self.allowed_names.contains(name.as_str())
&& self.allowed_names.contains(name.as_ref())
}
_ => false,
}
Expand Down
12 changes: 4 additions & 8 deletions crates/oxc_semantic/src/class/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ impl ClassTableBuilder {
self.classes.add_element(
class_id,
Element::new(
name,
name.into(),
property.key.span(),
property.r#static,
is_private,
Expand All @@ -83,7 +83,7 @@ impl ClassTableBuilder {
self.classes.add_element(
class_id,
Element::new(
name,
name.into(),
property.key.span(),
property.r#static,
is_private,
Expand Down Expand Up @@ -124,18 +124,14 @@ impl ClassTableBuilder {
return;
}
let is_private = method.key.is_private_identifier();
let name = if is_private {
method.key.private_name().map(|name| name.to_compact_str())
} else {
method.key.static_name()
};
let name = method.key.name();

if let Some(name) = name {
if let Some(class_id) = self.current_class_id {
self.classes.add_element(
class_id,
Element::new(
name,
name.into(),
method.key.span(),
method.r#static,
is_private,
Expand Down
Loading

0 comments on commit a207923

Please sign in to comment.