Reduce size of Expr from 80 to 64 bytes (#9900)

## Summary This PR reduces the size of `Expr` from 80 to 64 bytes, by reducing the sizes of... - `ExprCall` from 72 to 56 bytes, by using boxed slices for `Arguments`. - `ExprCompare` from 64 to 48 bytes, by using boxed slices for its various vectors. In testing, the parser gets a bit faster, and the linter benchmarks improve quite a bit.
astral-sh · Feb 9, 2024 · 49fe1b8 · 49fe1b8
1 parent bd8123c
commit 49fe1b8
Show file tree

Hide file tree

Showing 78 changed files with 326 additions and 258 deletions.
diff --git a/crates/ruff_linter/src/checkers/ast/mod.rs b/crates/ruff_linter/src/checkers/ast/mod.rs
@@ -31,8 +31,8 @@ use std::path::Path;
 use itertools::Itertools;
 use log::debug;
 use ruff_python_ast::{
-    self as ast, Arguments, Comprehension, ElifElseClause, ExceptHandler, Expr, ExprContext,
-    Keyword, MatchCase, Parameter, ParameterWithDefault, Parameters, Pattern, Stmt, Suite, UnaryOp,
+    self as ast, Comprehension, ElifElseClause, ExceptHandler, Expr, ExprContext, Keyword,
+    MatchCase, Parameter, ParameterWithDefault, Parameters, Pattern, Stmt, Suite, UnaryOp,
 };
 use ruff_text_size::{Ranged, TextRange, TextSize};
 
@@ -989,12 +989,7 @@ where
             }
             Expr::Call(ast::ExprCall {
                 func,
-                arguments:
-                    Arguments {
-                        args,
-                        keywords,
-                        range: _,
-                    },
+                arguments,
                 range: _,
             }) => {
                 self.visit_expr(func);
@@ -1037,7 +1032,7 @@ where
                 });
                 match callable {
                     Some(typing::Callable::Bool) => {
-                        let mut args = args.iter();
+                        let mut args = arguments.args.iter();
                         if let Some(arg) = args.next() {
                             self.visit_boolean_test(arg);
                         }
@@ -1046,7 +1041,7 @@ where
                         }
                     }
                     Some(typing::Callable::Cast) => {
-                        let mut args = args.iter();
+                        let mut args = arguments.args.iter();
                         if let Some(arg) = args.next() {
                             self.visit_type_definition(arg);
                         }
@@ -1055,7 +1050,7 @@ where
                         }
                     }
                     Some(typing::Callable::NewType) => {
-                        let mut args = args.iter();
+                        let mut args = arguments.args.iter();
                         if let Some(arg) = args.next() {
                             self.visit_non_type_definition(arg);
                         }
@@ -1064,21 +1059,21 @@ where
                         }
                     }
                     Some(typing::Callable::TypeVar) => {
-                        let mut args = args.iter();
+                        let mut args = arguments.args.iter();
                         if let Some(arg) = args.next() {
                             self.visit_non_type_definition(arg);
                         }
                         for arg in args {
                             self.visit_type_definition(arg);
                         }
-                        for keyword in keywords {
+                        for keyword in arguments.keywords.iter() {
                             let Keyword {
                                 arg,
                                 value,
                                 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);
@@ -1088,7 +1083,7 @@ where
                     }
                     Some(typing::Callable::NamedTuple) => {
                         // Ex) NamedTuple("a", [("a", int)])
-                        let mut args = args.iter();
+                        let mut args = arguments.args.iter();
                         if let Some(arg) = args.next() {
                             self.visit_non_type_definition(arg);
                         }
@@ -1117,7 +1112,7 @@ where
                             }
                         }
 
-                        for keyword in keywords {
+                        for keyword in arguments.keywords.iter() {
                             let Keyword { arg, value, .. } = keyword;
                             match (arg.as_ref(), value) {
                                 // Ex) NamedTuple("a", **{"a": int})
@@ -1144,7 +1139,7 @@ where
                     }
                     Some(typing::Callable::TypedDict) => {
                         // Ex) TypedDict("a", {"a": int})
-                        let mut args = args.iter();
+                        let mut args = arguments.args.iter();
                         if let Some(arg) = args.next() {
                             self.visit_non_type_definition(arg);
                         }
@@ -1167,27 +1162,27 @@ where
                         }
 
                         // Ex) TypedDict("a", a=int)
-                        for keyword in keywords {
+                        for keyword in arguments.keywords.iter() {
                             let Keyword { value, .. } = keyword;
                             self.visit_type_definition(value);
                         }
                     }
                     Some(typing::Callable::MypyExtension) => {
-                        let mut args = args.iter();
+                        let mut args = arguments.args.iter();
                         if let Some(arg) = args.next() {
                             // Ex) DefaultNamedArg(bool | None, name="some_prop_name")
                             self.visit_type_definition(arg);
 
                             for arg in args {
                                 self.visit_non_type_definition(arg);
                             }
-                            for keyword in keywords {
+                            for keyword in arguments.keywords.iter() {
                                 let Keyword { value, .. } = keyword;
                                 self.visit_non_type_definition(value);
                             }
                         } else {
                             // Ex) DefaultNamedArg(type="bool", name="some_prop_name")
-                            for keyword in keywords {
+                            for keyword in arguments.keywords.iter() {
                                 let Keyword {
                                     value,
                                     arg,
@@ -1205,10 +1200,10 @@ where
                         // If we're in a type definition, we need to treat the arguments to any
                         // other callables as non-type definitions (i.e., we don't want to treat
                         // any strings as deferred type definitions).
-                        for arg in args {
+                        for arg in arguments.args.iter() {
                             self.visit_non_type_definition(arg);
                         }
-                        for keyword in keywords {
+                        for keyword in arguments.keywords.iter() {
                             let Keyword { value, .. } = keyword;
                             self.visit_non_type_definition(value);
                         }

diff --git a/crates/ruff_linter/src/rules/flake8_bugbear/rules/assert_false.rs b/crates/ruff_linter/src/rules/flake8_bugbear/rules/assert_false.rs
@@ -59,11 +59,11 @@ fn assertion_error(msg: Option<&Expr>) -> Stmt {
             })),
             arguments: Arguments {
                 args: if let Some(msg) = msg {
-                    vec![msg.clone()]
+                    Box::from([msg.clone()])
                 } else {
-                    vec![]
+                    Box::from([])
                 },
-                keywords: vec![],
+                keywords: Box::from([]),
                 range: TextRange::default(),
             },
             range: TextRange::default(),

diff --git a/crates/ruff_linter/src/rules/flake8_bugbear/rules/assert_raises_exception.rs b/crates/ruff_linter/src/rules/flake8_bugbear/rules/assert_raises_exception.rs
@@ -91,7 +91,7 @@ pub(crate) fn assert_raises_exception(checker: &mut Checker, items: &[WithItem])
             return;
         }
 
-        let [arg] = arguments.args.as_slice() else {
+        let [arg] = &*arguments.args else {
             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
@@ -3,7 +3,7 @@ use ruff_macros::{derive_message_formats, violation};
 use ruff_python_ast::types::Node;
 use ruff_python_ast::visitor;
 use ruff_python_ast::visitor::Visitor;
-use ruff_python_ast::{self as ast, Arguments, Comprehension, Expr, ExprContext, Stmt};
+use ruff_python_ast::{self as ast, Comprehension, Expr, ExprContext, Stmt};
 use ruff_text_size::Ranged;
 
 use crate::checkers::ast::Checker;
@@ -126,18 +126,13 @@ impl<'a> Visitor<'a> for SuspiciousVariablesVisitor<'a> {
         match expr {
             Expr::Call(ast::ExprCall {
                 func,
-                arguments:
-                    Arguments {
-                        args,
-                        keywords,
-                        range: _,
-                    },
+                arguments,
                 range: _,
             }) => {
                 match func.as_ref() {
                     Expr::Name(ast::ExprName { id, .. }) => {
                         if matches!(id.as_str(), "filter" | "reduce" | "map") {
-                            for arg in args {
+                            for arg in arguments.args.iter() {
                                 if arg.is_lambda_expr() {
                                     self.safe_functions.push(arg);
                                 }
@@ -148,7 +143,7 @@ impl<'a> Visitor<'a> for SuspiciousVariablesVisitor<'a> {
                         if attr == "reduce" {
                             if let Expr::Name(ast::ExprName { id, .. }) = value.as_ref() {
                                 if id == "functools" {
-                                    for arg in args {
+                                    for arg in arguments.args.iter() {
                                         if arg.is_lambda_expr() {
                                             self.safe_functions.push(arg);
                                         }
@@ -160,7 +155,7 @@ impl<'a> Visitor<'a> for SuspiciousVariablesVisitor<'a> {
                     _ => {}
                 }
 
-                for keyword in keywords {
+                for keyword in arguments.keywords.iter() {
                     if keyword.arg.as_ref().is_some_and(|arg| arg == "key")
                         && keyword.value.is_lambda_expr()
                     {

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
@@ -114,7 +114,7 @@ fn is_infinite_iterator(arg: &Expr, semantic: &SemanticModel) -> bool {
                 }
 
                 // Ex) `iterools.repeat(1, times=None)`
-                for keyword in keywords {
+                for keyword in keywords.iter() {
                     if keyword.arg.as_ref().is_some_and(|name| name == "times") {
                         if keyword.value.is_none_literal_expr() {
                             return true;

diff --git a/crates/ruff_linter/src/rules/flake8_django/rules/nullable_model_string_field.rs b/crates/ruff_linter/src/rules/flake8_django/rules/nullable_model_string_field.rs
@@ -88,7 +88,7 @@ fn is_nullable_field<'a>(value: &'a Expr, semantic: &'a SemanticModel) -> Option
     let mut null_key = false;
     let mut blank_key = false;
     let mut unique_key = false;
-    for keyword in &call.arguments.keywords {
+    for keyword in call.arguments.keywords.iter() {
         let Some(argument) = &keyword.arg else {
             continue;
         };

diff --git a/crates/ruff_linter/src/rules/flake8_logging_format/rules/logging_call.rs b/crates/ruff_linter/src/rules/flake8_logging_format/rules/logging_call.rs
@@ -113,7 +113,7 @@ fn check_log_record_attr_clash(checker: &mut Checker, extra: &Keyword) {
                 .resolve_call_path(func)
                 .is_some_and(|call_path| matches!(call_path.as_slice(), ["", "dict"]))
             {
-                for keyword in keywords {
+                for keyword in keywords.iter() {
                     if let Some(attr) = &keyword.arg {
                         if is_reserved_attr(attr) {
                             checker.diagnostics.push(Diagnostic::new(

diff --git a/crates/ruff_linter/src/rules/flake8_pie/rules/multiple_starts_ends_with.rs b/crates/ruff_linter/src/rules/flake8_pie/rules/multiple_starts_ends_with.rs
@@ -97,7 +97,7 @@ pub(crate) fn multiple_starts_ends_with(checker: &mut Checker, expr: &Expr) {
             continue;
         }
 
-        let [arg] = args.as_slice() else {
+        let [arg] = &**args else {
             continue;
         };
 
@@ -188,8 +188,8 @@ pub(crate) fn multiple_starts_ends_with(checker: &mut Checker, expr: &Expr) {
             let node3 = Expr::Call(ast::ExprCall {
                 func: Box::new(node2),
                 arguments: Arguments {
-                    args: vec![node],
-                    keywords: vec![],
+                    args: Box::from([node]),
+                    keywords: Box::from([]),
                     range: TextRange::default(),
                 },
                 range: TextRange::default(),

diff --git a/crates/ruff_linter/src/rules/flake8_pie/rules/unnecessary_dict_kwargs.rs b/crates/ruff_linter/src/rules/flake8_pie/rules/unnecessary_dict_kwargs.rs
@@ -59,7 +59,7 @@ impl Violation for UnnecessaryDictKwargs {
 /// PIE804
 pub(crate) fn unnecessary_dict_kwargs(checker: &mut Checker, call: &ast::ExprCall) {
     let mut duplicate_keywords = None;
-    for keyword in &call.arguments.keywords {
+    for keyword in call.arguments.keywords.iter() {
         // keyword is a spread operator (indicated by None).
         if keyword.arg.is_some() {
             continue;
@@ -145,7 +145,7 @@ fn duplicates(call: &ast::ExprCall) -> FxHashSet<&str> {
         call.arguments.keywords.len(),
         BuildHasherDefault::default(),
     );
-    for keyword in &call.arguments.keywords {
+    for keyword in call.arguments.keywords.iter() {
         if let Some(name) = &keyword.arg {
             if !seen.insert(name.as_str()) {
                 duplicates.insert(name.as_str());

diff --git a/crates/ruff_linter/src/rules/flake8_pie/rules/unnecessary_range_start.rs b/crates/ruff_linter/src/rules/flake8_pie/rules/unnecessary_range_start.rs
@@ -60,7 +60,7 @@ pub(crate) fn unnecessary_range_start(checker: &mut Checker, call: &ast::ExprCal
     }
 
     // Verify that the call has exactly two arguments (no `step`).
-    let [start, _] = call.arguments.args.as_slice() else {
+    let [start, _] = &*call.arguments.args else {
         return;
     };
 

diff --git a/crates/ruff_linter/src/rules/flake8_pyi/rules/bad_version_info_comparison.rs b/crates/ruff_linter/src/rules/flake8_pyi/rules/bad_version_info_comparison.rs
@@ -69,7 +69,7 @@ pub(crate) fn bad_version_info_comparison(checker: &mut Checker, test: &Expr) {
         return;
     };
 
-    let ([op], [_right]) = (ops.as_slice(), comparators.as_slice()) else {
+    let ([op], [_right]) = (&**ops, &**comparators) else {
         return;
     };
 

diff --git a/crates/ruff_linter/src/rules/flake8_pyi/rules/unrecognized_platform.rs b/crates/ruff_linter/src/rules/flake8_pyi/rules/unrecognized_platform.rs
@@ -101,7 +101,7 @@ pub(crate) fn unrecognized_platform(checker: &mut Checker, test: &Expr) {
         return;
     };
 
-    let ([op], [right]) = (ops.as_slice(), comparators.as_slice()) else {
+    let ([op], [right]) = (&**ops, &**comparators) else {
         return;
     };
 

diff --git a/crates/ruff_linter/src/rules/flake8_pyi/rules/unrecognized_version_info.rs b/crates/ruff_linter/src/rules/flake8_pyi/rules/unrecognized_version_info.rs
@@ -129,7 +129,7 @@ pub(crate) fn unrecognized_version_info(checker: &mut Checker, test: &Expr) {
         return;
     };
 
-    let ([op], [comparator]) = (ops.as_slice(), comparators.as_slice()) else {
+    let ([op], [comparator]) = (&**ops, &**comparators) else {
         return;
     };
 

diff --git a/crates/ruff_linter/src/rules/flake8_pytest_style/rules/assertion.rs b/crates/ruff_linter/src/rules/flake8_pytest_style/rules/assertion.rs
@@ -411,7 +411,7 @@ fn to_pytest_raises_args<'a>(
 ) -> Option<Cow<'a, str>> {
     let args = match attr {
         "assertRaises" | "failUnlessRaises" => {
-            match (arguments.args.as_slice(), arguments.keywords.as_slice()) {
+            match (&*arguments.args, &*arguments.keywords) {
                 // Ex) `assertRaises(Exception)`
                 ([arg], []) => Cow::Borrowed(checker.locator().slice(arg)),
                 // Ex) `assertRaises(expected_exception=Exception)`
@@ -427,7 +427,7 @@ fn to_pytest_raises_args<'a>(
             }
         }
         "assertRaisesRegex" | "assertRaisesRegexp" => {
-            match (arguments.args.as_slice(), arguments.keywords.as_slice()) {
+            match (&*arguments.args, &*arguments.keywords) {
                 // Ex) `assertRaisesRegex(Exception, regex)`
                 ([arg1, arg2], []) => Cow::Owned(format!(
                     "{}, match={}",

diff --git a/crates/ruff_linter/src/rules/flake8_pytest_style/rules/parametrize.rs b/crates/ruff_linter/src/rules/flake8_pytest_style/rules/parametrize.rs
@@ -638,17 +638,17 @@ pub(crate) fn parametrize(checker: &mut Checker, decorators: &[Decorator]) {
             }) = &decorator.expression
             {
                 if checker.enabled(Rule::PytestParametrizeNamesWrongType) {
-                    if let [names, ..] = args.as_slice() {
+                    if let [names, ..] = &**args {
                         check_names(checker, decorator, names);
                     }
                 }
                 if checker.enabled(Rule::PytestParametrizeValuesWrongType) {
-                    if let [names, values, ..] = args.as_slice() {
+                    if let [names, values, ..] = &**args {
                         check_values(checker, names, values);
                     }
                 }
                 if checker.enabled(Rule::PytestDuplicateParametrizeTestCases) {
-                    if let [_, values, ..] = args.as_slice() {
+                    if let [_, values, ..] = &**args {
                         check_duplicates(checker, values);
                     }
                 }

diff --git a/crates/ruff_linter/src/rules/flake8_pytest_style/rules/unittest_assert.rs b/crates/ruff_linter/src/rules/flake8_pytest_style/rules/unittest_assert.rs
@@ -173,8 +173,8 @@ fn assert(expr: &Expr, msg: Option<&Expr>) -> Stmt {
 fn compare(left: &Expr, cmp_op: CmpOp, right: &Expr) -> Expr {
     Expr::Compare(ast::ExprCompare {
         left: Box::new(left.clone()),
-        ops: vec![cmp_op],
-        comparators: vec![right.clone()],
+        ops: Box::from([cmp_op]),
+        comparators: Box::from([right.clone()]),
         range: TextRange::default(),
     })
 }
@@ -390,8 +390,8 @@ impl UnittestAssert {
                 let node1 = ast::ExprCall {
                     func: Box::new(node.into()),
                     arguments: Arguments {
-                        args: vec![(**obj).clone(), (**cls).clone()],
-                        keywords: vec![],
+                        args: Box::from([(**obj).clone(), (**cls).clone()]),
+                        keywords: Box::from([]),
                         range: TextRange::default(),
                     },
                     range: TextRange::default(),
@@ -434,8 +434,8 @@ impl UnittestAssert {
                 let node2 = ast::ExprCall {
                     func: Box::new(node1.into()),
                     arguments: Arguments {
-                        args: vec![(**regex).clone(), (**text).clone()],
-                        keywords: vec![],
+                        args: Box::from([(**regex).clone(), (**text).clone()]),
+                        keywords: Box::from([]),
                         range: TextRange::default(),
                     },
                     range: TextRange::default(),