From 76fe139334b64c9ba62a98dc5319523d21d633f4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Donny/=EA=B0=95=EB=8F=99=EC=9C=A4?= Date: Sat, 20 Jul 2024 10:26:10 +0900 Subject: [PATCH] perf(es/minifier): Pre-allocate collections (#9289) **Related issue:** - Inspiration: https://github.com/oxc-project/oxc/pull/4328 --- Cargo.lock | 1 + crates/swc_allocator/src/collections.rs | 3 ++ crates/swc_ecma_minifier/Cargo.toml | 2 + crates/swc_ecma_minifier/benches/full.rs | 4 ++ .../src/compress/optimize/iife.rs | 2 +- .../src/compress/optimize/sequences.rs | 46 ++++++++++--------- .../src/compress/optimize/util.rs | 8 ++++ crates/swc_ecma_minifier/src/lib.rs | 1 + crates/swc_ecma_minifier/src/size_hint.rs | 2 + .../src/rename/analyzer/mod.rs | 21 +++++++++ .../src/rename/analyzer/scope.rs | 10 ++++ .../src/rename/collector.rs | 8 +++- .../src/resolver/mod.rs | 42 ++++++++--------- .../src/debug.rs | 1 + crates/swc_ecma_usage_analyzer/src/lib.rs | 4 ++ xtask/src/bench.rs | 24 ++++++++-- 16 files changed, 127 insertions(+), 52 deletions(-) create mode 100644 crates/swc_ecma_minifier/src/size_hint.rs diff --git a/Cargo.lock b/Cargo.lock index 6d7422dd248b..067d5eb8eace 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4481,6 +4481,7 @@ dependencies = [ "ryu-js", "serde", "serde_json", + "swc_allocator", "swc_atoms", "swc_common", "swc_config", diff --git a/crates/swc_allocator/src/collections.rs b/crates/swc_allocator/src/collections.rs index 2a83e55b2486..fd7e5732b0ed 100644 --- a/crates/swc_allocator/src/collections.rs +++ b/crates/swc_allocator/src/collections.rs @@ -31,3 +31,6 @@ pub type FxHashMap = HashMap; /// Faster `HashSet` which uses `FxHasher`. pub type FxHashSet = HashSet; + +/// Re-export for convenience. +pub use hashbrown::hash_map; diff --git a/crates/swc_ecma_minifier/Cargo.toml b/crates/swc_ecma_minifier/Cargo.toml index f38af9a139f3..0d1bcd9bdfe1 100644 --- a/crates/swc_ecma_minifier/Cargo.toml +++ b/crates/swc_ecma_minifier/Cargo.toml @@ -51,6 +51,7 @@ serde = { workspace = true, features = ["derive"] } serde_json = { workspace = true } tracing = { workspace = true } +swc_allocator = { version = "0.1.7", path = "../swc_allocator", default-features = false } swc_atoms = { version = "0.6.5", path = "../swc_atoms" } swc_common = { version = "0.36.0", path = "../swc_common" } swc_config = { version = "0.1.13", path = "../swc_config", features = [ @@ -76,6 +77,7 @@ pretty_assertions = { workspace = true } walkdir = { workspace = true } codspeed-criterion-compat = { workspace = true } +swc_allocator = { version = "0.1.7", path = "../swc_allocator" } swc_ecma_testing = { version = "0.25.0", path = "../swc_ecma_testing" } swc_malloc = { version = "0.5.10", path = "../swc_malloc" } testing = { version = "0.38.0", path = "../testing" } diff --git a/crates/swc_ecma_minifier/benches/full.rs b/crates/swc_ecma_minifier/benches/full.rs index 5f5f3244fa60..86c58d31744b 100644 --- a/crates/swc_ecma_minifier/benches/full.rs +++ b/crates/swc_ecma_minifier/benches/full.rs @@ -5,6 +5,7 @@ extern crate swc_malloc; use std::fs::read_to_string; use codspeed_criterion_compat::{black_box, criterion_group, criterion_main, Criterion}; +use swc_allocator::Allocator; use swc_common::{errors::HANDLER, sync::Lrc, FileName, Mark, SourceMap}; use swc_ecma_codegen::text_writer::JsWriter; use swc_ecma_minifier::{ @@ -25,6 +26,9 @@ pub fn bench_files(c: &mut Criterion) { group.bench_function(&format!("es/minifier/libs/{}", name), |b| { b.iter(|| { // We benchmark full time, including time for creating cm, handler + let allocator = Allocator::default(); + let _guard = unsafe { allocator.guard() }; + run(&src) }) }); diff --git a/crates/swc_ecma_minifier/src/compress/optimize/iife.rs b/crates/swc_ecma_minifier/src/compress/optimize/iife.rs index 7666322c06e1..480b7639a472 100644 --- a/crates/swc_ecma_minifier/src/compress/optimize/iife.rs +++ b/crates/swc_ecma_minifier/src/compress/optimize/iife.rs @@ -863,7 +863,7 @@ impl Optimizer<'_> { args: &mut [ExprOrSpread], exprs: &mut Vec>, ) -> Vec { - let mut vars = Vec::new(); + let mut vars = Vec::with_capacity(params.len()); for (idx, param) in params.iter().enumerate() { let arg = args.get_mut(idx).map(|arg| arg.expr.take()); diff --git a/crates/swc_ecma_minifier/src/compress/optimize/sequences.rs b/crates/swc_ecma_minifier/src/compress/optimize/sequences.rs index 4d46aa950eea..a4ee884ef8ef 100644 --- a/crates/swc_ecma_minifier/src/compress/optimize/sequences.rs +++ b/crates/swc_ecma_minifier/src/compress/optimize/sequences.rs @@ -1,6 +1,6 @@ -use std::mem::take; +use std::{iter::once, mem::take}; -use swc_common::{util::take::Take, Spanned, DUMMY_SP}; +use swc_common::{pass::Either, util::take::Take, Spanned, DUMMY_SP}; use swc_ecma_ast::*; use swc_ecma_usage_analyzer::{ alias::{collect_infects_from, AccessKind, AliasConfig}, @@ -565,61 +565,59 @@ impl Optimizer<'_> { &mut self, s: &'a mut Stmt, options: &CompressOptions, - ) -> Option>> { + ) -> Option>, std::iter::Once>>> { Some(match s { Stmt::Expr(e) => { if self.options.sequences() || self.options.collapse_vars || self.options.side_effects { - vec![Mergable::Expr(&mut e.expr)] + Either::Right(once(Mergable::Expr(&mut e.expr))) } else { return None; } } Stmt::Decl(Decl::Var(v)) => { if options.reduce_vars || options.collapse_vars { - v.decls.iter_mut().map(Mergable::Var).collect() + Either::Left(v.decls.iter_mut().map(Mergable::Var)) } else { return None; } } Stmt::Return(ReturnStmt { arg: Some(arg), .. }) => { - vec![Mergable::Expr(arg)] + Either::Right(once(Mergable::Expr(arg))) } - Stmt::If(s) if options.sequences() => { - vec![Mergable::Expr(&mut s.test)] - } + Stmt::If(s) if options.sequences() => Either::Right(once(Mergable::Expr(&mut s.test))), Stmt::Switch(s) if options.sequences() => { - vec![Mergable::Expr(&mut s.discriminant)] + Either::Right(once(Mergable::Expr(&mut s.discriminant))) } Stmt::For(s) if options.sequences() => { if let Some(VarDeclOrExpr::Expr(e)) = &mut s.init { - vec![Mergable::Expr(e)] + Either::Right(once(Mergable::Expr(e))) } else { return None; } } Stmt::ForOf(s) if options.sequences() => { - vec![Mergable::Expr(&mut s.right)] + Either::Right(once(Mergable::Expr(&mut s.right))) } Stmt::ForIn(s) if options.sequences() => { - vec![Mergable::Expr(&mut s.right)] + Either::Right(once(Mergable::Expr(&mut s.right))) } Stmt::Throw(s) if options.sequences() => { - vec![Mergable::Expr(&mut s.arg)] + Either::Right(once(Mergable::Expr(&mut s.arg))) } Stmt::Decl(Decl::Fn(f)) => { // Check for side effects - vec![Mergable::FnDecl(f)] + Either::Right(once(Mergable::FnDecl(f))) } _ => return None, @@ -646,12 +644,9 @@ impl Optimizer<'_> { return; } - let mut exprs = Vec::new(); - let mut buf = Vec::new(); - - for stmt in stmts.iter_mut() { - let is_end = matches!( - stmt.as_stmt(), + fn is_end(s: Option<&Stmt>) -> bool { + matches!( + s, Some( Stmt::If(..) | Stmt::Throw(..) @@ -661,7 +656,14 @@ impl Optimizer<'_> { | Stmt::ForIn(..) | Stmt::ForOf(..) ) | None - ); + ) + } + + let mut exprs = Vec::new(); + let mut buf = Vec::new(); + + for stmt in stmts.iter_mut() { + let is_end = is_end(stmt.as_stmt()); let can_skip = match stmt.as_stmt() { Some(Stmt::Decl(Decl::Fn(..))) => true, _ => false, diff --git a/crates/swc_ecma_minifier/src/compress/optimize/util.rs b/crates/swc_ecma_minifier/src/compress/optimize/util.rs index 0b4ef1a72284..e86eb866ceea 100644 --- a/crates/swc_ecma_minifier/src/compress/optimize/util.rs +++ b/crates/swc_ecma_minifier/src/compress/optimize/util.rs @@ -533,6 +533,14 @@ impl VisitMut for NormalMultiReplacer<'_> { } } } + + fn visit_mut_stmt(&mut self, node: &mut Stmt) { + if self.vars.is_empty() { + return; + } + + node.visit_mut_children_with(self); + } } pub(crate) fn replace_id_with_expr(node: &mut N, from: Id, to: Box) -> Option> diff --git a/crates/swc_ecma_minifier/src/lib.rs b/crates/swc_ecma_minifier/src/lib.rs index 30874cce1b83..54b9ea7d2334 100644 --- a/crates/swc_ecma_minifier/src/lib.rs +++ b/crates/swc_ecma_minifier/src/lib.rs @@ -77,6 +77,7 @@ mod mode; pub mod option; mod pass; mod program_data; +mod size_hint; pub mod timing; mod util; diff --git a/crates/swc_ecma_minifier/src/size_hint.rs b/crates/swc_ecma_minifier/src/size_hint.rs new file mode 100644 index 000000000000..55cb4cbca680 --- /dev/null +++ b/crates/swc_ecma_minifier/src/size_hint.rs @@ -0,0 +1,2 @@ +#[derive(Debug, Default, Clone, Copy)] +pub struct SizeHint {} diff --git a/crates/swc_ecma_transforms_base/src/rename/analyzer/mod.rs b/crates/swc_ecma_transforms_base/src/rename/analyzer/mod.rs index 671daec1c9df..4cbcec8baef9 100644 --- a/crates/swc_ecma_transforms_base/src/rename/analyzer/mod.rs +++ b/crates/swc_ecma_transforms_base/src/rename/analyzer/mod.rs @@ -40,10 +40,29 @@ impl Analyzer { } } + fn reserve_decl(&mut self, len: usize, belong_to_fn_scope: bool) { + if belong_to_fn_scope { + match self.scope.kind { + ScopeKind::Fn => { + self.scope.reserve_decl(len); + } + ScopeKind::Block => { + self.hoisted_vars.reserve(len); + } + } + } else { + self.scope.reserve_decl(len); + } + } + fn add_usage(&mut self, id: Id) { self.scope.add_usage(id); } + fn reserve_usage(&mut self, len: usize) { + self.scope.reserve_usage(len); + } + fn with_scope(&mut self, kind: ScopeKind, op: F) where F: FnOnce(&mut Analyzer), @@ -66,6 +85,7 @@ impl Analyzer { op(&mut v); if !v.hoisted_vars.is_empty() { debug_assert!(matches!(v.scope.kind, ScopeKind::Block)); + self.reserve_usage(v.hoisted_vars.len()); v.hoisted_vars.clone().into_iter().for_each(|id| { // For variables declared in block scope using `var` and `function`, // We should create a fake usage in the block to prevent conflicted @@ -74,6 +94,7 @@ impl Analyzer { }); match self.scope.kind { ScopeKind::Fn => { + self.reserve_decl(v.hoisted_vars.len(), true); v.hoisted_vars .into_iter() .for_each(|id| self.add_decl(id, true)); diff --git a/crates/swc_ecma_transforms_base/src/rename/analyzer/scope.rs b/crates/swc_ecma_transforms_base/src/rename/analyzer/scope.rs index a94276f5294e..67fedeff1387 100644 --- a/crates/swc_ecma_transforms_base/src/rename/analyzer/scope.rs +++ b/crates/swc_ecma_transforms_base/src/rename/analyzer/scope.rs @@ -68,6 +68,12 @@ impl Scope { } } + pub(crate) fn reserve_decl(&mut self, len: usize) { + self.data.all.reserve(len); + + self.data.queue.reserve(len); + } + pub(super) fn add_usage(&mut self, id: Id) { if id.0 == "arguments" { return; @@ -76,6 +82,10 @@ impl Scope { self.data.all.insert(id); } + pub(crate) fn reserve_usage(&mut self, len: usize) { + self.data.all.reserve(len); + } + /// Copy `children.data.all` to `self.data.all`. pub(crate) fn prepare_renaming(&mut self) { self.children.iter_mut().for_each(|child| { diff --git a/crates/swc_ecma_transforms_base/src/rename/collector.rs b/crates/swc_ecma_transforms_base/src/rename/collector.rs index 7f4f0e527d71..46d1437e6493 100644 --- a/crates/swc_ecma_transforms_base/src/rename/collector.rs +++ b/crates/swc_ecma_transforms_base/src/rename/collector.rs @@ -21,7 +21,7 @@ impl Visit for IdCollector { fn visit_export_namespace_specifier(&mut self, _: &ExportNamespaceSpecifier) {} - fn visit_expr(&mut self, n: &Expr) { + fn visit_bin_expr(&mut self, n: &BinExpr) { maybe_grow_default(|| n.visit_children_with(self)); } @@ -135,10 +135,14 @@ where fn visit_expr(&mut self, node: &Expr) { let old = self.is_pat_decl; self.is_pat_decl = false; - maybe_grow_default(|| node.visit_children_with(self)); + node.visit_children_with(self); self.is_pat_decl = old; } + fn visit_bin_expr(&mut self, node: &BinExpr) { + maybe_grow_default(|| node.visit_children_with(self)); + } + fn visit_fn_decl(&mut self, node: &FnDecl) { node.visit_children_with(self); diff --git a/crates/swc_ecma_transforms_base/src/resolver/mod.rs b/crates/swc_ecma_transforms_base/src/resolver/mod.rs index 3ce1585723c3..2109d4141c1f 100644 --- a/crates/swc_ecma_transforms_base/src/resolver/mod.rs +++ b/crates/swc_ecma_transforms_base/src/resolver/mod.rs @@ -1913,10 +1913,9 @@ impl VisitMut for Hoister<'_, '_> { /// that there is already an global declaration of Ic when deal with the try /// block. fn visit_mut_module_items(&mut self, items: &mut Vec) { - let mut other_items = Vec::new(); - - for item in items { - match item { + let others = items + .iter_mut() + .filter_map(|item| match item { ModuleItem::Stmt(Stmt::Decl(Decl::Var(v))) | ModuleItem::ModuleDecl(ModuleDecl::ExportDecl(ExportDecl { decl: Decl::Var(v), @@ -1930,6 +1929,7 @@ impl VisitMut for Hoister<'_, '_> { ) => { item.visit_mut_with(self); + None } ModuleItem::Stmt(Stmt::Decl(Decl::Fn(..))) @@ -1938,37 +1938,35 @@ impl VisitMut for Hoister<'_, '_> { .. })) => { item.visit_mut_with(self); + None } - _ => { - other_items.push(item); - } - } - } + _ => Some(item), + }) + .collect::>(); - for other_item in other_items { - other_item.visit_mut_with(self); - } + others.into_iter().for_each(|item| { + item.visit_mut_with(self); + }); } /// see docs for `self.visit_mut_module_items` fn visit_mut_stmts(&mut self, stmts: &mut Vec) { - let mut other_stmts = Vec::new(); - - for item in stmts { - match item { + let others = stmts + .iter_mut() + .filter_map(|item| match item { Stmt::Decl(Decl::Var(..)) => { item.visit_mut_with(self); + None } Stmt::Decl(Decl::Fn(..)) => { item.visit_mut_with(self); + None } - _ => { - other_stmts.push(item); - } - } - } + _ => Some(item), + }) + .collect::>(); - for other_stmt in other_stmts { + for other_stmt in others { other_stmt.visit_mut_with(self); } } diff --git a/crates/swc_ecma_transforms_optimization/src/debug.rs b/crates/swc_ecma_transforms_optimization/src/debug.rs index 5cd2ea86c053..e5d56df9f0e4 100644 --- a/crates/swc_ecma_transforms_optimization/src/debug.rs +++ b/crates/swc_ecma_transforms_optimization/src/debug.rs @@ -6,6 +6,7 @@ use swc_ecma_ast::*; use swc_ecma_visit::{noop_visit_type, Visit, VisitWith}; /// Assert in debug mode. This is noop in release build. +#[cfg_attr(not(debug_assertions), inline(always))] pub fn debug_assert_valid(node: &N) where N: VisitWith, diff --git a/crates/swc_ecma_usage_analyzer/src/lib.rs b/crates/swc_ecma_usage_analyzer/src/lib.rs index b0f9b4eb9785..fe8a1ffdd109 100644 --- a/crates/swc_ecma_usage_analyzer/src/lib.rs +++ b/crates/swc_ecma_usage_analyzer/src/lib.rs @@ -1,3 +1,7 @@ +//! This crate is an internal crate that is extracted just to reduce compile +//! time. Do not use this crate directly, and this package does not follow +//! semver. + #![allow(clippy::mutable_key_type)] pub mod alias; diff --git a/xtask/src/bench.rs b/xtask/src/bench.rs index 851b9e42b6f1..6d51c3aae492 100644 --- a/xtask/src/bench.rs +++ b/xtask/src/bench.rs @@ -32,6 +32,10 @@ pub(super) struct BenchCmd { #[clap(long)] instrument: bool, + /// Instrument using https://github.com/mstange/samply + #[clap(long)] + samply: bool, + #[clap(long)] features: Vec, @@ -50,7 +54,17 @@ impl BenchCmd { } fn build_cmd(&self) -> Result { - let mut cmd = if self.instrument { + let mut cmd = if self.samply { + // ddt profile instruments cargo -t time + let mut cmd = Command::new("ddt"); + cmd.arg("profile").arg("samply").arg("cargo"); + + if !self.debug { + cmd.arg("--release"); + } + + cmd + } else if self.instrument { // ddt profile instruments cargo -t time let mut cmd = Command::new("ddt"); cmd.arg("profile").arg("instruments").arg("cargo"); @@ -61,9 +75,6 @@ impl BenchCmd { cmd.arg("--release"); } - // TODO: This should use cargo metadata - cmd.current_dir(repository_root()?.join("crates").join(&self.package)); - cmd } else { let mut cmd = Command::new("cargo"); @@ -90,12 +101,15 @@ impl BenchCmd { cmd.arg("--features").arg(f); } - if self.instrument { + if self.samply || self.instrument { cmd.arg("--").arg("--bench").args(&self.args); } else { cmd.arg("--").args(&self.args); } + // TODO: This should use cargo metadata + cmd.current_dir(repository_root()?.join("crates").join(&self.package)); + Ok(cmd) } }