Skip to content

Commit 6bc13f6

Browse files
BoshenBoshen
committed
feat(minifier): add MinimizeConditions pass (#5747)
I expect small performance regression. But managed to improve the following case from react.developmement.js ``` oxc  main ❯ diff before.js after.js 670c670 < if (!(dispatcher !== null)) throw Error("Invalid hook call. Hooks can only be called inside of the body of a function component. This could happen for one of the following reasons:\n1. You might have mismatching versions of React and the renderer (such as React DOM)\n2. You might be breaking the Rules of Hooks\n3. You might have more than one copy of React in the same app\nSee https://reactjs.org/link/invalid-hook-call for tips about how to debug and fix this problem."); --- > if (dispatcher === null) throw Error("Invalid hook call. Hooks can only be called inside of the body of a function component. This could happen for one of the following reasons:\n1. You might have mismatching versions of React and the renderer (such as React DOM)\n2. You might be breaking the Rules of Hooks\n3. You might have more than one copy of React in the same app\nSee https://reactjs.org/link/invalid-hook-call for tips about how to debug and fix this problem."); ```
1 parent b4ed564 commit 6bc13f6

12 files changed

Lines changed: 231 additions & 176 deletions

File tree

‎crates/oxc_minifier/Cargo.toml‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,5 +39,6 @@ num-traits = { workspace = true }
3939
[dev-dependencies]
4040
oxc_parser = { workspace = true }
4141

42+
cow-utils = { workspace = true }
4243
insta = { workspace = true }
4344
pico-args = { workspace = true }

‎crates/oxc_minifier/src/ast_passes/fold_constants.rs‎

Lines changed: 4 additions & 144 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,7 @@
1-
//! Constant Folding
2-
//!
3-
//! <https://github.com/google/closure-compiler/blob/master/src/com/google/javascript/jscomp/PeepholeFoldConstants.java>
4-
51
use std::{cmp::Ordering, mem};
62

73
use num_bigint::BigInt;
8-
use oxc_ast::{ast::*, AstBuilder, Visit};
4+
use oxc_ast::{ast::*, AstBuilder};
95
use oxc_span::{GetSpan, Span, SPAN};
106
use oxc_syntax::{
117
number::NumberBase,
@@ -14,13 +10,15 @@ use oxc_syntax::{
1410
use oxc_traverse::{Traverse, TraverseCtx};
1511

1612
use crate::{
17-
keep_var::KeepVar,
1813
node_util::{is_exact_int64, IsLiteralValue, MayHaveSideEffects, NodeUtil, NumberValue},
1914
tri::Tri,
2015
ty::Ty,
2116
CompressorPass,
2217
};
2318

19+
/// Constant Folding
20+
///
21+
/// <https://github.com/google/closure-compiler/blob/master/src/com/google/javascript/jscomp/PeepholeFoldConstants.java>
2422
pub struct FoldConstants<'a> {
2523
ast: AstBuilder<'a>,
2624
evaluate: bool,
@@ -29,10 +27,6 @@ pub struct FoldConstants<'a> {
2927
impl<'a> CompressorPass<'a> for FoldConstants<'a> {}
3028

3129
impl<'a> Traverse<'a> for FoldConstants<'a> {
32-
fn exit_statement(&mut self, stmt: &mut Statement<'a>, ctx: &mut TraverseCtx<'a>) {
33-
self.fold_condition(stmt, ctx);
34-
}
35-
3630
fn exit_expression(&mut self, expr: &mut Expression<'a>, ctx: &mut TraverseCtx<'a>) {
3731
self.fold_expression(expr, ctx);
3832
}
@@ -58,8 +52,6 @@ impl<'a> FoldConstants<'a> {
5852
{
5953
self.try_fold_and_or(e, ctx)
6054
}
61-
// TODO: move to `PeepholeMinimizeConditions`
62-
Expression::ConditionalExpression(e) => self.try_fold_conditional_expression(e, ctx),
6355
Expression::UnaryExpression(e) => self.try_fold_unary_expression(e, ctx),
6456
_ => None,
6557
} {
@@ -109,65 +101,6 @@ impl<'a> FoldConstants<'a> {
109101
}
110102
}
111103

112-
fn fold_expression_and_get_boolean_value(
113-
&self,
114-
expr: &mut Expression<'a>,
115-
ctx: &mut TraverseCtx<'a>,
116-
) -> Option<bool> {
117-
self.fold_expression(expr, ctx);
118-
ctx.get_boolean_value(expr).to_option()
119-
}
120-
121-
fn fold_if_statement(&self, stmt: &mut Statement<'a>, ctx: &mut TraverseCtx<'a>) {
122-
let Statement::IfStatement(if_stmt) = stmt else { return };
123-
124-
// Descend and remove `else` blocks first.
125-
if let Some(alternate) = &mut if_stmt.alternate {
126-
self.fold_if_statement(alternate, ctx);
127-
if matches!(alternate, Statement::EmptyStatement(_)) {
128-
if_stmt.alternate = None;
129-
}
130-
}
131-
132-
match self.fold_expression_and_get_boolean_value(&mut if_stmt.test, ctx) {
133-
Some(true) => {
134-
*stmt = self.ast.move_statement(&mut if_stmt.consequent);
135-
}
136-
Some(false) => {
137-
*stmt = if let Some(alternate) = &mut if_stmt.alternate {
138-
self.ast.move_statement(alternate)
139-
} else {
140-
// Keep hoisted `vars` from the consequent block.
141-
let mut keep_var = KeepVar::new(self.ast);
142-
keep_var.visit_statement(&if_stmt.consequent);
143-
keep_var
144-
.get_variable_declaration_statement()
145-
.unwrap_or_else(|| self.ast.statement_empty(SPAN))
146-
};
147-
}
148-
None => {}
149-
}
150-
}
151-
152-
fn try_fold_conditional_expression(
153-
&self,
154-
expr: &mut ConditionalExpression<'a>,
155-
ctx: &mut TraverseCtx<'a>,
156-
) -> Option<Expression<'a>> {
157-
match self.fold_expression_and_get_boolean_value(&mut expr.test, ctx) {
158-
Some(true) => {
159-
// Bail `let o = { f() { assert.ok(this !== o); } }; (true ? o.f : false)(); (true ? o.f : false)``;`
160-
let parent = ctx.ancestry.parent();
161-
if parent.is_tagged_template_expression() || parent.is_call_expression() {
162-
return None;
163-
}
164-
Some(self.ast.move_expression(&mut expr.consequent))
165-
}
166-
Some(false) => Some(self.ast.move_expression(&mut expr.alternate)),
167-
_ => None,
168-
}
169-
}
170-
171104
fn try_fold_unary_expression(
172105
&self,
173106
expr: &mut UnaryExpression<'a>,
@@ -722,77 +655,4 @@ impl<'a> FoldConstants<'a> {
722655
}
723656
None
724657
}
725-
726-
pub(crate) fn fold_condition<'b>(
727-
&self,
728-
stmt: &'b mut Statement<'a>,
729-
ctx: &mut TraverseCtx<'a>,
730-
) {
731-
match stmt {
732-
Statement::WhileStatement(while_stmt) => {
733-
let minimized_expr = self.fold_expression_in_condition(&mut while_stmt.test);
734-
735-
if let Some(min_expr) = minimized_expr {
736-
while_stmt.test = min_expr;
737-
}
738-
}
739-
Statement::ForStatement(for_stmt) => {
740-
let test_expr = for_stmt.test.as_mut();
741-
742-
if let Some(test_expr) = test_expr {
743-
let minimized_expr = self.fold_expression_in_condition(test_expr);
744-
745-
if let Some(min_expr) = minimized_expr {
746-
for_stmt.test = Some(min_expr);
747-
}
748-
}
749-
}
750-
Statement::IfStatement(_) => {
751-
self.fold_if_statement(stmt, ctx);
752-
}
753-
_ => {}
754-
};
755-
}
756-
757-
fn fold_expression_in_condition(&self, expr: &mut Expression<'a>) -> Option<Expression<'a>> {
758-
let folded_expr = match expr {
759-
Expression::UnaryExpression(unary_expr) => match unary_expr.operator {
760-
UnaryOperator::LogicalNot => {
761-
let should_fold = Self::try_minimize_not(&mut unary_expr.argument);
762-
763-
if should_fold {
764-
Some(self.ast.move_expression(&mut unary_expr.argument))
765-
} else {
766-
None
767-
}
768-
}
769-
_ => None,
770-
},
771-
_ => None,
772-
};
773-
774-
folded_expr
775-
}
776-
777-
/// ported from [closure compiler](https://github.com/google/closure-compiler/blob/master/src/com/google/javascript/jscomp/PeepholeMinimizeConditions.java#L401-L435)
778-
fn try_minimize_not(expr: &mut Expression<'a>) -> bool {
779-
let span = &mut expr.span();
780-
781-
match expr {
782-
Expression::BinaryExpression(binary_expr) => {
783-
let new_op = binary_expr.operator.equality_inverse_operator();
784-
785-
match new_op {
786-
Some(new_op) => {
787-
binary_expr.operator = new_op;
788-
binary_expr.span = *span;
789-
790-
true
791-
}
792-
_ => false,
793-
}
794-
}
795-
_ => false,
796-
}
797-
}
798658
}

‎crates/oxc_minifier/src/ast_passes/minimize_conditions.rs‎

Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,70 @@
1+
use oxc_ast::{ast::*, AstBuilder};
2+
use oxc_traverse::{Traverse, TraverseCtx};
3+
4+
use crate::{node_util::NodeUtil, tri::Tri, CompressorPass};
5+
6+
/// Minimize Conditions
7+
///
8+
/// A peephole optimization that minimizes conditional expressions according to De Morgan's laws.
9+
/// Also rewrites conditional statements as expressions by replacing them
10+
/// with `? :` and short-circuit binary operators.
11+
///
12+
/// <https://github.com/google/closure-compiler/blob/master/src/com/google/javascript/jscomp/PeepholeMinimizeConditions.java>
13+
pub struct MinimizeConditions<'a> {
14+
ast: AstBuilder<'a>,
15+
}
16+
17+
impl<'a> CompressorPass<'a> for MinimizeConditions<'a> {}
18+
19+
impl<'a> Traverse<'a> for MinimizeConditions<'a> {
20+
fn exit_expression(&mut self, expr: &mut Expression<'a>, ctx: &mut TraverseCtx<'a>) {
21+
self.fold_expression(expr, ctx);
22+
}
23+
}
24+
25+
impl<'a> MinimizeConditions<'a> {
26+
pub fn new(ast: AstBuilder<'a>) -> Self {
27+
Self { ast }
28+
}
29+
30+
fn fold_expression(&self, expr: &mut Expression<'a>, ctx: &mut TraverseCtx<'a>) {
31+
if let Some(folded_expr) = match expr {
32+
Expression::ConditionalExpression(e) => self.try_fold_conditional_expression(e, ctx),
33+
Expression::UnaryExpression(e) if e.operator.is_not() => self.try_minimize_not(e),
34+
_ => None,
35+
} {
36+
*expr = folded_expr;
37+
};
38+
}
39+
40+
fn try_fold_conditional_expression(
41+
&self,
42+
expr: &mut ConditionalExpression<'a>,
43+
ctx: &mut TraverseCtx<'a>,
44+
) -> Option<Expression<'a>> {
45+
match ctx.get_boolean_value(&expr.test) {
46+
Tri::True => {
47+
// Bail `let o = { f() { assert.ok(this !== o); } }; (true ? o.f : false)(); (true ? o.f : false)``;`
48+
let parent = ctx.ancestry.parent();
49+
if parent.is_tagged_template_expression() || parent.is_call_expression() {
50+
return None;
51+
}
52+
Some(self.ast.move_expression(&mut expr.consequent))
53+
}
54+
Tri::False => Some(self.ast.move_expression(&mut expr.alternate)),
55+
Tri::Unknown => None,
56+
}
57+
}
58+
59+
/// Try to minimize NOT nodes such as `!(x==y)`.
60+
fn try_minimize_not(&self, expr: &mut UnaryExpression<'a>) -> Option<Expression<'a>> {
61+
debug_assert!(expr.operator.is_not());
62+
if let Expression::BinaryExpression(binary_expr) = &mut expr.argument {
63+
if let Some(new_op) = binary_expr.operator.equality_inverse_operator() {
64+
binary_expr.operator = new_op;
65+
return Some(self.ast.move_expression(&mut expr.argument));
66+
}
67+
}
68+
None
69+
}
70+
}

