perf: replace some CompactStr usages with Cows (#4377)

DonIsaac · DonIsaac · commit a207923af1b8 · 2024-07-20T19:19:55.000Z
Reduce memory allocations in semantic and linter by using `Cow&lt;'a, str&gt;` over `CompactStr`
diff --git a/crates/oxc_ast/src/ast_impl/js.rs b/crates/oxc_ast/src/ast_impl/js.rs
@@ -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},
@@ -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,
         }
     }
@@ -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()
         }
@@ -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(),
@@ -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())
     }
 }
 
diff --git a/crates/oxc_isolated_declarations/src/module.rs b/crates/oxc_isolated_declarations/src/module.rs
@@ -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() {
diff --git a/crates/oxc_linter/src/rules/eslint/sort_imports.rs b/crates/oxc_linter/src/rules/eslint/sort_imports.rs
@@ -1,4 +1,5 @@
 use std::{
+    borrow::Cow,
     fmt::{Display, Write},
     str::FromStr,
 };
@@ -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};
 
@@ -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"]
@@ -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));
                     }
@@ -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)
 }
diff --git a/crates/oxc_linter/src/rules/react/no_danger.rs b/crates/oxc_linter/src/rules/react/no_danger.rs
@@ -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()));
                             }
                         }
diff --git a/crates/oxc_linter/src/rules/react/rules_of_hooks.rs b/crates/oxc_linter/src/rules/react/rules_of_hooks.rs
@@ -1,3 +1,5 @@
+use std::borrow::Cow;
+
 use oxc_ast::{
     ast::{ArrowFunctionExpression, Function},
     AstKind,
@@ -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::{
@@ -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(
@@ -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())
                     }
@@ -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)
@@ -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() {} }
diff --git a/crates/oxc_linter/src/rules/typescript/adjacent_overload_signatures.rs b/crates/oxc_linter/src/rules/typescript/adjacent_overload_signatures.rs
@@ -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),
@@ -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),
diff --git a/crates/oxc_linter/src/rules/typescript/consistent_type_imports.rs b/crates/oxc_linter/src/rules/typescript/consistent_type_imports.rs
@@ -1,4 +1,4 @@
-use std::{error::Error, ops::Deref};
+use std::{borrow::Cow, error::Error, ops::Deref};
 
 use itertools::Itertools;
 use oxc_ast::{
@@ -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,
@@ -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 {
@@ -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)
         }
     }
 }
diff --git a/crates/oxc_linter/src/rules/typescript/explicit_function_return_type.rs b/crates/oxc_linter/src/rules/typescript/explicit_function_return_type.rs
@@ -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,
         }
diff --git a/crates/oxc_semantic/src/class/builder.rs b/crates/oxc_semantic/src/class/builder.rs
@@ -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,
@@ -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,
@@ -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,
diff --git a/crates/oxc_span/src/atom.rs b/crates/oxc_span/src/atom.rs
diff --git a/tasks/prettier_conformance/src/spec.rs b/tasks/prettier_conformance/src/spec.rs

Original file line number	Diff line number	Diff line change
`@@ -1,9 +1,9 @@`
`1`	`1`	`use crate::ast::*;`
`2`	`2`
`3`		`-use std::{cell::Cell, fmt, hash::Hash};`
	`3`	`+use std::{borrow::Cow, cell::Cell, fmt, hash::Hash};`
`4`	`4`
`5`	`5`	`use oxc_allocator::{Box, FromIn, Vec};`
`6`		`-use oxc_span::{Atom, CompactStr, GetSpan, SourceType, Span};`
	`6`	`+use oxc_span::{Atom, GetSpan, SourceType, Span};`
`7`	`7`	`use oxc_syntax::{`
`8`	`8`	`operator::UnaryOperator,`
`9`	`9`	`reference::{ReferenceFlag, ReferenceId},`
`@@ -332,20 +332,20 @@ impl<'a> ObjectExpression<'a> {`
`332`	`332`	`}`
`333`	`333`
`334`	`334`	`impl<'a> PropertyKey<'a> {`
`335`		`- pub fn static_name(&self) -> Option<CompactStr> {`
	`335`	`+ pub fn static_name(&self) -> Option<Cow<'a, str>> {`
`336`	`336`	`match self {`
`337`		`- Self::StaticIdentifier(ident) => Some(ident.name.to_compact_str()),`
`338`		`- Self::StringLiteral(lit) => Some(lit.value.to_compact_str()),`
`339`		`- Self::RegExpLiteral(lit) => Some(lit.regex.to_string().into()),`
`340`		`- Self::NumericLiteral(lit) => Some(lit.value.to_string().into()),`
`341`		`- Self::BigIntLiteral(lit) => Some(lit.raw.to_compact_str()),`
`342`		`- Self::NullLiteral(_) => Some("null".into()),`
	`337`	`+ Self::StaticIdentifier(ident) => Some(Cow::Borrowed(ident.name.as_str())),`
	`338`	`+ Self::StringLiteral(lit) => Some(Cow::Borrowed(lit.value.as_str())),`
	`339`	`+ Self::RegExpLiteral(lit) => Some(Cow::Owned(lit.regex.to_string())),`
	`340`	`+ Self::NumericLiteral(lit) => Some(Cow::Owned(lit.value.to_string())),`
	`341`	`+ Self::BigIntLiteral(lit) => Some(Cow::Borrowed(lit.raw.as_str())),`
	`342`	`+ Self::NullLiteral(_) => Some(Cow::Borrowed("null")),`
