perf: use compact_str

There are many low-hanging fruits and other systems to change to use compact_str, but this is a good start. I am interested in seeing how much this improves performance.
astral-sh · Oct 25, 2023 · 747ab77 · 747ab77
1 parent 599047c
commit 747ab77
Show file tree

Hide file tree

Showing 92 changed files with 345 additions and 271 deletions.
diff --git a/Cargo.lock b/Cargo.lock
diff --git a/Cargo.toml b/Cargo.toml
@@ -17,6 +17,7 @@ bitflags = { version = "2.3.1" }
 chrono = { version = "0.4.31", default-features = false, features = ["clock"] }
 clap = { version = "4.4.6", features = ["derive"] }
 colored = { version = "2.0.0" }
+compact_str = { version = "0.7.1" }
 filetime = { version = "0.2.20" }
 glob = { version = "0.3.1" }
 globset = { version = "0.4.10" }
@@ -52,7 +53,12 @@ tracing-subscriber = { version = "0.3.17", features = ["env-filter"] }
 unicode-ident = { version = "1.0.12" }
 unicode_names2 = { version = "1.2.0" }
 unicode-width = { version = "0.1.11" }
-uuid = { version = "1.4.1", features = ["v4", "fast-rng", "macro-diagnostics", "js"] }
+uuid = { version = "1.4.1", features = [
+    "v4",
+    "fast-rng",
+    "macro-diagnostics",
+    "js",
+] }
 wsl = { version = "0.1.0" }
 
 [profile.release]

diff --git a/crates/ruff_linter/Cargo.toml b/crates/ruff_linter/Cargo.toml
@@ -36,6 +36,7 @@ bitflags = { workspace = true }
 chrono = { workspace = true }
 clap = { workspace = true, features = ["derive", "string"], optional = true }
 colored = { workspace = true }
+compact_str = { workspace = true }
 fern = { version = "0.6.1" }
 glob = { workspace = true }
 globset = { workspace = true }