‎crates/oxc_minifier/src/ast_passes/mod.rs‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,18 +1,21 @@
11
mod collapse;
22
mod fold_constants;
3+
mod minimize_conditions;
34
mod remove_dead_code;
45
mod remove_syntax;
56
mod substitute_alternate_syntax;
67

78
pub use collapse::Collapse;
89
pub use fold_constants::FoldConstants;
9-
use oxc_ast::ast::Program;
10-
use oxc_semantic::{ScopeTree, SymbolTable};
11-
use oxc_traverse::{walk_program, Traverse, TraverseCtx};
10+
pub use minimize_conditions::MinimizeConditions;
1211
pub use remove_dead_code::RemoveDeadCode;
1312
pub use remove_syntax::RemoveSyntax;
1413
pub use substitute_alternate_syntax::SubstituteAlternateSyntax;
1514

15+
use oxc_ast::ast::Program;
16+
use oxc_semantic::{ScopeTree, SymbolTable};
17+
use oxc_traverse::{walk_program, Traverse, TraverseCtx};
18+
1619
use crate::node_util::NodeUtil;
1720

1821
impl<'a> NodeUtil for TraverseCtx<'a> {

‎crates/oxc_minifier/src/ast_passes/remove_dead_code.rs‎

Lines changed: 38 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,9 @@
11
use oxc_allocator::Vec;
22
use oxc_ast::{ast::*, AstBuilder, Visit};
3+
use oxc_span::SPAN;
34
use oxc_traverse::{Traverse, TraverseCtx};
45

5-
use crate::{keep_var::KeepVar, CompressorPass};
6+
use crate::{keep_var::KeepVar, node_util::NodeUtil, tri::Tri, CompressorPass};
67

78
/// Remove Dead Code from the AST.
89
///
@@ -16,7 +17,11 @@ pub struct RemoveDeadCode<'a> {
1617
impl<'a> CompressorPass<'a> for RemoveDeadCode<'a> {}
1718

1819
impl<'a> Traverse<'a> for RemoveDeadCode<'a> {
19-
fn enter_statements(&mut self, stmts: &mut Vec<'a, Statement<'a>>, _ctx: &mut TraverseCtx<'a>) {
20+
fn enter_statement(&mut self, stmt: &mut Statement<'a>, ctx: &mut TraverseCtx<'a>) {
21+
self.fold_if_statement(stmt, ctx);
22+
}
23+
24+
fn exit_statements(&mut self, stmts: &mut Vec<'a, Statement<'a>>, _ctx: &mut TraverseCtx<'a>) {
2025
stmts.retain(|stmt| !matches!(stmt, Statement::EmptyStatement(_)));
2126
self.dead_code_elimination(stmts);
2227
}
@@ -76,4 +81,35 @@ impl<'a> RemoveDeadCode<'a> {
7681
stmts.push(stmt);
7782
}
7883
}
84+
85+
fn fold_if_statement(&self, stmt: &mut Statement<'a>, ctx: &mut TraverseCtx<'a>) {
86+
let Statement::IfStatement(if_stmt) = stmt else { return };
87+
88+
// Descend and remove `else` blocks first.
89+
if let Some(alternate) = &mut if_stmt.alternate {
90+
self.fold_if_statement(alternate, ctx);
91+
if matches!(alternate, Statement::EmptyStatement(_)) {
92+
if_stmt.alternate = None;
93+
}
94+
}
95+
96+
match ctx.get_boolean_value(&if_stmt.test) {
97+
Tri::True => {
98+
*stmt = self.ast.move_statement(&mut if_stmt.consequent);
99+
}
100+
Tri::False => {
101+
*stmt = if let Some(alternate) = &mut if_stmt.alternate {
102+
self.ast.move_statement(alternate)
103+
} else {
104+
// Keep hoisted `vars` from the consequent block.
105+
let mut keep_var = KeepVar::new(self.ast);
106+
keep_var.visit_statement(&if_stmt.consequent);
107+
keep_var
108+
.get_variable_declaration_statement()
109+
.unwrap_or_else(|| self.ast.statement_empty(SPAN))
110+
};
111+
}
112+
Tri::Unknown => {}
113+
}
114+
}
79115
}

