From da4da9b8d9a6bf58135975f429d9d26d1871466b Mon Sep 17 00:00:00 2001 From: Tobias Koppers Date: Tue, 30 Apr 2024 14:55:17 +0200 Subject: [PATCH 1/6] reduce JSValue size by 40 bytes (from 88 bytes) --- crates/turbopack-ecmascript/src/analyzer/mod.rs | 2 +- crates/turbopack-ecmascript/src/analyzer/well_known.rs | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/crates/turbopack-ecmascript/src/analyzer/mod.rs b/crates/turbopack-ecmascript/src/analyzer/mod.rs index 75c9d416e234a..dbd26db7b8f5a 100644 --- a/crates/turbopack-ecmascript/src/analyzer/mod.rs +++ b/crates/turbopack-ecmascript/src/analyzer/mod.rs @@ -370,7 +370,7 @@ pub enum JsValue { /// A constant primitive value. Constant(ConstantValue), /// An constant URL object. - Url(Url), + Url(Box), /// Some kind of well-known object /// (must not be an array, otherwise Array.concat needs to be changed) WellKnownObject(WellKnownObjectKind), diff --git a/crates/turbopack-ecmascript/src/analyzer/well_known.rs b/crates/turbopack-ecmascript/src/analyzer/well_known.rs index 9bf6061829a3a..04bd40eb43563 100644 --- a/crates/turbopack-ecmascript/src/analyzer/well_known.rs +++ b/crates/turbopack-ecmascript/src/analyzer/well_known.rs @@ -478,6 +478,7 @@ pub fn path_to_file_url(args: Vec) -> JsValue { if args.len() == 1 { if let Some(path) = args[0].as_str() { Url::from_file_path(path) + .map(Box::new) .map(JsValue::Url) .unwrap_or_else(|_| { JsValue::unknown( From b18ccae298b2d95ffc5c8a7de8b204227747f2c1 Mon Sep 17 00:00:00 2001 From: Tobias Koppers Date: Tue, 30 Apr 2024 14:55:49 +0200 Subject: [PATCH 2/6] Box Effects to reduce memmove and overallocation --- crates/turbopack-ecmascript/src/analyzer/graph.rs | 9 +++++---- crates/turbopack-ecmascript/src/analyzer/mod.rs | 4 ++-- crates/turbopack-ecmascript/src/references/mod.rs | 10 +++++----- 3 files changed, 12 insertions(+), 11 deletions(-) diff --git a/crates/turbopack-ecmascript/src/analyzer/graph.rs b/crates/turbopack-ecmascript/src/analyzer/graph.rs index dd5e634b41601..6567d9cfd8346 100644 --- a/crates/turbopack-ecmascript/src/analyzer/graph.rs +++ b/crates/turbopack-ecmascript/src/analyzer/graph.rs @@ -20,7 +20,7 @@ use crate::{analyzer::is_unresolved, utils::unparen}; #[derive(Debug, Clone, Default)] pub struct EffectsBlock { - pub effects: Vec, + pub effects: Vec>, /// The ast path to the block or expression. pub ast_path: Vec, } @@ -219,7 +219,7 @@ impl Effect { pub struct VarGraph { pub values: HashMap, - pub effects: Vec, + pub effects: Vec>, } impl VarGraph { @@ -647,7 +647,7 @@ impl EvalContext { struct Analyzer<'a> { data: &'a mut VarGraph, - effects: Vec, + effects: Vec>, eval_context: &'a EvalContext, @@ -711,8 +711,9 @@ impl Analyzer<'_> { self.add_value(id, value); } + #[inline] fn add_effect(&mut self, effect: Effect) { - self.effects.push(effect); + self.effects.push(Box::new(effect)); } fn check_iife<'ast: 'r, 'r>( diff --git a/crates/turbopack-ecmascript/src/analyzer/mod.rs b/crates/turbopack-ecmascript/src/analyzer/mod.rs index dbd26db7b8f5a..4c7e85d4e86f2 100644 --- a/crates/turbopack-ecmascript/src/analyzer/mod.rs +++ b/crates/turbopack-ecmascript/src/analyzer/mod.rs @@ -3681,7 +3681,7 @@ mod tests { let start = Instant::now(); async fn handle_args( args: Vec, - queue: &mut Vec<(usize, Effect)>, + queue: &mut Vec<(usize, Box)>, var_graph: &VarGraph, i: usize, ) -> Vec { @@ -3704,7 +3704,7 @@ mod tests { } new_args } - match effect { + match *effect { Effect::Conditional { condition, kind, .. } => { diff --git a/crates/turbopack-ecmascript/src/references/mod.rs b/crates/turbopack-ecmascript/src/references/mod.rs index 37ec2c7953cb7..a4468cce7b203 100644 --- a/crates/turbopack-ecmascript/src/references/mod.rs +++ b/crates/turbopack-ecmascript/src/references/mod.rs @@ -789,7 +789,7 @@ pub(crate) async fn analyse_ecmascript_module_internal( }; enum Action { - Effect(Effect), + Effect(Box), LeaveScope(u32), } @@ -811,13 +811,13 @@ pub(crate) async fn analyse_ecmascript_module_internal( Action::Effect(effect) => effect, }; - let add_effects = |effects: Vec| { + let add_effects = |effects: Vec>| { queue_stack .lock() .extend(effects.into_iter().map(Action::Effect).rev()) }; - match effect { + match *effect { Effect::Conditional { condition, kind, @@ -1106,7 +1106,7 @@ pub(crate) async fn analyse_ecmascript_module_internal( .await } -fn handle_call_boxed<'a, G: Fn(Vec) + Send + Sync + 'a>( +fn handle_call_boxed<'a, G: Fn(Vec>) + Send + Sync + 'a>( ast_path: &'a [AstParentKind], span: Span, func: JsValue, @@ -1130,7 +1130,7 @@ fn handle_call_boxed<'a, G: Fn(Vec) + Send + Sync + 'a>( )) } -async fn handle_call) + Send + Sync>( +async fn handle_call>) + Send + Sync>( ast_path: &[AstParentKind], span: Span, func: JsValue, From 74228271768dd310fefc44b46048020c8ea46be4 Mon Sep 17 00:00:00 2001 From: Tobias Koppers Date: Tue, 30 Apr 2024 21:05:22 +0200 Subject: [PATCH 3/6] clippy --- crates/turbopack-ecmascript/src/analyzer/graph.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/crates/turbopack-ecmascript/src/analyzer/graph.rs b/crates/turbopack-ecmascript/src/analyzer/graph.rs index 6567d9cfd8346..44b8e5560957e 100644 --- a/crates/turbopack-ecmascript/src/analyzer/graph.rs +++ b/crates/turbopack-ecmascript/src/analyzer/graph.rs @@ -647,6 +647,7 @@ impl EvalContext { struct Analyzer<'a> { data: &'a mut VarGraph, + #[allow(clippy::vec_box)] effects: Vec>, eval_context: &'a EvalContext, @@ -713,7 +714,7 @@ impl Analyzer<'_> { #[inline] fn add_effect(&mut self, effect: Effect) { - self.effects.push(Box::new(effect)); + self.effects.push(effect); } fn check_iife<'ast: 'r, 'r>( From 537e108b676f5292f0d37d29e4aee6f9e497bdeb Mon Sep 17 00:00:00 2001 From: Tobias Koppers Date: Thu, 2 May 2024 17:05:30 +0200 Subject: [PATCH 4/6] revert double box --- crates/turbopack-ecmascript/src/analyzer/graph.rs | 7 +++---- crates/turbopack-ecmascript/src/references/mod.rs | 13 +++++++------ 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/crates/turbopack-ecmascript/src/analyzer/graph.rs b/crates/turbopack-ecmascript/src/analyzer/graph.rs index 44b8e5560957e..4ef9786627668 100644 --- a/crates/turbopack-ecmascript/src/analyzer/graph.rs +++ b/crates/turbopack-ecmascript/src/analyzer/graph.rs @@ -20,7 +20,7 @@ use crate::{analyzer::is_unresolved, utils::unparen}; #[derive(Debug, Clone, Default)] pub struct EffectsBlock { - pub effects: Vec>, + pub effects: Vec, /// The ast path to the block or expression. pub ast_path: Vec, } @@ -219,7 +219,7 @@ impl Effect { pub struct VarGraph { pub values: HashMap, - pub effects: Vec>, + pub effects: Vec, } impl VarGraph { @@ -647,8 +647,7 @@ impl EvalContext { struct Analyzer<'a> { data: &'a mut VarGraph, - #[allow(clippy::vec_box)] - effects: Vec>, + effects: Vec, eval_context: &'a EvalContext, diff --git a/crates/turbopack-ecmascript/src/references/mod.rs b/crates/turbopack-ecmascript/src/references/mod.rs index a4468cce7b203..06ef941939c0b 100644 --- a/crates/turbopack-ecmascript/src/references/mod.rs +++ b/crates/turbopack-ecmascript/src/references/mod.rs @@ -789,7 +789,7 @@ pub(crate) async fn analyse_ecmascript_module_internal( }; enum Action { - Effect(Box), + Effect(Effect), LeaveScope(u32), } @@ -797,7 +797,8 @@ pub(crate) async fn analyse_ecmascript_module_internal( // of an effect we might want to add more effects into the middle of the // processing. Using a stack where effects are appended in reverse // order allows us to do that. It's recursion implemented as Stack. - let mut queue_stack = Mutex::new(Vec::new()); + let mut queue_stack: parking_lot::lock_api::Mutex> = + Mutex::new(Vec::new()); queue_stack .get_mut() .extend(effects.into_iter().map(Action::Effect).rev()); @@ -811,13 +812,13 @@ pub(crate) async fn analyse_ecmascript_module_internal( Action::Effect(effect) => effect, }; - let add_effects = |effects: Vec>| { + let add_effects = |effects: Vec| { queue_stack .lock() .extend(effects.into_iter().map(Action::Effect).rev()) }; - match *effect { + match effect { Effect::Conditional { condition, kind, @@ -1106,7 +1107,7 @@ pub(crate) async fn analyse_ecmascript_module_internal( .await } -fn handle_call_boxed<'a, G: Fn(Vec>) + Send + Sync + 'a>( +fn handle_call_boxed<'a, G: Fn(Vec) + Send + Sync + 'a>( ast_path: &'a [AstParentKind], span: Span, func: JsValue, @@ -1130,7 +1131,7 @@ fn handle_call_boxed<'a, G: Fn(Vec>) + Send + Sync + 'a>( )) } -async fn handle_call>) + Send + Sync>( +async fn handle_call) + Send + Sync>( ast_path: &[AstParentKind], span: Span, func: JsValue, From 10858cebc48e9d6e17a3390ae32030cad3925ae0 Mon Sep 17 00:00:00 2001 From: Tobias Koppers Date: Tue, 7 May 2024 08:03:37 +0200 Subject: [PATCH 5/6] revert unnecessary changes --- crates/turbopack-ecmascript/src/analyzer/graph.rs | 1 - crates/turbopack-ecmascript/src/references/mod.rs | 3 +-- 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/crates/turbopack-ecmascript/src/analyzer/graph.rs b/crates/turbopack-ecmascript/src/analyzer/graph.rs index 4ef9786627668..dd5e634b41601 100644 --- a/crates/turbopack-ecmascript/src/analyzer/graph.rs +++ b/crates/turbopack-ecmascript/src/analyzer/graph.rs @@ -711,7 +711,6 @@ impl Analyzer<'_> { self.add_value(id, value); } - #[inline] fn add_effect(&mut self, effect: Effect) { self.effects.push(effect); } diff --git a/crates/turbopack-ecmascript/src/references/mod.rs b/crates/turbopack-ecmascript/src/references/mod.rs index 06ef941939c0b..37ec2c7953cb7 100644 --- a/crates/turbopack-ecmascript/src/references/mod.rs +++ b/crates/turbopack-ecmascript/src/references/mod.rs @@ -797,8 +797,7 @@ pub(crate) async fn analyse_ecmascript_module_internal( // of an effect we might want to add more effects into the middle of the // processing. Using a stack where effects are appended in reverse // order allows us to do that. It's recursion implemented as Stack. - let mut queue_stack: parking_lot::lock_api::Mutex> = - Mutex::new(Vec::new()); + let mut queue_stack = Mutex::new(Vec::new()); queue_stack .get_mut() .extend(effects.into_iter().map(Action::Effect).rev()); From cf3d3464fb4a9adb53b7af41d9597994a136343f Mon Sep 17 00:00:00 2001 From: Tobias Koppers Date: Tue, 7 May 2024 08:04:43 +0200 Subject: [PATCH 6/6] revert box changes --- crates/turbopack-ecmascript/src/analyzer/mod.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/turbopack-ecmascript/src/analyzer/mod.rs b/crates/turbopack-ecmascript/src/analyzer/mod.rs index 4c7e85d4e86f2..dbd26db7b8f5a 100644 --- a/crates/turbopack-ecmascript/src/analyzer/mod.rs +++ b/crates/turbopack-ecmascript/src/analyzer/mod.rs @@ -3681,7 +3681,7 @@ mod tests { let start = Instant::now(); async fn handle_args( args: Vec, - queue: &mut Vec<(usize, Box)>, + queue: &mut Vec<(usize, Effect)>, var_graph: &VarGraph, i: usize, ) -> Vec { @@ -3704,7 +3704,7 @@ mod tests { } new_args } - match *effect { + match effect { Effect::Conditional { condition, kind, .. } => {