perf(minifier): fold logical expressions in place (#26768)

refactor of logical-expression folding to update expressions in place,
avoiding unnecessary temporary nodes.

crates/oxc_minifier/src/peephole/fold_constants.rs

@@ -68,11 +68,9 @@ impl<'a> PeepholeOptimizations {
 
     pub fn fold_logical_expr(expr: &mut Expression<'a>, ctx: &mut TraverseCtx<'a>) {
         let Expression::LogicalExpression(e) = expr else { return };
-        if let Some(changed) = match e.operator {
-            LogicalOperator::And | LogicalOperator::Or => Self::try_fold_and_or(e, ctx),
-            LogicalOperator::Coalesce => Self::try_fold_coalesce(e, ctx),
-        } {
-            ctx.replace_expression(expr, changed);
+        match e.operator {
+            LogicalOperator::And | LogicalOperator::Or => Self::try_fold_and_or(expr, ctx),
+            LogicalOperator::Coalesce => Self::try_fold_coalesce(expr, ctx),
         }
     }
 
@@ -116,20 +114,18 @@ impl<'a> PeepholeOptimizations {
     /// Try to fold a AND / OR node.
     ///
     /// port from [closure-compiler](https://github.com/google/closure-compiler/blob/09094b551915a6487a980a783831cba58b5739d1/src/com/google/javascript/jscomp/PeepholeFoldConstants.java#L587)
-    pub fn try_fold_and_or(
-        logical_expr: &mut LogicalExpression<'a>,
-        ctx: &TraverseCtx<'a>,
-    ) -> Option<Expression<'a>> {
+    pub fn try_fold_and_or(expr: &mut Expression<'a>, ctx: &mut TraverseCtx<'a>) {
+        let Expression::LogicalExpression(logical_expr) = expr else {
+            return;
+        };
+
         let op = logical_expr.operator;
         debug_assert!(matches!(op, LogicalOperator::And | LogicalOperator::Or));
 
-        let left = &logical_expr.left;
-        let left_val = left.evaluate_value_to_boolean(ctx);
-
-        if let Some(lval) = left_val {
+        if let Some(lval) = logical_expr.left.evaluate_value_to_boolean(ctx) {
             // (TRUE || x) => TRUE (also, (3 || x) => 3)
             // (FALSE && x) => FALSE
-            if if lval { op.is_or() } else { op.is_and() } {
+            if lval == op.is_or() {
                 // Preserve `0 && (module.exports = { ... })` — esbuild emits
                 // it on Node platform as a parse-time hint for
                 // `cjs-module-lexer` to detect named CJS exports
@@ -139,104 +135,100 @@ impl<'a> PeepholeOptimizations {
                 // Restores the bailout removed by the #8618 refactor; the
                 // original lived at #4878.
                 if !lval && op.is_and() && is_cjs_module_exports_hint(&logical_expr.right) {
-                    return None;
+                    return;
                 }
-                return Some(logical_expr.left.take_in(ctx));
-            } else if !left.may_have_side_effects(ctx) {
-                // `(true && o.f)` => `(0, o.f)`
-                if Self::should_keep_indirect_access(&logical_expr.right, ctx) {
-                    return Some(Self::preserve_indirect_access(
-                        logical_expr.left.span(),
-                        logical_expr.right.take_in(ctx),
-                        ctx,
-                    ));
-                }
-                // (FALSE || x) => x
-                // (TRUE && x) => x
-                return Some(logical_expr.right.take_in(ctx));
+                ctx.drop_expression(&logical_expr.right);
+                ctx.replace_expression_with(expr, |e, _ctx| {
+                    let Expression::LogicalExpression(e) = e else {
+                        unreachable!();
+                    };
+                    e.unbox().left
+                });
+                return;
             }
-            // Left side may have side effects, but we know its boolean value.
-            // e.g. true_with_sideeffects || foo() => true_with_sideeffects, foo()
-            // or: false_with_sideeffects && foo() => false_with_sideeffects, foo()
-            let left = logical_expr.left.take_in(ctx);
-            let right = logical_expr.right.take_in(ctx);
-            let sequence_expr =
-                Expression::new_sequence_expression(logical_expr.span, [left, right], ctx);
-            return Some(sequence_expr);
-        } else if let Expression::LogicalExpression(left_child) = &mut logical_expr.left
+            ctx.replace_expression_with(expr, Self::unfold_right_from_logical_expression);
+        } else if let Expression::LogicalExpression(left_child) = &logical_expr.left
             && left_child.operator == logical_expr.operator
+            && let Some(right_boolean) = left_child.right.evaluate_value_to_boolean(ctx)
+            && right_boolean == logical_expr.operator.is_and()
+            && !left_child.right.may_have_side_effects(ctx)
         {
-            let left_child_right_boolean = left_child.right.evaluate_value_to_boolean(ctx);
-            let left_child_op = left_child.operator;
-            if let Some(right_boolean) = left_child_right_boolean
-                && !left_child.right.may_have_side_effects(ctx)
-            {
-                // a || false || b => a || b
-                // a && true && b => a && b
-                if !right_boolean && left_child_op.is_or()
-                    || right_boolean && left_child_op.is_and()
-                {
-                    let left = left_child.left.take_in(ctx);
-                    let right = logical_expr.right.take_in(ctx);
-                    let logic_expr = Expression::new_logical_expression(
-                        logical_expr.span,
-                        left,
-                        left_child_op,
-                        right,
-                        ctx,
-                    );
-                    return Some(logic_expr);
-                }
-            }
+            // `a || false || b` => `a || b`
+            // `a && true && b` => `a && b`
+            ctx.drop_expression(&left_child.right);
+            ctx.replace_expression_with(&mut logical_expr.left, |e, _ctx| {
+                let Expression::LogicalExpression(e) = e else {
+                    unreachable!();
+                };
+                e.unbox().left
+            });
         }
-        None
     }
 
     /// Try to fold a nullish coalesce `foo ?? bar`.
-    pub fn try_fold_coalesce(
-        logical_expr: &mut LogicalExpression<'a>,
-        ctx: &TraverseCtx<'a>,
-    ) -> Option<Expression<'a>> {
+    pub fn try_fold_coalesce(expr: &mut Expression<'a>, ctx: &mut TraverseCtx<'a>) {
+        let Expression::LogicalExpression(logical_expr) = expr else {
+            return;
+        };
         debug_assert_eq!(logical_expr.operator, LogicalOperator::Coalesce);
         let left = &logical_expr.left;
         let left_val = left.value_type(ctx);
         match left_val {
             ValueType::Null | ValueType::Undefined => {
-                Some(if left.may_have_side_effects(ctx) {
-                    // `(a(), null) ?? 1` => `(a(), null, 1)`
-                    let expressions =
-                        [logical_expr.left.take_in(ctx), logical_expr.right.take_in(ctx)];
-                    Expression::new_sequence_expression(logical_expr.span, expressions, ctx)
-                } else {
-                    // `(null ?? o.f)` => `(0, o.f)`
-                    if Self::should_keep_indirect_access(&logical_expr.right, ctx) {
-                        return Some(Self::preserve_indirect_access(
-                            logical_expr.left.span(),
-                            logical_expr.right.take_in(ctx),
-                            ctx,
-                        ));
-                    }
-                    // nullish condition => this expression evaluates to the right side.
-                    logical_expr.right.take_in(ctx)
-                })
+                ctx.replace_expression_with(expr, Self::unfold_right_from_logical_expression);
             }
             ValueType::Number
             | ValueType::BigInt
             | ValueType::String
             | ValueType::Boolean
             | ValueType::Object => {
-                // `(o.f ?? something)` => `(0, o.f)`
-                if Self::should_keep_indirect_access(&logical_expr.left, ctx) {
-                    return Some(Self::preserve_indirect_access(
-                        logical_expr.right.span(),
-                        logical_expr.left.take_in(ctx),
-                        ctx,
-                    ));
-                }
                 // non-nullish condition => this expression evaluates to the left side.
-                Some(logical_expr.left.take_in(ctx))
+                ctx.replace_expression_with(expr, Self::unfold_left_from_logical_expression);
             }
-            ValueType::Undetermined => None,
+            ValueType::Undetermined => {}
+        }
+    }
+
+    /// Unfold the left side of the logical expression.
+    /// `X && true`, `X || false`, `5 ?? **`
+    fn unfold_left_from_logical_expression(
+        e: Expression<'a>,
+        ctx: &mut TraverseCtx<'a>,
+    ) -> Expression<'a> {
+        let Expression::LogicalExpression(e) = e else {
+            unreachable!();
+        };
+        let e = e.unbox();
+        ctx.drop_expression(&e.right);
+        if Self::should_keep_indirect_access(&e.left, ctx) {
+            // `(o.f OP V)()` => `(0, o.f)()`
+            Self::preserve_indirect_access(e.right.span(), e.left, ctx)
+        } else {
+            e.left
+        }
+    }
+
+    /// Unfold the right side of the logical expression.
+    /// `true && X`, `false || X`, `null ?? X`, `undefined ?? X`
+    fn unfold_right_from_logical_expression(
+        e: Expression<'a>,
+        ctx: &mut TraverseCtx<'a>,
+    ) -> Expression<'a> {
+        let Expression::LogicalExpression(e) = e else {
+            unreachable!();
+        };
+        let mut e = e.unbox();
+        if !Self::remove_unused_expression(&mut e.left, ctx) {
+            // `(a(), V) OP 1` => `(a(), V, 1)`
+            Self::join_sequence(e.left, e.right, ctx)
+        } else if Self::should_keep_indirect_access(&e.right, ctx) {
+            // `(V OP o.f)` => `(0, o.f)`
+            ctx.drop_expression(&e.left);
+            Self::preserve_indirect_access(e.left.span(), e.right, ctx)
+        } else {
+            // `(V OP x)` => `x`
+            ctx.drop_expression(&e.left);
+            e.right
         }
     }
 

crates/oxc_minifier/src/peephole/minimize_for_statement.rs

@@ -1,4 +1,4 @@
-use oxc_allocator::{ArenaBox, TakeIn};
+use oxc_allocator::TakeIn;
 use oxc_ast::ast::*;
 use oxc_span::GetSpan;
 
@@ -68,14 +68,16 @@ impl<'a> PeepholeOptimizations {
             let IfStatement { test, alternate, .. } = if_stmt.unbox();
             let expr = Self::minimize_not(test.span(), test, ctx, true);
 
-            if let Some(test) = &mut for_stmt.test {
-                let left = test.take_in(ctx);
-                let mut logical_expr =
-                    LogicalExpression::new(test.span(), left, LogicalOperator::And, expr, ctx);
-                let new_test = Self::try_fold_and_or(&mut logical_expr, ctx).unwrap_or_else(|| {
-                    Expression::LogicalExpression(ArenaBox::new_in(logical_expr, ctx))
-                });
-                ctx.replace_expression(test, new_test);
+            if let Some(left) = for_stmt.test.take() {
+                let mut logical_expr = Expression::new_logical_expression(
+                    left.span(),
+                    left,
+                    LogicalOperator::And,
+                    expr,
+                    ctx,
+                );
+                Self::try_fold_and_or(&mut logical_expr, ctx);
+                for_stmt.test = Some(logical_expr);
             } else {
                 for_stmt.test = Some(expr);
             }
@@ -111,14 +113,16 @@ impl<'a> PeepholeOptimizations {
 
             let expr = test;
 
-            if let Some(test) = &mut for_stmt.test {
-                let left = test.take_in(ctx);
-                let mut logical_expr =
-                    LogicalExpression::new(test.span(), left, LogicalOperator::And, expr, ctx);
-                let new_test = Self::try_fold_and_or(&mut logical_expr, ctx).unwrap_or_else(|| {
-                    Expression::LogicalExpression(ArenaBox::new_in(logical_expr, ctx))
-                });
-                ctx.replace_expression(test, new_test);
+            if let Some(left) = for_stmt.test.take() {
+                let mut logical_expr = Expression::new_logical_expression(
+                    left.span(),
+                    left,
+                    LogicalOperator::And,
+                    expr,
+                    ctx,
+                );
+                Self::try_fold_and_or(&mut logical_expr, ctx);
+                for_stmt.test = Some(logical_expr);
             } else {
                 for_stmt.test = Some(expr);
             }

crates/oxc_minifier/src/peephole/minimize_statements.rs

@@ -207,7 +207,7 @@ impl<'a> PeepholeOptimizations {
         ctx.replace_expression_with(target, |b, ctx| Self::join_sequence(a, b, ctx));
     }
 
-    fn join_sequence(
+    pub fn join_sequence(
         a: Expression<'a>,
         b: Expression<'a>,
         ctx: &TraverseCtx<'a>,

tasks/track_memory_allocations/allocs_minifier.yaml

@@ -5,7 +5,7 @@ checker.ts:
   sys deallocs: 122
   sys alloc bytes: 14996287 # 15.00 MB
   sys peak growth: 10169560 # 10.17 MB
-  arena allocs: 56184
+  arena allocs: 56181
   arena reallocs: 5084
   arena size: 16316976 # 16.32 MB
 
@@ -16,7 +16,7 @@ App.tsx:
   sys deallocs: 61
   sys alloc bytes: 2128539 # 2.13 MB
   sys peak growth: 1612427 # 1.61 MB
-  arena allocs: 7452
+  arena allocs: 7451
   arena reallocs: 920
   arena size: 2683840 # 2.68 MB
 
@@ -38,7 +38,7 @@ pdf.mjs:
   sys deallocs: 2427
   sys alloc bytes: 5313795 # 5.31 MB
   sys peak growth: 3693355 # 3.69 MB
-  arena allocs: 19499
+  arena allocs: 19497
   arena reallocs: 2574
   arena size: 5708368 # 5.71 MB
 
@@ -49,9 +49,9 @@ antd.js:
   sys deallocs: 860
   sys alloc bytes: 29973448 # 29.97 MB
   sys peak growth: 19934725 # 19.93 MB
-  arena allocs: 181679
+  arena allocs: 181501
   arena reallocs: 34406
-  arena size: 43044248 # 43.04 MB
+  arena size: 43041368 # 43.04 MB
 
 binder.ts:
   file size: 193077 # 193.08 kB
@@ -71,6 +71,6 @@ kitchen-sink.tsx:
   sys deallocs: 1854
   sys alloc bytes: 5304252 # 5.30 MB
   sys peak growth: 3707425 # 3.71 MB
-  arena allocs: 31129
+  arena allocs: 31107
   arena reallocs: 3012
   arena size: 8443808 # 8.44 MB