Skip to content

Commit

Permalink
feat(minifier): add MinimizeConditions pass (#5747)
Browse files Browse the repository at this point in the history
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.");
```
  • Loading branch information
Boshen committed Sep 13, 2024
1 parent b4ed564 commit 6bc13f6
Show file tree
Hide file tree
Showing 12 changed files with 231 additions and 176 deletions.
1 change: 1 addition & 0 deletions crates/oxc_minifier/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -39,5 +39,6 @@ num-traits = { workspace = true }
[dev-dependencies]
oxc_parser = { workspace = true }

cow-utils = { workspace = true }
insta = { workspace = true }
pico-args = { workspace = true }
148 changes: 4 additions & 144 deletions crates/oxc_minifier/src/ast_passes/fold_constants.rs
Original file line number Diff line number Diff line change
@@ -1,11 +1,7 @@
//! Constant Folding
//!
//! <https://github.com/google/closure-compiler/blob/master/src/com/google/javascript/jscomp/PeepholeFoldConstants.java>

use std::{cmp::Ordering, mem};

use num_bigint::BigInt;
use oxc_ast::{ast::*, AstBuilder, Visit};
use oxc_ast::{ast::*, AstBuilder};
use oxc_span::{GetSpan, Span, SPAN};
use oxc_syntax::{
number::NumberBase,
Expand All @@ -14,13 +10,15 @@ use oxc_syntax::{
use oxc_traverse::{Traverse, TraverseCtx};

use crate::{
keep_var::KeepVar,
node_util::{is_exact_int64, IsLiteralValue, MayHaveSideEffects, NodeUtil, NumberValue},
tri::Tri,
ty::Ty,
CompressorPass,
};

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

impl<'a> Traverse<'a> for FoldConstants<'a> {
fn exit_statement(&mut self, stmt: &mut Statement<'a>, ctx: &mut TraverseCtx<'a>) {
self.fold_condition(stmt, ctx);
}

fn exit_expression(&mut self, expr: &mut Expression<'a>, ctx: &mut TraverseCtx<'a>) {
self.fold_expression(expr, ctx);
}
Expand All @@ -58,8 +52,6 @@ impl<'a> FoldConstants<'a> {
{
self.try_fold_and_or(e, ctx)
}
// TODO: move to `PeepholeMinimizeConditions`
Expression::ConditionalExpression(e) => self.try_fold_conditional_expression(e, ctx),
Expression::UnaryExpression(e) => self.try_fold_unary_expression(e, ctx),
_ => None,
} {
Expand Down Expand Up @@ -109,65 +101,6 @@ impl<'a> FoldConstants<'a> {
}
}

fn fold_expression_and_get_boolean_value(
&self,
expr: &mut Expression<'a>,
ctx: &mut TraverseCtx<'a>,
) -> Option<bool> {
self.fold_expression(expr, ctx);
ctx.get_boolean_value(expr).to_option()
}

fn fold_if_statement(&self, stmt: &mut Statement<'a>, ctx: &mut TraverseCtx<'a>) {
let Statement::IfStatement(if_stmt) = stmt else { return };

// Descend and remove `else` blocks first.
if let Some(alternate) = &mut if_stmt.alternate {
self.fold_if_statement(alternate, ctx);
if matches!(alternate, Statement::EmptyStatement(_)) {
if_stmt.alternate = None;
}
}

match self.fold_expression_and_get_boolean_value(&mut if_stmt.test, ctx) {
Some(true) => {
*stmt = self.ast.move_statement(&mut if_stmt.consequent);
}
Some(false) => {
*stmt = if let Some(alternate) = &mut if_stmt.alternate {
self.ast.move_statement(alternate)
} else {
// Keep hoisted `vars` from the consequent block.
let mut keep_var = KeepVar::new(self.ast);
keep_var.visit_statement(&if_stmt.consequent);
keep_var
.get_variable_declaration_statement()
.unwrap_or_else(|| self.ast.statement_empty(SPAN))
};
}
None => {}
}
}

fn try_fold_conditional_expression(
&self,
expr: &mut ConditionalExpression<'a>,
ctx: &mut TraverseCtx<'a>,
) -> Option<Expression<'a>> {
match self.fold_expression_and_get_boolean_value(&mut expr.test, ctx) {
Some(true) => {
// Bail `let o = { f() { assert.ok(this !== o); } }; (true ? o.f : false)(); (true ? o.f : false)``;`
let parent = ctx.ancestry.parent();
if parent.is_tagged_template_expression() || parent.is_call_expression() {
return None;
}
Some(self.ast.move_expression(&mut expr.consequent))
}
Some(false) => Some(self.ast.move_expression(&mut expr.alternate)),
_ => None,
}
}

fn try_fold_unary_expression(
&self,
expr: &mut UnaryExpression<'a>,
Expand Down Expand Up @@ -722,77 +655,4 @@ impl<'a> FoldConstants<'a> {
}
None
}

pub(crate) fn fold_condition<'b>(
&self,
stmt: &'b mut Statement<'a>,
ctx: &mut TraverseCtx<'a>,
) {
match stmt {
Statement::WhileStatement(while_stmt) => {
let minimized_expr = self.fold_expression_in_condition(&mut while_stmt.test);

if let Some(min_expr) = minimized_expr {
while_stmt.test = min_expr;
}
}
Statement::ForStatement(for_stmt) => {
let test_expr = for_stmt.test.as_mut();

if let Some(test_expr) = test_expr {
let minimized_expr = self.fold_expression_in_condition(test_expr);

if let Some(min_expr) = minimized_expr {
for_stmt.test = Some(min_expr);
}
}
}
Statement::IfStatement(_) => {
self.fold_if_statement(stmt, ctx);
}
_ => {}
};
}

fn fold_expression_in_condition(&self, expr: &mut Expression<'a>) -> Option<Expression<'a>> {
let folded_expr = match expr {
Expression::UnaryExpression(unary_expr) => match unary_expr.operator {
UnaryOperator::LogicalNot => {
let should_fold = Self::try_minimize_not(&mut unary_expr.argument);

if should_fold {
Some(self.ast.move_expression(&mut unary_expr.argument))
} else {
None
}
}
_ => None,
},
_ => None,
};

folded_expr
}

/// ported from [closure compiler](https://github.com/google/closure-compiler/blob/master/src/com/google/javascript/jscomp/PeepholeMinimizeConditions.java#L401-L435)
fn try_minimize_not(expr: &mut Expression<'a>) -> bool {
let span = &mut expr.span();

match expr {
Expression::BinaryExpression(binary_expr) => {
let new_op = binary_expr.operator.equality_inverse_operator();

match new_op {
Some(new_op) => {
binary_expr.operator = new_op;
binary_expr.span = *span;

true
}
_ => false,
}
}
_ => false,
}
}
}
70 changes: 70 additions & 0 deletions crates/oxc_minifier/src/ast_passes/minimize_conditions.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
use oxc_ast::{ast::*, AstBuilder};
use oxc_traverse::{Traverse, TraverseCtx};

use crate::{node_util::NodeUtil, tri::Tri, CompressorPass};

/// Minimize Conditions
///
/// A peephole optimization that minimizes conditional expressions according to De Morgan's laws.
/// Also rewrites conditional statements as expressions by replacing them
/// with `? :` and short-circuit binary operators.
///
/// <https://github.com/google/closure-compiler/blob/master/src/com/google/javascript/jscomp/PeepholeMinimizeConditions.java>
pub struct MinimizeConditions<'a> {
ast: AstBuilder<'a>,
}

impl<'a> CompressorPass<'a> for MinimizeConditions<'a> {}

impl<'a> Traverse<'a> for MinimizeConditions<'a> {
fn exit_expression(&mut self, expr: &mut Expression<'a>, ctx: &mut TraverseCtx<'a>) {
self.fold_expression(expr, ctx);
}
}

impl<'a> MinimizeConditions<'a> {
pub fn new(ast: AstBuilder<'a>) -> Self {
Self { ast }
}

fn fold_expression(&self, expr: &mut Expression<'a>, ctx: &mut TraverseCtx<'a>) {
if let Some(folded_expr) = match expr {
Expression::ConditionalExpression(e) => self.try_fold_conditional_expression(e, ctx),
Expression::UnaryExpression(e) if e.operator.is_not() => self.try_minimize_not(e),
_ => None,
} {
*expr = folded_expr;
};
}

fn try_fold_conditional_expression(
&self,
expr: &mut ConditionalExpression<'a>,
ctx: &mut TraverseCtx<'a>,
) -> Option<Expression<'a>> {
match ctx.get_boolean_value(&expr.test) {
Tri::True => {
// Bail `let o = { f() { assert.ok(this !== o); } }; (true ? o.f : false)(); (true ? o.f : false)``;`
let parent = ctx.ancestry.parent();
if parent.is_tagged_template_expression() || parent.is_call_expression() {
return None;
}
Some(self.ast.move_expression(&mut expr.consequent))
}
Tri::False => Some(self.ast.move_expression(&mut expr.alternate)),
Tri::Unknown => None,
}
}

/// Try to minimize NOT nodes such as `!(x==y)`.
fn try_minimize_not(&self, expr: &mut UnaryExpression<'a>) -> Option<Expression<'a>> {
debug_assert!(expr.operator.is_not());
if let Expression::BinaryExpression(binary_expr) = &mut expr.argument {
if let Some(new_op) = binary_expr.operator.equality_inverse_operator() {
binary_expr.operator = new_op;
return Some(self.ast.move_expression(&mut expr.argument));
}
}
None
}
}
9 changes: 6 additions & 3 deletions crates/oxc_minifier/src/ast_passes/mod.rs
Original file line number Diff line number Diff line change
@@ -1,18 +1,21 @@
mod collapse;
mod fold_constants;
mod minimize_conditions;
mod remove_dead_code;
mod remove_syntax;
mod substitute_alternate_syntax;

pub use collapse::Collapse;
pub use fold_constants::FoldConstants;
use oxc_ast::ast::Program;
use oxc_semantic::{ScopeTree, SymbolTable};
use oxc_traverse::{walk_program, Traverse, TraverseCtx};
pub use minimize_conditions::MinimizeConditions;
pub use remove_dead_code::RemoveDeadCode;
pub use remove_syntax::RemoveSyntax;
pub use substitute_alternate_syntax::SubstituteAlternateSyntax;

use oxc_ast::ast::Program;
use oxc_semantic::{ScopeTree, SymbolTable};
use oxc_traverse::{walk_program, Traverse, TraverseCtx};

use crate::node_util::NodeUtil;

impl<'a> NodeUtil for TraverseCtx<'a> {
Expand Down
40 changes: 38 additions & 2 deletions crates/oxc_minifier/src/ast_passes/remove_dead_code.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
use oxc_allocator::Vec;
use oxc_ast::{ast::*, AstBuilder, Visit};
use oxc_span::SPAN;
use oxc_traverse::{Traverse, TraverseCtx};

use crate::{keep_var::KeepVar, CompressorPass};
use crate::{keep_var::KeepVar, node_util::NodeUtil, tri::Tri, CompressorPass};

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

impl<'a> Traverse<'a> for RemoveDeadCode<'a> {
fn enter_statements(&mut self, stmts: &mut Vec<'a, Statement<'a>>, _ctx: &mut TraverseCtx<'a>) {
fn enter_statement(&mut self, stmt: &mut Statement<'a>, ctx: &mut TraverseCtx<'a>) {
self.fold_if_statement(stmt, ctx);
}

fn exit_statements(&mut self, stmts: &mut Vec<'a, Statement<'a>>, _ctx: &mut TraverseCtx<'a>) {
stmts.retain(|stmt| !matches!(stmt, Statement::EmptyStatement(_)));
self.dead_code_elimination(stmts);
}
Expand Down Expand Up @@ -76,4 +81,35 @@ impl<'a> RemoveDeadCode<'a> {
stmts.push(stmt);
}
}

fn fold_if_statement(&self, stmt: &mut Statement<'a>, ctx: &mut TraverseCtx<'a>) {
let Statement::IfStatement(if_stmt) = stmt else { return };

// Descend and remove `else` blocks first.
if let Some(alternate) = &mut if_stmt.alternate {
self.fold_if_statement(alternate, ctx);
if matches!(alternate, Statement::EmptyStatement(_)) {
if_stmt.alternate = None;
}
}

match ctx.get_boolean_value(&if_stmt.test) {
Tri::True => {
*stmt = self.ast.move_statement(&mut if_stmt.consequent);
}
Tri::False => {
*stmt = if let Some(alternate) = &mut if_stmt.alternate {
self.ast.move_statement(alternate)
} else {
// Keep hoisted `vars` from the consequent block.
let mut keep_var = KeepVar::new(self.ast);
keep_var.visit_statement(&if_stmt.consequent);
keep_var
.get_variable_declaration_statement()
.unwrap_or_else(|| self.ast.statement_empty(SPAN))
};
}
Tri::Unknown => {}
}
}
}
10 changes: 9 additions & 1 deletion crates/oxc_minifier/src/compressor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,8 @@ use oxc_traverse::TraverseCtx;

use crate::{
ast_passes::{
Collapse, FoldConstants, RemoveDeadCode, RemoveSyntax, SubstituteAlternateSyntax,
Collapse, FoldConstants, MinimizeConditions, RemoveDeadCode, RemoveSyntax,
SubstituteAlternateSyntax,
},
CompressOptions, CompressorPass,
};
Expand Down Expand Up @@ -37,6 +38,7 @@ impl<'a> Compressor<'a> {
// TODO: inline variables
self.remove_syntax(program, &mut ctx);
self.fold_constants(program, &mut ctx);
self.minimize_conditions(program, &mut ctx);
self.remove_dead_code(program, &mut ctx);
// TODO: StatementFusion
self.substitute_alternate_syntax(program, &mut ctx);
Expand All @@ -49,6 +51,12 @@ impl<'a> Compressor<'a> {
}
}

fn minimize_conditions(&self, program: &mut Program<'a>, ctx: &mut TraverseCtx<'a>) {
if self.options.minimize_conditions {
MinimizeConditions::new(ctx.ast).build(program, ctx);
}
}

fn fold_constants(&self, program: &mut Program<'a>, ctx: &mut TraverseCtx<'a>) {
if self.options.fold_constants {
FoldConstants::new(ctx.ast).with_evaluate(self.options.evaluate).build(program, ctx);
Expand Down
Loading

0 comments on commit 6bc13f6

Please sign in to comment.