Skip to content

Commit 03b75ab

Browse files
sapphi-redBoshen
authored andcommitted
feat(minifier): inline single use variables within the same variable declarations
1 parent 7e42b0a commit 03b75ab

File tree

5 files changed

+161
-69
lines changed

5 files changed

+161
-69
lines changed

crates/oxc_minifier/src/peephole/minimize_conditions.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1123,7 +1123,7 @@ mod test {
11231123

11241124
#[test]
11251125
fn test_coercion_substitution_boolean_result0() {
1126-
test_same("var x = {}, y = x != null;");
1126+
test("var x = {}, y = x != null;", "var y = !0;");
11271127
}
11281128

11291129
#[test]

crates/oxc_minifier/src/peephole/minimize_statements.rs

Lines changed: 113 additions & 57 deletions
Original file line numberDiff line numberDiff line change
@@ -394,6 +394,13 @@ impl<'a> PeepholeOptimizations {
394394
ctx.state.changed = true;
395395
}
396396
}
397+
if Self::substitute_single_use_symbol_within_declaration(
398+
var_decl.kind,
399+
&mut var_decl.declarations,
400+
ctx,
401+
) {
402+
ctx.state.changed = true;
403+
}
397404

398405
// If `join_vars` is off, but there are unused declarators ... just join them to make our code simpler.
399406
if !ctx.options().join_vars
@@ -850,6 +857,13 @@ impl<'a> PeepholeOptimizations {
850857
ctx.state.changed = true;
851858
}
852859
}
860+
if Self::substitute_single_use_symbol_within_declaration(
861+
var_decl.kind,
862+
&mut var_decl.declarations,
863+
ctx,
864+
) {
865+
ctx.state.changed = true;
866+
}
853867
}
854868
match_expression!(ForStatementInit) => {
855869
let init = init.to_expression_mut();
@@ -1123,67 +1137,109 @@ impl<'a> PeepholeOptimizations {
11231137
if prev_var_decl.kind.is_using() {
11241138
break;
11251139
}
1140+
let old_len = prev_var_decl.declarations.len();
1141+
let new_len = Self::substitute_single_use_symbol_in_expression_from_declarators(
1142+
expr_in_stmt,
1143+
&mut prev_var_decl.declarations,
1144+
ctx,
1145+
non_scoped_literal_only,
1146+
);
1147+
if new_len == 0 {
1148+
inlined = true;
1149+
stmts.pop();
1150+
} else if old_len != new_len {
1151+
inlined = true;
1152+
prev_var_decl.declarations.truncate(new_len);
1153+
break;
1154+
} else {
1155+
break;
1156+
}
1157+
}
1158+
inlined
1159+
}
11261160