‎crates/oxc_minifier/src/compressor.rs‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,8 @@ use oxc_traverse::TraverseCtx;
55

66
use crate::{
77
ast_passes::{
8-
Collapse, FoldConstants, RemoveDeadCode, RemoveSyntax, SubstituteAlternateSyntax,
8+
Collapse, FoldConstants, MinimizeConditions, RemoveDeadCode, RemoveSyntax,
9+
SubstituteAlternateSyntax,
910
},
1011
CompressOptions, CompressorPass,
1112
};
@@ -37,6 +38,7 @@ impl<'a> Compressor<'a> {
3738
// TODO: inline variables
3839
self.remove_syntax(program, &mut ctx);
3940
self.fold_constants(program, &mut ctx);
41+
self.minimize_conditions(program, &mut ctx);
4042
self.remove_dead_code(program, &mut ctx);
4143
// TODO: StatementFusion
4244
self.substitute_alternate_syntax(program, &mut ctx);
@@ -49,6 +51,12 @@ impl<'a> Compressor<'a> {
4951
}
5052
}
5153

54+
fn minimize_conditions(&self, program: &mut Program<'a>, ctx: &mut TraverseCtx<'a>) {
55+
if self.options.minimize_conditions {
56+
MinimizeConditions::new(ctx.ast).build(program, ctx);
57+
}
58+
}
59+
5260
fn fold_constants(&self, program: &mut Program<'a>, ctx: &mut TraverseCtx<'a>) {
5361
if self.options.fold_constants {
5462
FoldConstants::new(ctx.ast).with_evaluate(self.options.evaluate).build(program, ctx);

0 commit comments

Comments
 (0)