diff --git a/crates/ruff_linter/src/checkers/ast/mod.rs b/crates/ruff_linter/src/checkers/ast/mod.rs
@@ -832,7 +832,7 @@ where
                 range: _,
             }) => {
                 if let Expr::Name(ast::ExprName { id, ctx, range: _ }) = func.as_ref() {
-                    if id == "locals" && ctx.is_load() {
+                    if id.as_str() == "locals" && ctx.is_load() {
                         let scope = self.semantic.current_scope_mut();
                         scope.set_uses_locals();
                     }
@@ -1005,7 +1005,7 @@ where
                                 range: _,
                             } = keyword;
                             if let Some(id) = arg {
-                                if id == "bound" {
+                                if id.as_str() == "bound" {
                                     self.visit_type_definition(value);
                                 } else {
                                     self.visit_non_type_definition(value);
@@ -1664,21 +1664,21 @@ impl<'a> Checker<'a> {
             && match parent {
                 Stmt::Assign(ast::StmtAssign { targets, .. }) => {
                     if let Some(Expr::Name(ast::ExprName { id, .. })) = targets.first() {
-                        id == "__all__"
+                        id.as_str() == "__all__"
                     } else {
                         false
                     }
                 }
                 Stmt::AugAssign(ast::StmtAugAssign { target, .. }) => {
                     if let Expr::Name(ast::ExprName { id, .. }) = target.as_ref() {
-                        id == "__all__"
+                        id.as_str() == "__all__"
                     } else {
                         false
                     }
                 }
                 Stmt::AnnAssign(ast::StmtAnnAssign { target, .. }) => {
                     if let Expr::Name(ast::ExprName { id, .. }) = target.as_ref() {
-                        id == "__all__"
+                        id.as_str() == "__all__"
                     } else {
                         false
                     }

diff --git a/crates/ruff_linter/src/checkers/imports.rs b/crates/ruff_linter/src/checkers/imports.rs
@@ -2,6 +2,7 @@
 use std::borrow::Cow;
 use std::path::Path;
 
+use compact_str::CompactString;
 use ruff_diagnostics::Diagnostic;
 use ruff_python_ast::helpers::to_module_path;
 use ruff_python_ast::imports::{ImportMap, ModuleImport};
@@ -46,9 +47,9 @@ fn extract_import_map(path: &Path, package: Option<&Path>, blocks: &[&Block]) ->
             }) => {
                 let level = level.unwrap_or_default() as usize;
                 let module = if let Some(module) = module {
-                    let module: &String = module.as_ref();
+                    let module: &CompactString = module.as_ref();
                     if level == 0 {
-                        Cow::Borrowed(module)
+                        Cow::Borrowed(module.as_str())
                     } else {
                         if module_path.len() <= level {
                             continue;

diff --git a/crates/ruff_linter/src/rules/airflow/rules/task_variable_name.rs b/crates/ruff_linter/src/rules/airflow/rules/task_variable_name.rs
@@ -88,7 +88,7 @@ pub(crate) fn variable_name_task_id(
     };
 
     // If the target name is the same as the task_id, no violation.
-    if id == task_id {
+    if id.as_str() == task_id.as_str() {
         return None;
     }
 

diff --git a/crates/ruff_linter/src/rules/flake8_bandit/rules/jinja2_autoescape_false.rs b/crates/ruff_linter/src/rules/flake8_bandit/rules/jinja2_autoescape_false.rs
@@ -72,7 +72,7 @@ pub(crate) fn jinja2_autoescape_false(checker: &mut Checker, call: &ast::ExprCal
                 }) => (),
                 Expr::Call(ast::ExprCall { func, .. }) => {
                     if let Expr::Name(ast::ExprName { id, .. }) = func.as_ref() {
-                        if id != "select_autoescape" {
+                        if id.as_str() != "select_autoescape" {
                             checker.diagnostics.push(Diagnostic::new(
                                 Jinja2AutoescapeFalse { value: true },
                                 keyword.range(),

diff --git a/crates/ruff_linter/src/rules/flake8_blind_except/rules/blind_except.rs b/crates/ruff_linter/src/rules/flake8_blind_except/rules/blind_except.rs
@@ -77,7 +77,7 @@ pub(crate) fn blind_except(
         if let Stmt::Raise(ast::StmtRaise { exc, .. }) = stmt {
             if let Some(exc) = exc {
                 if let Expr::Name(ast::ExprName { id, .. }) = exc.as_ref() {
-                    name.is_some_and(|name| id == name)
+                    name.is_some_and(|name| id.as_str() == name)
                 } else {
                     false
                 }

diff --git a/.../ruff_linter/src/rules/flake8_boolean_trap/rules/boolean_type_hint_positional_argument.rs b/.../ruff_linter/src/rules/flake8_boolean_trap/rules/boolean_type_hint_positional_argument.rs
@@ -125,11 +125,11 @@ pub(crate) fn boolean_type_hint_positional_argument(
 
         // check for both bool (python class) and 'bool' (string annotation)
         let hint = match annotation.as_ref() {
-            Expr::Name(name) => &name.id == "bool",
+            Expr::Name(name) => name.id.as_str() == "bool",
             Expr::Constant(ast::ExprConstant {
                 value: Constant::Str(ast::StringConstant { value, .. }),
                 ..
-            }) => value == "bool",
+            }) => value.as_str() == "bool",
             _ => false,
         };
         if !hint || !checker.semantic().is_builtin("bool") {

diff --git a/crates/ruff_linter/src/rules/flake8_bugbear/rules/assignment_to_os_environ.rs b/crates/ruff_linter/src/rules/flake8_bugbear/rules/assignment_to_os_environ.rs
@@ -63,7 +63,7 @@ pub(crate) fn assignment_to_os_environ(checker: &mut Checker, targets: &[Expr])
     let Expr::Name(ast::ExprName { id, .. }) = value.as_ref() else {
         return;
     };
-    if id != "os" {
+    if id.as_str() != "os" {
         return;
     }
     checker

diff --git a/crates/ruff_linter/src/rules/flake8_bugbear/rules/cached_instance_method.rs b/crates/ruff_linter/src/rules/flake8_bugbear/rules/cached_instance_method.rs
@@ -85,7 +85,7 @@ pub(crate) fn cached_instance_method(checker: &mut Checker, decorator_list: &[De
         // TODO(charlie): This should take into account `classmethod-decorators` and
         // `staticmethod-decorators`.
         if let Expr::Name(ast::ExprName { id, .. }) = &decorator.expression {
-            if id == "classmethod" || id == "staticmethod" {
+            if id.as_str() == "classmethod" || id.as_str() == "staticmethod" {
                 return;
             }
         }

diff --git a/crates/ruff_linter/src/rules/flake8_bugbear/rules/function_uses_loop_variable.rs b/crates/ruff_linter/src/rules/flake8_bugbear/rules/function_uses_loop_variable.rs
@@ -95,7 +95,11 @@ impl<'a> Visitor<'a> for SuspiciousVariablesVisitor<'a> {
                 // Treat any non-arguments as "suspicious".
                 self.names
                     .extend(visitor.loaded.into_iter().filter(|loaded| {
-                        if visitor.stored.iter().any(|stored| stored.id == loaded.id) {
+                        if visitor
+                            .stored
+                            .iter()
+                            .any(|stored| stored.id.as_str() == loaded.id)
+                        {
                             return false;
                         }
 
@@ -147,7 +151,7 @@ impl<'a> Visitor<'a> for SuspiciousVariablesVisitor<'a> {
                     Expr::Attribute(ast::ExprAttribute { value, attr, .. }) => {
                         if attr == "reduce" {
                             if let Expr::Name(ast::ExprName { id, .. }) = value.as_ref() {
-                                if id == "functools" {
+                                if id.as_str() == "functools" {
                                     for arg in args {
                                         if arg.is_lambda_expr() {
                                             self.safe_functions.push(arg);
@@ -181,7 +185,11 @@ impl<'a> Visitor<'a> for SuspiciousVariablesVisitor<'a> {
                     // Treat any non-arguments as "suspicious".
                     self.names
                         .extend(visitor.loaded.into_iter().filter(|loaded| {
-                            if visitor.stored.iter().any(|stored| stored.id == loaded.id) {
+                            if visitor
+                                .stored
+                                .iter()
+                                .any(|stored| stored.id.as_str() == loaded.id)
+                            {
                                 return false;
                             }
 

diff --git a/crates/ruff_linter/src/rules/flake8_bugbear/rules/getattr_with_constant.rs b/crates/ruff_linter/src/rules/flake8_bugbear/rules/getattr_with_constant.rs
@@ -57,7 +57,7 @@ pub(crate) fn getattr_with_constant(
     let Expr::Name(ast::ExprName { id, .. }) = func else {
         return;
     };
-    if id != "getattr" {
+    if id.as_str() != "getattr" {
         return;
     }
     let [obj, arg] = args else {

diff --git a/crates/ruff_linter/src/rules/flake8_bugbear/rules/raise_without_from_inside_except.rs b/crates/ruff_linter/src/rules/flake8_bugbear/rules/raise_without_from_inside_except.rs
@@ -87,7 +87,7 @@ pub(crate) fn raise_without_from_inside_except(
                 if let Some(name) = name {
                     if exc
                         .as_name_expr()
-                        .is_some_and(|ast::ExprName { id, .. }| name == id)
+                        .is_some_and(|ast::ExprName { id, .. }| name == id.as_str())
                     {
                         continue;
                     }

diff --git a/crates/ruff_linter/src/rules/flake8_bugbear/rules/reuse_of_groupby_generator.rs b/crates/ruff_linter/src/rules/flake8_bugbear/rules/reuse_of_groupby_generator.rs
@@ -83,7 +83,7 @@ impl<'a> GroupNameFinder<'a> {
 
     fn name_matches(&self, expr: &Expr) -> bool {
         if let Expr::Name(ast::ExprName { id, .. }) = expr {
-            id == self.group_name
+            id.as_str() == self.group_name
         } else {
             false
         }

diff --git a/crates/ruff_linter/src/rules/flake8_bugbear/rules/setattr_with_constant.rs b/crates/ruff_linter/src/rules/flake8_bugbear/rules/setattr_with_constant.rs
@@ -71,7 +71,7 @@ pub(crate) fn setattr_with_constant(
     let Expr::Name(ast::ExprName { id, .. }) = func else {
         return;
     };
-    if id != "setattr" {
+    if id.as_str() != "setattr" {
         return;
     }
     let [obj, name, value] = args else {

diff --git a/crates/ruff_linter/src/rules/flake8_bugbear/rules/unintentional_type_annotation.rs b/crates/ruff_linter/src/rules/flake8_bugbear/rules/unintentional_type_annotation.rs
@@ -54,7 +54,7 @@ pub(crate) fn unintentional_type_annotation(
         }
         Expr::Attribute(ast::ExprAttribute { value, .. }) => {
             if let Expr::Name(ast::ExprName { id, .. }) = value.as_ref() {
-                if id != "self" {
+                if id.as_str() != "self" {
                     checker
                         .diagnostics
                         .push(Diagnostic::new(UnintentionalTypeAnnotation, stmt.range()));

diff --git a/crates/ruff_linter/src/rules/flake8_bugbear/rules/unreliable_callable_check.rs b/crates/ruff_linter/src/rules/flake8_bugbear/rules/unreliable_callable_check.rs
@@ -74,12 +74,12 @@ pub(crate) fn unreliable_callable_check(
     else {
         return;
     };
-    if value != "__call__" {
+    if value.as_str() != "__call__" {
         return;
     }
 
     let mut diagnostic = Diagnostic::new(UnreliableCallableCheck, expr.range());
-    if id == "hasattr" {
+    if id.as_str() == "hasattr" {
         if checker.semantic().is_builtin("callable") {
             diagnostic.set_fix(Fix::safe_edit(Edit::range_replacement(
                 format!("callable({})", checker.locator().slice(obj)),

diff --git a/crates/ruff_linter/src/rules/flake8_bugbear/rules/zip_without_explicit_strict.rs b/crates/ruff_linter/src/rules/flake8_bugbear/rules/zip_without_explicit_strict.rs
@@ -43,7 +43,7 @@ impl Violation for ZipWithoutExplicitStrict {
 /// B905
 pub(crate) fn zip_without_explicit_strict(checker: &mut Checker, call: &ast::ExprCall) {
     if let Expr::Name(ast::ExprName { id, .. }) = call.func.as_ref() {
-        if id == "zip"
+        if id.as_str() == "zip"
             && checker.semantic().is_builtin("zip")
             && call.arguments.find_keyword("strict").is_none()
             && !call

diff --git a/crates/ruff_linter/src/rules/flake8_comprehensions/rules/helpers.rs b/crates/ruff_linter/src/rules/flake8_comprehensions/rules/helpers.rs
@@ -13,7 +13,7 @@ pub(super) fn exactly_one_argument_with_matching_function<'a>(
         return None;
     }
     let func = func.as_name_expr()?;
-    if func.id != name {
+    if func.id.as_str() != name {
         return None;
     }
     Some(arg)
@@ -24,7 +24,10 @@ pub(super) fn first_argument_with_matching_function<'a>(
     func: &Expr,
     args: &'a [Expr],
 ) -> Option<&'a Expr> {
-    if func.as_name_expr().is_some_and(|func| func.id == name) {
+    if func
+        .as_name_expr()
+        .is_some_and(|func| func.id.as_str() == name)
+    {
         args.first()
     } else {
         None

diff --git a/crates/ruff_linter/src/rules/flake8_comprehensions/rules/unnecessary_call_around_sorted.rs b/crates/ruff_linter/src/rules/flake8_comprehensions/rules/unnecessary_call_around_sorted.rs
@@ -70,7 +70,7 @@ pub(crate) fn unnecessary_call_around_sorted(
     let Some(inner) = func.as_name_expr() else {
         return;
     };
-    if inner.id != "sorted" {
+    if inner.id.as_str() != "sorted" {
         return;
     }
     if !checker.semantic().is_builtin(&inner.id) || !checker.semantic().is_builtin(&outer.id) {
@@ -85,7 +85,7 @@ pub(crate) fn unnecessary_call_around_sorted(
     diagnostic.try_set_fix(|| {
         let edit =
             fixes::fix_unnecessary_call_around_sorted(expr, checker.locator(), checker.stylist())?;
-        if outer.id == "reversed" {
+        if outer.id.as_str() == "reversed" {
             Ok(Fix::unsafe_edit(edit))
         } else {
             Ok(Fix::safe_edit(edit))

diff --git a/...s/ruff_linter/src/rules/flake8_comprehensions/rules/unnecessary_double_cast_or_process.rs b/...s/ruff_linter/src/rules/flake8_comprehensions/rules/unnecessary_double_cast_or_process.rs
@@ -101,7 +101,7 @@ pub(crate) fn unnecessary_double_cast_or_process(
 
     // Avoid collapsing nested `sorted` calls with non-identical keyword arguments
     // (i.e., `key`, `reverse`).
-    if inner.id == "sorted" && outer.id == "sorted" {
+    if inner.id.as_str() == "sorted" && outer.id.as_str() == "sorted" {
         if inner_kw.len() != outer_kw.len() {
             return;
         }

diff --git a/crates/ruff_linter/src/rules/flake8_django/rules/all_with_model_form.rs b/crates/ruff_linter/src/rules/flake8_django/rules/all_with_model_form.rs
@@ -77,15 +77,15 @@ pub(crate) fn all_with_model_form(
                 let Expr::Name(ast::ExprName { id, .. }) = target else {
                     continue;
                 };
-                if id != "fields" {
+                if id.as_str() != "fields" {
                     continue;
                 }
                 let Expr::Constant(ast::ExprConstant { value, .. }) = value.as_ref() else {
                     continue;
                 };
                 match value {
                     Constant::Str(ast::StringConstant { value, .. }) => {
-                        if value == "__all__" {
+                        if value.as_str() == "__all__" {
                             return Some(Diagnostic::new(DjangoAllWithModelForm, element.range()));
                         }
                     }

diff --git a/crates/ruff_linter/src/rules/flake8_django/rules/exclude_with_model_form.rs b/crates/ruff_linter/src/rules/flake8_django/rules/exclude_with_model_form.rs
@@ -75,7 +75,7 @@ pub(crate) fn exclude_with_model_form(
                 let Expr::Name(ast::ExprName { id, .. }) = target else {
                     continue;
                 };
-                if id == "exclude" {
+                if id.as_str() == "exclude" {
                     return Some(Diagnostic::new(DjangoExcludeWithModelForm, target.range()));
                 }
             }

diff --git a/crates/ruff_linter/src/rules/flake8_django/rules/model_without_dunder_str.rs b/crates/ruff_linter/src/rules/flake8_django/rules/model_without_dunder_str.rs
@@ -117,7 +117,7 @@ fn is_model_abstract(body: &[Stmt]) -> bool {
                 let Expr::Name(ast::ExprName { id, .. }) = target else {
                     continue;
                 };
-                if id != "abstract" {
+                if id.as_str() != "abstract" {
                     continue;
                 }
                 if !is_const_true(value) {
-Original file line number
+Diff line change
@@ Expand Up @@
         let Expr::Name(ast::ExprName { id, .. }) = value.as_ref() else {
             return;
         };
-        if id != "os" {
+        if id.as_str() != "os" {
             return;
         }
         checker
@@ Expand Down @@