Skip to content

Commit 6e5632f

Browse files
authored
perf(es/minifier): Speed up merge_sequences_in_exprs by caching computation (#9843)
## Problem In https://github.com/chenjiahan/rspack-swc-minify-test: Rspack + SWC minify: 15.89 s Rspack + esbuild minify: 1.74 s This is caused by `merge_sequences_in_exprs` which tries to merge every pair of exprs with time complexity of O(n^2). In the given example, a vec of 5003 exprs (react component declarations and other declarations) are passed to this function. From the profiling result below we can see `visit_children_with` takes a lot of times, which is called by `IdentUsageFinder` and `idents_used_by` to check whether an ident is used in an ast node. However, in the O(n^2) loops, this part of the result is heavily recalculated. Example: ```js // input.js export function source() { let c = 0, a = 1; c += a, a += 5; let b = c; console.log(a, b, c); } // esbuild/terser output export function source(){let o=0,e=1;o+=e,e+=5,console.log(e,o,o)} // swc output export function source(){console.log(6,1,1)} ``` ## Solution To be honest, I don't come up with a better algorithm to reduce time complexity or pruning. But caching part of the computation does work very well in this bad case. Anyway, I'm proposing this pr first. ## Result **Before: 10128ms** <img width="1246" alt="image" src="https://github.com/user-attachments/assets/43d38775-e487-4f5a-a95c-8331ee85f109" /> **After: 1013ms(~10x)** <img width="1207" alt="image" src="https://github.com/user-attachments/assets/1b0d2852-343f-4bf6-8140-2b9f2d2772a4" />
1 parent 925695f commit 6e5632f

File tree

2 files changed

+99
-48
lines changed

2 files changed

+99
-48
lines changed

Diff for: .changeset/forty-maps-smoke.md

+6
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
---
2+
swc_core: patch
3+
swc_ecma_minifier: patch
4+
---
5+
6+
perf(es/minifier): Speed up `merge_sequences_in_exprs` by caching computation

Diff for: crates/swc_ecma_minifier/src/compress/optimize/sequences.rs

+93-48
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,14 @@
11
use std::{iter::once, mem::take};
22

3+
use rustc_hash::FxHashSet;
34
use swc_common::{pass::Either, util::take::Take, Spanned, DUMMY_SP};
45
use swc_ecma_ast::*;
56
use swc_ecma_usage_analyzer::{
67
alias::{collect_infects_from, AccessKind, AliasConfig},
78
util::is_global_var_with_pure_property_access,
89
};
910
use swc_ecma_utils::{
10-
contains_arguments, contains_this_expr, prepend_stmts, ExprExt, IdentUsageFinder, StmtLike,
11-
Type, Value,
11+
contains_arguments, contains_this_expr, prepend_stmts, ExprExt, StmtLike, Type, Value,
1212
};
1313
use swc_ecma_visit::{noop_visit_type, Visit, VisitWith};
1414
#[cfg(feature = "debug")]
@@ -23,7 +23,10 @@ use crate::{
2323
util::{is_directive, is_ident_used_by, replace_expr},
2424
},
2525
option::CompressOptions,
26-
util::{idents_used_by, idents_used_by_ignoring_nested, ExprOptExt, ModuleItemExt},
26+
util::{
27+
idents_used_by, idents_used_by_ignoring_nested, ExprOptExt, IdentUsageCollector,
28+
ModuleItemExt,
29+
},
2730
};
2831

2932
/// Methods related to the option `sequences`. All methods are noop if
@@ -821,22 +824,17 @@ impl Optimizer<'_> {
821824
)
822825
};
823826

