perf(minifier): add LatePeepholeOptimizations (#8651)

This PR adds a `LatePeepholeOptimizations` pass for minifications that
don't interact with the fixed point loop.

While working on this I found a couple of cases where the previous fixed
point loop is not idempotent.

crates/oxc_ecmascript/src/constant_evaluation/mod.rs

@@ -188,6 +188,7 @@ pub trait ConstantEvaluation<'a> {
                 Some(ConstantValue::String(Cow::Borrowed(lit.value.as_str())))
             }
             Expression::StaticMemberExpression(e) => self.eval_static_member_expression(e),
+            Expression::ComputedMemberExpression(e) => self.eval_computed_member_expression(e),
             _ => None,
         }
     }
@@ -421,7 +422,30 @@ pub trait ConstantEvaluation<'a> {
         match expr.property.name.as_str() {
             "length" => {
                 if let Some(ConstantValue::String(s)) = self.eval_expression(&expr.object) {
-                    // TODO(perf): no need to actually convert, only need the length
+                    Some(ConstantValue::Number(s.encode_utf16().count().to_f64().unwrap()))
+                } else {
+                    if expr.object.may_have_side_effects() {
+                        return None;
+                    }
+
+                    if let Expression::ArrayExpression(arr) = &expr.object {
+                        Some(ConstantValue::Number(arr.elements.len().to_f64().unwrap()))
+                    } else {
+                        None
+                    }
+                }
+            }
+            _ => None,
+        }
+    }
+
+    fn eval_computed_member_expression(
+        &self,
+        expr: &ComputedMemberExpression<'a>,
+    ) -> Option<ConstantValue<'a>> {
+        match &expr.expression {
+            Expression::StringLiteral(s) if s.value == "length" => {
+                if let Some(ConstantValue::String(s)) = self.eval_expression(&expr.object) {
                     Some(ConstantValue::Number(s.encode_utf16().count().to_f64().unwrap()))
                 } else {
                     if expr.object.may_have_side_effects() {

crates/oxc_minifier/src/compressor.rs

@@ -4,7 +4,10 @@ use oxc_semantic::{ScopeTree, SemanticBuilder, SymbolTable};
 use oxc_traverse::ReusableTraverseCtx;
 
 use crate::{
-    peephole::{DeadCodeElimination, Normalize, NormalizeOptions, PeepholeOptimizations},
+    peephole::{
+        DeadCodeElimination, LatePeepholeOptimizations, Normalize, NormalizeOptions,
+        PeepholeOptimizations,
+    },
     CompressOptions,
 };
 
@@ -33,8 +36,8 @@ impl<'a> Compressor<'a> {
         let mut ctx = ReusableTraverseCtx::new(scopes, symbols, self.allocator);
         let normalize_options = NormalizeOptions { convert_while_to_fors: true };
         Normalize::new(normalize_options, self.options).build(program, &mut ctx);
-        PeepholeOptimizations::new(self.options.target, true).run_in_loop(program, &mut ctx);
-        PeepholeOptimizations::new(self.options.target, false).build(program, &mut ctx);
+        PeepholeOptimizations::new(self.options.target).run_in_loop(program, &mut ctx);
+        LatePeepholeOptimizations::new(self.options.target).build(program, &mut ctx);
     }
 
     pub fn dead_code_elimination(self, program: &mut Program<'a>) {

crates/oxc_minifier/src/peephole/convert_to_dotted_properties.rs

@@ -2,30 +2,20 @@ use oxc_ast::ast::*;
 use oxc_syntax::identifier::is_identifier_name;
 use oxc_traverse::TraverseCtx;
 
-use super::PeepholeOptimizations;
+use super::LatePeepholeOptimizations;
 use crate::ctx::Ctx;
 
-impl<'a> PeepholeOptimizations {
+impl<'a> LatePeepholeOptimizations {
     /// Converts property accesses from quoted string or bracket access syntax to dot or unquoted string
     /// syntax, where possible. Dot syntax is more compact.
     ///
     /// <https://github.com/google/closure-compiler/blob/v20240609/src/com/google/javascript/jscomp/ConvertToDottedProperties.java>
-    pub fn convert_to_dotted_properties(
-        &mut self,
-        expr: &mut MemberExpression<'a>,
-        ctx: &mut TraverseCtx<'a>,
-    ) {
-        if !self.in_fixed_loop {
-            self.try_compress_computed_member_expression(expr, Ctx(ctx));
-        }
-    }
-
+    ///
     /// `foo['bar']` -> `foo.bar`
     /// `foo?.['bar']` -> `foo?.bar`
-    fn try_compress_computed_member_expression(
-        &mut self,
+    pub fn convert_to_dotted_properties(
         expr: &mut MemberExpression<'a>,
-        ctx: Ctx<'a, '_>,
+        ctx: &mut TraverseCtx<'a>,
     ) {
         let MemberExpression::ComputedMemberExpression(e) = expr else { return };
         let Expression::StringLiteral(s) = &e.expression else { return };
@@ -35,15 +25,15 @@ impl<'a> PeepholeOptimizations {
             *expr = MemberExpression::StaticMemberExpression(
                 ctx.ast.alloc_static_member_expression(e.span, object, property, e.optional),
             );
-            self.mark_current_function_as_changed();
             return;
         }
         let v = s.value.as_str();
         if e.optional {
             return;
         }
         if let Some(n) = Ctx::string_to_equivalent_number_value(v) {
-            e.expression = ctx.ast.expression_numeric_literal(s.span, n, None, NumberBase::Decimal);
+            e.expression =
+                Ctx(ctx).ast.expression_numeric_literal(s.span, n, None, NumberBase::Decimal);
         }
     }
 }

crates/oxc_minifier/src/peephole/fold_constants.rs

@@ -29,6 +29,7 @@ impl<'a, 'b> PeepholeOptimizations {
                 .or_else(|| Self::try_fold_binary_typeof_comparison(e, ctx)),
             Expression::UnaryExpression(e) => Self::try_fold_unary_expr(e, ctx),
             Expression::StaticMemberExpression(e) => Self::try_fold_static_member_expr(e, ctx),
+            Expression::ComputedMemberExpression(e) => Self::try_fold_computed_member_expr(e, ctx),
             Expression::LogicalExpression(e) => Self::try_fold_logical_expr(e, ctx),
             Expression::ChainExpression(e) => Self::try_fold_optional_chain(e, ctx),
             Expression::CallExpression(e) => Self::try_fold_number_constructor(e, ctx),
@@ -56,12 +57,19 @@ impl<'a, 'b> PeepholeOptimizations {
     }
 
     fn try_fold_static_member_expr(
-        static_member_expr: &mut StaticMemberExpression<'a>,
+        e: &mut StaticMemberExpression<'a>,
         ctx: Ctx<'a, 'b>,
     ) -> Option<Expression<'a>> {
         // TODO: tryFoldObjectPropAccess(n, left, name)
-        ctx.eval_static_member_expression(static_member_expr)
-            .map(|value| ctx.value_to_expr(static_member_expr.span, value))
+        ctx.eval_static_member_expression(e).map(|value| ctx.value_to_expr(e.span, value))
+    }
+
+    fn try_fold_computed_member_expr(
+        e: &mut ComputedMemberExpression<'a>,
+        ctx: Ctx<'a, 'b>,
+    ) -> Option<Expression<'a>> {
+        // TODO: tryFoldObjectPropAccess(n, left, name)
+        ctx.eval_computed_member_expression(e).map(|value| ctx.value_to_expr(e.span, value))
     }
 
     fn try_fold_logical_expr(
@@ -1618,6 +1626,7 @@ mod test {
         test("x = [].length", "x = 0");
         test("x = [1,2,3].length", "x = 3");
         // test("x = [a,b].length", "x = 2");
+        test("x = 'abc'['length']", "x = 3");
 
         // Not handled yet
         test("x = [,,1].length", "x = 3");

crates/oxc_minifier/src/peephole/minimize_conditions.rs

@@ -1,8 +1,9 @@
 use oxc_allocator::Vec;
 use oxc_ast::{ast::*, NONE};
 use oxc_ecmascript::constant_evaluation::{ConstantEvaluation, ValueType};
+use oxc_semantic::ReferenceFlags;
 use oxc_span::{cmp::ContentEq, GetSpan};
-use oxc_traverse::{Ancestor, TraverseCtx};
+use oxc_traverse::{Ancestor, MaybeBoundIdentifier, TraverseCtx};
 
 use crate::ctx::Ctx;
 
@@ -22,15 +23,15 @@ impl<'a> PeepholeOptimizations {
         ctx: &mut TraverseCtx<'a>,
     ) {
         let mut changed = false;
-        let mut changed2 = false;
-        Self::try_replace_if(stmts, &mut changed2, ctx);
-        while changed2 {
-            changed2 = false;
-            Self::try_replace_if(stmts, &mut changed2, ctx);
-            if stmts.iter().any(|stmt| matches!(stmt, Statement::EmptyStatement(_))) {
+        loop {
+            let mut local_change = false;
+            Self::try_replace_if(stmts, &mut local_change, ctx);
+            if local_change {
                 stmts.retain(|stmt| !matches!(stmt, Statement::EmptyStatement(_)));
+                changed = local_change;
+            } else {
+                break;
             }
-            changed = changed2;
         }
         if changed {
             self.mark_current_function_as_changed();
@@ -77,25 +78,27 @@ impl<'a> PeepholeOptimizations {
         expr: &mut Expression<'a>,
         ctx: &mut TraverseCtx<'a>,
     ) {
+        let mut changed = false;
         loop {
-            let mut changed = false;
+            let mut local_change = false;
             if let Expression::ConditionalExpression(logical_expr) = expr {
+                if Self::try_fold_expr_in_boolean_context(&mut logical_expr.test, Ctx(ctx)) {
+                    local_change = true;
+                }
                 if let Some(e) = Self::try_minimize_conditional(logical_expr, ctx) {
                     *expr = e;
-                    changed = true;
-                }
-            }
-            if let Expression::ConditionalExpression(logical_expr) = expr {
-                if Self::try_fold_expr_in_boolean_context(&mut logical_expr.test, Ctx(ctx)) {
-                    changed = true;
+                    local_change = true;
                 }
             }
-            if changed {
-                self.mark_current_function_as_changed();
+            if local_change {
+                changed = true;
             } else {
                 break;
             }
         }
+        if changed {
+            self.mark_current_function_as_changed();
+        }
 
         if let Some(folded_expr) = match expr {
             Expression::UnaryExpression(e) => Self::try_minimize_not(e, ctx),
@@ -124,6 +127,15 @@ impl<'a> PeepholeOptimizations {
                 e.operator = e.operator.equality_inverse_operator().unwrap();
                 Some(ctx.ast.move_expression(&mut expr.argument))
             }
+            Expression::ConditionalExpression(conditional_expr) => {
+                if let Expression::BinaryExpression(e) = &mut conditional_expr.test {
+                    if e.operator.is_equality() {
+                        e.operator = e.operator.equality_inverse_operator().unwrap();
+                        return Some(ctx.ast.move_expression(&mut expr.argument));
+                    }
+                }
+                None
+            }
             _ => None,
         }
     }
@@ -331,9 +343,13 @@ impl<'a> PeepholeOptimizations {
             unreachable!()
         };
         let return_stmt = return_stmt.unbox();
-        match return_stmt.argument {
-            Some(e) => e,
-            None => ctx.ast.void_0(return_stmt.span),
+        if let Some(e) = return_stmt.argument {
+            e
+        } else {
+            let name = "undefined";
+            let symbol_id = ctx.scopes().find_binding(ctx.current_scope_id(), name);
+            let ident = MaybeBoundIdentifier::new(Atom::from(name), symbol_id);
+            ident.create_expression(ReferenceFlags::read(), ctx)
         }
     }
 

crates/oxc_minifier/src/peephole/mod.rs

@@ -20,10 +20,6 @@ use rustc_hash::FxHashSet;
 pub struct PeepholeOptimizations {
     target: ESTarget,
 
-    /// `in_fixed_loop`: Do not compress syntaxes that are hard to analyze inside the fixed loop.
-    /// Opposite of `late` in Closure Compiler.
-    in_fixed_loop: bool,
-
     /// Walk the ast in a fixed point loop until no changes are made.
     /// `prev_function_changed`, `functions_changed` and `current_function` track changes
     /// in top level and each function. No minification code are run if the function is not changed
@@ -37,10 +33,9 @@ pub struct PeepholeOptimizations {
 }
 
 impl<'a> PeepholeOptimizations {
-    pub fn new(target: ESTarget, in_fixed_loop: bool) -> Self {
+    pub fn new(target: ESTarget) -> Self {
         Self {
             target,
-            in_fixed_loop,
             iteration: 0,
             prev_functions_changed: FxHashSet::default(),
             functions_changed: FxHashSet::default(),
@@ -84,7 +79,7 @@ impl<'a> PeepholeOptimizations {
     }
 
     fn is_prev_function_changed(&self) -> bool {
-        if !self.in_fixed_loop || self.iteration == 0 {
+        if self.iteration == 0 {
             return true;
         }
         if let Some((_, prev_changed, _)) = self.current_function_stack.last() {
@@ -159,13 +154,6 @@ impl<'a> Traverse<'a> for PeepholeOptimizations {
         self.minimize_exit_points(body, ctx);
     }
 
-    fn exit_class_body(&mut self, body: &mut ClassBody<'a>, ctx: &mut TraverseCtx<'a>) {
-        if !self.is_prev_function_changed() {
-            return;
-        }
-        self.remove_dead_code_exit_class_body(body, ctx);
-    }
-
     fn exit_variable_declaration(
         &mut self,
         decl: &mut VariableDeclaration<'a>,
@@ -195,17 +183,6 @@ impl<'a> Traverse<'a> for PeepholeOptimizations {
         self.substitute_call_expression(expr, ctx);
     }
 
-    fn exit_member_expression(
-        &mut self,
-        expr: &mut MemberExpression<'a>,
-        ctx: &mut TraverseCtx<'a>,
-    ) {
-        if !self.is_prev_function_changed() {
-            return;
-        }
-        self.convert_to_dotted_properties(expr, ctx);
-    }
-
     fn exit_object_property(&mut self, prop: &mut ObjectProperty<'a>, ctx: &mut TraverseCtx<'a>) {
         if !self.is_prev_function_changed() {
             return;
@@ -263,11 +240,42 @@ impl<'a> Traverse<'a> for PeepholeOptimizations {
         }
         self.substitute_accessor_property(prop, ctx);
     }
+}
+
+/// Changes that do not interfere with optimizations that are run inside the fixed-point loop,
+/// which can be done as a last AST pass.
+pub struct LatePeepholeOptimizations {
+    target: ESTarget,
+}
+
+impl<'a> LatePeepholeOptimizations {
+    pub fn new(target: ESTarget) -> Self {
+        Self { target }
+    }
+
+    pub fn build(&mut self, program: &mut Program<'a>, ctx: &mut ReusableTraverseCtx<'a>) {
+        traverse_mut_with_ctx(self, program, ctx);
+    }
+}
+
+impl<'a> Traverse<'a> for LatePeepholeOptimizations {
+    fn exit_member_expression(
+        &mut self,
+        expr: &mut MemberExpression<'a>,
+        ctx: &mut TraverseCtx<'a>,
+    ) {
+        Self::convert_to_dotted_properties(expr, ctx);
+    }
+
+    fn exit_class_body(&mut self, body: &mut ClassBody<'a>, ctx: &mut TraverseCtx<'a>) {
+        Self::remove_dead_code_exit_class_body(body, ctx);
+    }
+
+    fn exit_expression(&mut self, expr: &mut Expression<'a>, ctx: &mut TraverseCtx<'a>) {
+        Self::substitute_exit_expression(expr, ctx);
+    }
 
     fn exit_catch_clause(&mut self, catch: &mut CatchClause<'a>, ctx: &mut TraverseCtx<'a>) {
-        if !self.is_prev_function_changed() {
-            return;
-        }
         self.substitute_catch_clause(catch, ctx);
     }
 }
@@ -278,7 +286,7 @@ pub struct DeadCodeElimination {
 
 impl<'a> DeadCodeElimination {
     pub fn new() -> Self {
-        Self { inner: PeepholeOptimizations::new(ESTarget::ESNext, false) }
+        Self { inner: PeepholeOptimizations::new(ESTarget::ESNext) }
     }
 
     pub fn build(&mut self, program: &mut Program<'a>, ctx: &mut ReusableTraverseCtx<'a>) {