`343`	`343`	`Self::TemplateLiteral(lit) => lit`
`344`	`344`	`.expressions`
`345`	`345`	`.is_empty()`
`346`	`346`	`.then(\|\| lit.quasi())`
`347`	`347`	`.flatten()`
`348`		`- .map(\|quasi\| quasi.to_compact_str()),`
	`348`	`+ .map(std::convert::Into::into),`
`349`	`349`	`_ => None,`
`350`	`350`	`}`
`351`	`351`	`}`
`@@ -369,9 +369,9 @@ impl<'a> PropertyKey<'a> {`
`369`	`369`	`}`
`370`	`370`	`}`
`371`	`371`
`372`		`- pub fn name(&self) -> Option<CompactStr> {`
	`372`	`+ pub fn name(&self) -> Option<Cow<'a, str>> {`
`373`	`373`	`if self.is_private_identifier() {`
`374`		`- self.private_name().map(\|name\| name.to_compact_str())`
	`374`	`+ self.private_name().map(\|name\| Cow::Borrowed(name.as_str()))`
`375`	`375`	`} else {`
`376`	`376`	`self.static_name()`
`377`	`377`	`}`
`@@ -1237,7 +1237,7 @@ impl<'a> ClassElement<'a> {`
`1237`	`1237`	`}`
`1238`	`1238`	`}`
`1239`	`1239`
`1240`		`- pub fn static_name(&self) -> Option<CompactStr> {`
	`1240`	`+ pub fn static_name(&self) -> Option<Cow<'a, str>> {`
`1241`	`1241`	`match self {`
`1242`	`1242`	`Self::TSIndexSignature(_) \| Self::StaticBlock(_) => None,`
`1243`	`1243`	`Self::MethodDefinition(def) => def.key.static_name(),`
`@@ -1424,8 +1424,8 @@ impl<'a> ImportDeclarationSpecifier<'a> {`
`1424`	`1424`	`ImportDeclarationSpecifier::ImportDefaultSpecifier(specifier) => &specifier.local,`
`1425`	`1425`	`}`
`1426`	`1426`	`}`
`1427`		`- pub fn name(&self) -> CompactStr {`
`1428`		`- self.local().name.to_compact_str()`
	`1427`	`+ pub fn name(&self) -> Cow<'a, str> {`
	`1428`	`+ Cow::Borrowed(self.local().name.as_str())`
`1429`	`1429`	`}`
`1430`	`1430`	`}`
`1431`	`1431`
Original file line number	Diff line number	Diff line change
`@@ -104,7 +104,7 @@ impl<'a> IsolatedDeclarations<'a> {`
`104`	`104`	`self.scope.has_reference(&specifier.local.name)`
`105`	`105`	`}`
`106`	`106`	`ImportDeclarationSpecifier::ImportNamespaceSpecifier(_) => {`
`107`		`- self.scope.has_reference(specifier.name().as_str())`
	`107`	`+ self.scope.has_reference(&specifier.name())`
`108`	`108`	`}`
`109`	`109`	`});`
`110`	`110`	`if specifiers.is_empty() {`
Original file line number	Diff line number	Diff line change
`@@ -64,7 +64,7 @@ impl Rule for NoDanger {`
`64`	`64`	`for prop in &obj_expr.properties {`
`65`	`65`	`if let ObjectPropertyKind::ObjectProperty(obj_prop) = prop {`
`66`	`66`	`if let Some(prop_name) = obj_prop.key.static_name() {`
`67`		`- if prop_name.as_str() == "dangerouslySetInnerHTML" {`
	`67`	`+ if prop_name == "dangerouslySetInnerHTML" {`
`68`	`68`	`ctx.diagnostic(no_danger_diagnostic(obj_prop.key.span()));`
`69`	`69`	`}`
`70`	`70`	`}`
Original file line number	Diff line number	Diff line change
`@@ -316,19 +316,19 @@ impl ExplicitFunctionReturnType {`
`316`	`316`	`let Some(name) = def.key.name() else { return false };`
`317`	`317`	`def.key.is_identifier()`
`318`	`318`	`&& !def.computed`
`319`		`- && self.allowed_names.contains(name.as_str())`
	`319`	`+ && self.allowed_names.contains(name.as_ref())`
`320`	`320`	`}`
`321`	`321`	`AstKind::PropertyDefinition(def) => {`
`322`	`322`	`let Some(name) = def.key.name() else { return false };`
`323`	`323`	`def.key.is_identifier()`
`324`	`324`	`&& !def.computed`
`325`		`- && self.allowed_names.contains(name.as_str())`
	`325`	`+ && self.allowed_names.contains(name.as_ref())`
`326`	`326`	`}`
`327`	`327`	`AstKind::ObjectProperty(prop) => {`
`328`	`328`	`let Some(name) = prop.key.name() else { return false };`
`329`	`329`	`prop.key.is_identifier()`
`330`	`330`	`&& !prop.computed`
`331`		`- && self.allowed_names.contains(name.as_str())`
	`331`	`+ && self.allowed_names.contains(name.as_ref())`
`332`	`332`	`}`
`333`	`333`	`_ => false,`
`334`	`334`	`}`