1127-
let last_non_inlined_index =
1128-
prev_var_decl.declarations.iter_mut().rposition(|prev_decl| {
1129-
let Some(prev_decl_init) = &mut prev_decl.init else {
1130-
return true;
1131-
};
1132-
let BindingPatternKind::BindingIdentifier(prev_decl_id) = &prev_decl.id.kind
1133-
else {
1134-
return true;
1135-
};
1136-
if ctx.is_expression_whose_name_needs_to_be_kept(prev_decl_init) {
1137-
return true;
1138-
}
1139-
let Some(symbol_value) =
1140-
ctx.state.symbol_values.get_symbol_value(prev_decl_id.symbol_id())
1141-
else {
1142-
return true;
1143-
};
1144-
// we should check whether it's exported by `symbol_value.exported`
1145-
// because the variable might be exported with `export { foo }` rather than `export var foo`
1146-
if symbol_value.exported
1147-
|| symbol_value.read_references_count > 1
1148-
|| symbol_value.write_references_count > 0
1149-
{
1150-
return true;
1151-
}
1152-
if non_scoped_literal_only && !prev_decl_init.is_literal_value(false, ctx) {
1153-
return true;
1154-
}
1155-
let replaced = Self::substitute_single_use_symbol_in_expression(
1156-
expr_in_stmt,
1157-
&prev_decl_id.name,
1158-
prev_decl_init,
1159-
prev_decl_init.may_have_side_effects(ctx),
1160-
ctx,
1161-
);
1162-
if replaced != Some(true) {
1163-
return true;
1164-
}
1165-
false
1166-
});
1167-
match last_non_inlined_index {
1168-
None => {
1169-
// all inlined
1170-
stmts.pop();
1171-
inlined = true;
1172-
}
1173-
Some(last_non_inlined_index)
1174-
if last_non_inlined_index + 1 == prev_var_decl.declarations.len() =>
1175-
{
1176-
// no change
1177-
break;
1178-
}
1179-
Some(last_non_inlined_index) => {
1180-
prev_var_decl.declarations.truncate(last_non_inlined_index + 1);
1181-
inlined = true;
1182-
break;
1161+
fn substitute_single_use_symbol_within_declaration(
1162+
kind: VariableDeclarationKind,
1163+
declarations: &mut Vec<'a, VariableDeclarator<'a>>,
1164+
ctx: &Ctx<'a, '_>,
1165+
) -> bool {
1166+
// TODO: we should skip this compression when direct eval exists
1167+
// because the code inside eval may reference the variable
1168+
1169+
let mut changed = false;
1170+
if !Self::keep_top_level_var_in_script_mode(ctx) && !kind.is_using() {
1171+
let mut i = 1;
1172+
while i < declarations.len() {
1173+
let (prev_decls, [decl, ..]) = declarations.split_at_mut(i) else { unreachable!() };
1174+
let Some(decl_init) = &mut decl.init else {
1175+
i += 1;
1176+
continue;
1177+
};
1178+
let old_len = prev_decls.len();
1179+
let new_len = Self::substitute_single_use_symbol_in_expression_from_declarators(
1180+
decl_init, prev_decls, ctx, false,
1181+
);
1182+
if old_len != new_len {
1183+
changed = true;
1184+
let drop_count = old_len - new_len;
1185+
declarations.drain(i - drop_count..i);
1186+
i -= drop_count;
11831187
}
1188+
i += 1;
11841189
}
11851190
}
1186-
inlined
1191+
changed
1192+
}
1193+
1194+
/// Returns new length
1195+
fn substitute_single_use_symbol_in_expression_from_declarators(
1196+
target_expr: &mut Expression<'a>,
1197+
declarators: &mut [VariableDeclarator<'a>],
1198+
ctx: &Ctx<'a, '_>,
1199+
non_scoped_literal_only: bool,
1200+
) -> usize {
1201+
let last_non_inlined_index = declarators.iter_mut().rposition(|prev_decl| {
1202+
let Some(prev_decl_init) = &mut prev_decl.init else {
1203+
return true;
1204+
};
1205+
let BindingPatternKind::BindingIdentifier(prev_decl_id) = &prev_decl.id.kind else {
1206+
return true;
1207+
};
1208+
if ctx.is_expression_whose_name_needs_to_be_kept(prev_decl_init) {
1209+
return true;
1210+
}
1211+
let Some(symbol_value) =
1212+
ctx.state.symbol_values.get_symbol_value(prev_decl_id.symbol_id())
1213+
else {
1214+
return true;
1215+
};
1216+
// we should check whether it's exported by `symbol_value.exported`
1217+
// because the variable might be exported with `export { foo }` rather than `export var foo`
1218+
if symbol_value.exported
1219+
|| symbol_value.read_references_count > 1
1220+
|| symbol_value.write_references_count > 0
1221+
{
1222+
return true;
1223+
}
1224+
if non_scoped_literal_only && !prev_decl_init.is_literal_value(false, ctx) {
1225+
return true;
1226+
}
1227+
let replaced = Self::substitute_single_use_symbol_in_expression(
1228+
target_expr,
1229+
&prev_decl_id.name,
1230+
prev_decl_init,
1231+
prev_decl_init.may_have_side_effects(ctx),
1232+
ctx,
1233+
);
1234+
if replaced != Some(true) {
1235+
return true;
1236+
}
1237+
false
1238+
});
1239+
match last_non_inlined_index {
1240+
None => 0,
1241+
Some(last_non_inlined_index) => last_non_inlined_index + 1,
1242+
}
11871243
}
11881244

11891245
/// Returns Some(true) when the expression is successfully replaced.

crates/oxc_minifier/tests/peephole/inline_single_use_variable.rs

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -223,6 +223,42 @@ fn test_inline_single_use_variable() {
223223
);
224224
}
225225

226+
#[test]
227+
fn test_within_same_variable_declarations() {
228+
test_script(
229+
"var a = foo, b = a; for (; bar;) console.log(b)",
230+
"for (var a = foo, b = a; bar;) console.log(b)",
231+
);
232+
test(
233+
"function wrapper() { var a = foo, b = a; for (; bar;) return b }",
234+
"function wrapper() { for (var b = foo; bar;) return b }",
235+
);
236+
test(
237+
"function wrapper() { let a = foo, b = a; for (; bar;) return b }",
238+
"function wrapper() { let b = foo; for (; bar;) return b }",
239+
);
240+
test_same("function wrapper() { using a = foo, b = a; for (; bar;) return b }");
241+
test(
242+
"function wrapper() { var a = foo, b = a, c = bar, d = c; for (; baz;) return [b, d] }",
243+
"function wrapper() { for (var b = foo, d = bar; baz;) return [b, d] }",
244+
);
245+
246+
test_script_same("for (var a = foo, b = a; bar;) console.log(b)");
247+
test(
248+
"function wrapper() { for (var a = foo, b = a; bar;) return b }",
249+
"function wrapper() { for (var b = foo; bar;) return b }",
250+
);
251+
test(
252+
"function wrapper() { for (let a = foo, b = a; bar;) return b }",
253+
"function wrapper() { for (let b = foo; bar;) return b }",
254+
);
255+
test_same("function wrapper() { for (using a = foo, b = a; bar;) return b }");
256+
test(
257+
"function wrapper() { for (var a = foo, b = a, c = bar, d = c; baz;) return [b, d] }",
258+
"function wrapper() { for (var b = foo, d = bar; baz;) return [b, d] }",
259+
);
260+
}
261+
226262
#[test]
227263
fn keep_exposed_variables() {
228264
test_same("var x = foo; x(); export { x }");

tasks/minsize/minsize.snap

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -5,23 +5,23 @@ Original | minified | minified | gzip | gzip | Iterations | Fi
55

66
173.90 kB | 59.44 kB | 59.82 kB | 19.16 kB | 19.33 kB | 2 | moment.js
77

8-
287.63 kB | 89.28 kB | 90.07 kB | 30.94 kB | 31.95 kB | 2 | jquery.js
8+
287.63 kB | 89.27 kB | 90.07 kB | 30.91 kB | 31.95 kB | 2 | jquery.js
99

1010
342.15 kB | 117 kB | 118.14 kB | 43.19 kB | 44.37 kB | 2 | vue.js
1111

12-
544.10 kB | 71.18 kB | 72.48 kB | 25.85 kB | 26.20 kB | 2 | lodash.js
12+
544.10 kB | 71.15 kB | 72.48 kB | 25.85 kB | 26.20 kB | 2 | lodash.js
1313

1414
555.77 kB | 267.47 kB | 270.13 kB | 88.05 kB | 90.80 kB | 2 | d3.js
1515

1616
1.01 MB | 439.53 kB | 458.89 kB | 122.13 kB | 126.71 kB | 2 | bundle.min.js
1717

18-
1.25 MB | 642.81 kB | 646.76 kB | 159.44 kB | 163.73 kB | 2 | three.js
18+
1.25 MB | 642.80 kB | 646.76 kB | 159.41 kB | 163.73 kB | 2 | three.js
1919

20-
2.14 MB | 712.15 kB | 724.14 kB | 160.92 kB | 181.07 kB | 2 | victory.js
20+
2.14 MB | 712.13 kB | 724.14 kB | 160.90 kB | 181.07 kB | 2 | victory.js
2121

22-
3.20 MB | 1.00 MB | 1.01 MB | 323.08 kB | 331.56 kB | 3 | echarts.js
22+
3.20 MB | 1.00 MB | 1.01 MB | 323.05 kB | 331.56 kB | 3 | echarts.js
2323

24-
6.69 MB | 2.22 MB | 2.31 MB | 459.15 kB | 488.28 kB | 4 | antd.js
24+
6.69 MB | 2.22 MB | 2.31 MB | 458.81 kB | 488.28 kB | 4 | antd.js
2525

26-
10.95 MB | 3.34 MB | 3.49 MB | 855.12 kB | 915.50 kB | 4 | typescript.js
26+
10.95 MB | 3.34 MB | 3.49 MB | 855.11 kB | 915.50 kB | 4 | typescript.js
2727

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,14 @@
11
File | File size || Sys allocs | Sys reallocs || Arena allocs | Arena reallocs | Arena bytes
22
-------------------------------------------------------------------------------------------------------------------------------------------
3-
checker.ts | 2.92 MB || 84076 | 14190 || 153691 | 29463
3+
checker.ts | 2.92 MB || 84076 | 14190 || 153733 | 29463
44

5-
cal.com.tsx | 1.06 MB || 40528 | 3033 || 37077 | 4736
5+
cal.com.tsx | 1.06 MB || 40528 | 3033 || 37173 | 4736
66

77
RadixUIAdoptionSection.jsx | 2.52 kB || 85 | 9 || 30 | 6
88

9-
pdf.mjs | 567.30 kB || 19825 | 2900 || 47400 | 7781
9+
pdf.mjs | 567.30 kB || 19825 | 2900 || 47404 | 7781
1010

11-
antd.js | 6.69 MB || 99864 | 13523 || 331767 | 70119
11+
antd.js | 6.69 MB || 99749 | 13523 || 331957 | 69885
1212

1313
binder.ts | 193.08 kB || 4771 | 974 || 7059 | 834
1414

0 commit comments

Comments
 (0)