827+
let mut merge_seq_cache = MergeSequenceCache::new(exprs.len());
824828
loop {
825-
let mut did_work = false;
826-
827-
for idx in 0..exprs.len() {
828-
for j in idx..exprs.len() {
829-
let (a1, a2) = exprs.split_at_mut(idx);
830-
831-
if a1.is_empty() || a2.is_empty() {
832-
break;
833-
}
834-
829+
let mut changed = false;
830+
for a_idx in 0..exprs.len().saturating_sub(1) {
831+
for b_idx in (a_idx + 1)..exprs.len() {
832+
let (a1, a2) = exprs.split_at_mut(a_idx + 1);
835833
let a = a1.last_mut().unwrap();
834+
let b = &mut a2[b_idx - a_idx - 1];
836835

837836
if self.options.unused && self.options.sequences() {
838-
if let (Mergable::Var(av), Mergable::Var(bv)) = (&mut *a, &mut a2[j - idx])
839-
{
837+
if let (Mergable::Var(av), Mergable::Var(bv)) = (&mut *a, &mut *b) {
840838
// We try dropping variable assignments first.
841839

842840
// Currently, we only drop variable declarations if they have the same
@@ -847,7 +845,7 @@ impl Optimizer<'_> {
847845

848846
match bv.init.as_deref_mut() {
849847
Some(b_init) => {
850-
if IdentUsageFinder::find(&an.to_id(), b_init) {
848+
if is_ident_used_by(an.to_id(), b_init) {
851849
log_abort!(
852850
"We can't duplicated binding because \
853851
initializer uses the previous declaration of \
@@ -859,14 +857,16 @@ impl Optimizer<'_> {
859857
if let Some(a_init) = av.init.take() {
860858
let b_seq = b_init.force_seq();
861859
b_seq.exprs.insert(0, a_init);
860+
merge_seq_cache.invalidate(a_idx);
861+
merge_seq_cache.invalidate(b_idx);
862862

863863
self.changed = true;
864864
report_change!(
865865
"Moving initializer sequentially as they have \
866866
a same name"
867867
);
868868
av.name.take();
869-
continue;
869+
break;
870870
} else {
871871
self.changed = true;
872872
report_change!(
@@ -875,7 +875,7 @@ impl Optimizer<'_> {
875875
an.id
876876
);
877877
av.name.take();
878-
continue;
878+
break;
879879
}
880880
}
881881
None => {
@@ -890,13 +890,15 @@ impl Optimizer<'_> {
890890
//
891891
// prints 5
892892
bv.init = av.init.take();
893+
merge_seq_cache.invalidate(a_idx);
894+
merge_seq_cache.invalidate(b_idx);
893895
self.changed = true;
894896
report_change!(
895897
"Moving initializer to the next variable \
896898
declaration as they have the same name"
897899
);
898900
av.name.take();
899-
continue;
901+
break;
900902
}
901903
}
902904
}
@@ -906,26 +908,36 @@ impl Optimizer<'_> {
906908

907909
// Merge sequentially
908910

909-
match &mut a2[j - idx] {
911+
match b {
910912
Mergable::Var(b) => match b.init.as_deref_mut() {
911913
Some(b) => {
912-
if self.merge_sequential_expr(a, b)? {
913-
did_work = true;
914+
if !merge_seq_cache.is_top_retain(self, a, a_idx)
915+
&& self.merge_sequential_expr(a, b)?
916+
{
917+
changed = true;
918+
merge_seq_cache.invalidate(a_idx);
919+
merge_seq_cache.invalidate(b_idx);
914920
break;
915921
}
916922
}
917923
None => continue,
918924
},
919925
Mergable::Expr(b) => {
920-
if self.merge_sequential_expr(a, b)? {
921-
did_work = true;
926+
if !merge_seq_cache.is_top_retain(self, a, a_idx)
927+
&& self.merge_sequential_expr(a, b)?
928+
{
929+
changed = true;
930+
merge_seq_cache.invalidate(a_idx);
931+
merge_seq_cache.invalidate(b_idx);
922932
break;
923933
}
924934
}
925935
Mergable::FnDecl(..) => continue,
926936
Mergable::Drop => {
927937
if self.drop_mergable_seq(a)? {
928-
did_work = true;
938+
changed = true;
939+
merge_seq_cache.invalidate(a_idx);
940+
merge_seq_cache.invalidate(b_idx);
929941
break;
930942
}
931943
}
@@ -972,16 +984,16 @@ impl Optimizer<'_> {
972984
_ => {}
973985
}
974986

975-
match &a2[j - idx] {
987+
match b {
976988
Mergable::Var(e2) => {
977989
if let Some(e2) = &e2.init {
978990
if !self.is_skippable_for_seq(Some(a), e2) {
979991
break;
980992
}
981993
}
982994

983-
if let Some(id) = a1.last_mut().unwrap().id() {
984-
if IdentUsageFinder::find(&id, &**e2) {
995+
if let Some(id) = a.id() {
996+
if merge_seq_cache.is_ident_used_by(&id, &**e2, b_idx) {
985997
break;
986998
}
987999
}
@@ -991,9 +1003,8 @@ impl Optimizer<'_> {
9911003
break;
9921004
}
9931005

994-
if let Some(id) = a1.last_mut().unwrap().id() {
995-
// TODO(kdy1): Optimize
996-
if IdentUsageFinder::find(&id, &**e2) {
1006+
if let Some(id) = a.id() {
1007+
if merge_seq_cache.is_ident_used_by(&id, &**e2, b_idx) {
9971008
break;
9981009
}
9991010
}
@@ -1019,7 +1030,7 @@ impl Optimizer<'_> {
10191030
}
10201031
}
10211032

1022-
if !did_work {
1033+
if !changed {
10231034
break;
10241035
}
10251036
}
@@ -1527,10 +1538,6 @@ impl Optimizer<'_> {
15271538
///
15281539
/// Returns [Err] iff we should stop checking.
15291540
fn merge_sequential_expr(&mut self, a: &mut Mergable, b: &mut Expr) -> Result<bool, ()> {
1530-
if let Mergable::Drop = a {
1531-
return Ok(false);
1532-
}
1533-
15341541
#[cfg(feature = "debug")]
15351542
let _tracing = {
15361543
let b_str = dump(&*b, false);
@@ -1552,16 +1559,6 @@ impl Optimizer<'_> {
15521559
)
15531560
};
15541561

1555-
// Respect top_retain
1556-
if let Some(a_id) = a.id() {
1557-
if a_id.0 == "arguments"
1558-
|| (matches!(a, Mergable::Var(_) | Mergable::FnDecl(_))
1559-
&& !self.may_remove_ident(&Ident::from(a_id)))
1560-
{
1561-
return Ok(false);
1562-
}
1563-
}
1564-
15651562
match &*b {
15661563
Expr::Arrow(..)
15671564
| Expr::Fn(..)
@@ -1801,7 +1798,7 @@ impl Optimizer<'_> {
18011798

18021799
if !self.is_skippable_for_seq(Some(a), &b_left.id.clone().into()) {
18031800
// Let's be safe
1804-
if IdentUsageFinder::find(&b_left.to_id(), &b_assign.right) {
1801+
if is_ident_used_by(b_left.to_id(), &b_assign.right) {
18051802
return Ok(false);
18061803
}
18071804

@@ -1817,7 +1814,7 @@ impl Optimizer<'_> {
18171814
return Ok(false);
18181815
}
18191816

1820-
if IdentUsageFinder::find(&b_left.to_id(), &b_assign.right) {
1817+
if is_ident_used_by(b_left.to_id(), &b_assign.right) {
18211818
return Err(());
18221819
}
18231820

@@ -2710,6 +2707,54 @@ impl Mergable<'_> {
27102707
}
27112708
}
27122709

2710+
#[derive(Debug, Default)]
2711+
struct MergeSequenceCache {
2712+
ident_usage_cache: Vec<Option<FxHashSet<Id>>>,
2713+
top_retain_cache: Vec<Option<bool>>,
2714+
}
2715+
2716+
impl MergeSequenceCache {
2717+
fn new(cap: usize) -> Self {
2718+
Self {
2719+
ident_usage_cache: vec![None; cap],
2720+
top_retain_cache: vec![None; cap],
2721+
}
2722+
}
2723+
2724+
fn is_ident_used_by<N: VisitWith<IdentUsageCollector>>(
2725+
&mut self,
2726+
ident: &Id,
2727+
node: &N,
2728+
node_id: usize,
2729+
) -> bool {
2730+
let idents = self.ident_usage_cache[node_id].get_or_insert_with(|| idents_used_by(node));
2731+
idents.contains(ident)
2732+
}
2733+
2734+
fn invalidate(&mut self, node_id: usize) {
2735+
self.ident_usage_cache[node_id] = None;
2736+
}
2737+
2738+
fn is_top_retain(&mut self, optimizer: &Optimizer, a: &Mergable, node_id: usize) -> bool {
2739+
*self.top_retain_cache[node_id].get_or_insert_with(|| {
2740+
if let Mergable::Drop = a {
2741+
return true;
2742+
}
2743+
2744+
if let Some(a_id) = a.id() {
2745+
if a_id.0 == "arguments"
2746+
|| (matches!(a, Mergable::Var(_) | Mergable::FnDecl(_))
2747+
&& !optimizer.may_remove_ident(&Ident::from(a_id)))
2748+
{
2749+
return true;
2750+
}
2751+
}
2752+
2753+
false
2754+
})
2755+
}
2756+
}
2757+
27132758
/// Returns true for trivial bool/numeric literals
27142759
pub(crate) fn is_trivial_lit(e: &Expr) -> bool {
27152760
match e {

0 commit comments

Comments
 (0)