perf(minifier): evaluate ternary branches once in minimize_conditional_expression#23479
Merged
camc314 merged 2 commits intoJun 16, 2026
Merged
Conversation
Merging this PR will not alter performance
Comparing Footnotes
|
5d591ee to
7214c4b
Compare
Boshen
added a commit
that referenced
this pull request
Jun 18, 2026
### 💥 BREAKING CHANGES - 7a76cd3 estree: [**BREAKING**] Make whether to include TS fields a runtime option (#23574) (overlookmotel) - e7b6b68 estree: [**BREAKING**] `ESTree` config use methods not consts (#23573) (overlookmotel) ### 🚀 Features - 556cc6d data_structures: Add `CodeBuffer::as_str` method (#23571) (overlookmotel) - 38c4b06 parser: Add friendly error for adjacent JSX elements (#23378) (sapphi-red) - 53509a8 minifier: Treeshake pure typed arrays and Set/Map array literals (#23469) (Dunqing) - 09762d9 minifier: Inline const value for read-only vars (#22593) (Dunqing) ### 🐛 Bug Fixes - 20375f9 react_compiler: Keep imports referenced only by a computed key (#23586) (Boshen) - 31bfd9b minifier: Keep Object introspection calls on a possible Proxy (#23483) (Dunqing) - 837a395 parser: Treat a line comment after ':' as leading, not trailing (#23515) (Dunqing) - e409fe0 minifier: Keep `new Map`/`WeakSet`/`WeakMap` with a string argument (#23470) (Dunqing) - ae02b4e ci/parser: Use `minimal` for vitest reporter (#23457) (camc314) ### ⚡ Performance - cf24329 mangler: Compile slot sort once instead of per CAPACITY (#23577) (Boshen) - 4058a6a parser: Reduce code bloat from verify_modifiers monomorphization (#23576) (Boshen) - 053b0c1 estree: Remove pointless `mem::take` (#23572) (overlookmotel) - dfb52b6 transformer: Pre-size statement vecs in TS enum & namespace lowering (#23516) (Yunfei He) - 970e09a minifier: Compute template-literal inline checks in a single pass (#23467) (Yunfei He) - 3170c0e semantic,mangler,minifier: Fix `Semantic::stats` node count and reuse stats in mangler builds (#23352) (Boshen) - d1fa6e0 minifier: Evaluate ternary branches once in minimize_conditional_expression (#23479) (Yunfei He) - 3fa8051 transformer: Pre-size JSX props vec to attribute count (#23466) (Yunfei He) - 488b382 react_compiler: Borrow binding names in prefilter instead of allocating (#23471) (Yunfei He) - bcb3894 minifier: Incremental scoping refresh, delete LiveUsageCollector (#23197) (Dunqing) ### 📚 Documentation - f68641e data_structures: Improve docs on safety contract (#23575) (overlookmotel) Co-authored-by: Boshen <1430279+Boshen@users.noreply.github.com>
camc314
added a commit
that referenced
this pull request
Jul 3, 2026
…l_expression (#23479) ## What & why `minimize_conditional_expression` ran two folds — the boolean fold (`a ? true : false` → `!!a`) and the number fold (`a ? 1 : 0` → `+a`) — and **each** independently called `expr.consequent.evaluate_value(ctx)` and `expr.alternate.evaluate_value(ctx)`. So every ternary evaluated each branch **twice** (4 `evaluate_value` calls per `ConditionalExpression`). `evaluate_value` is a recursive constant-evaluation walk over the operand subtree. This hoists the two results and reuses them across both folds (4 → 2 calls per ternary). `minimize_conditional_expression` runs in `exit_expression` on every `ConditionalExpression`, across the fixed-point peephole loop, and ternaries are common — so this is a hot path. ```rust let consequent_value = expr.consequent.evaluate_value(ctx); let alternate_value = expr.alternate.evaluate_value(ctx); // boolean fold reads the cached values clone-free (as_ref + variant match == into_boolean) // number fold consumes them (into_number) — last use ``` The boolean fold reads the cached values **clone-free** (an `as_ref()` + `Boolean` variant match, identical to `into_boolean`); the number fold consumes them. Nothing mutates the operands between the two folds (the boolean fold only mutates inside arms that `return`), so the cached values are valid. ## Allocations (measured) `cargo allocs` shows a small system-allocation reduction, no regressions: | file | sys allocs: before → after | | --- | --- | | checker.ts | 166 → 162 (**−4**) | | pdf.mjs | 2614 → 2613 (**−1**) | (Arena allocs unchanged.) The primary effect is the halved `evaluate_value` work (instruction count) — CodSpeed will arbitrate that on this PR. The change is structured to never do more work than before. ## Behavior-preserving `cargo test -p oxc_minifier` (535 tests) passes; no snapshot drift beyond the allocation snapshot above. --- Prepared with AI assistance. --------- Co-authored-by: Cameron Clark <cameron.clark@hey.com>
camc314
pushed a commit
that referenced
this pull request
Jul 3, 2026
### 💥 BREAKING CHANGES - 7a76cd3 estree: [**BREAKING**] Make whether to include TS fields a runtime option (#23574) (overlookmotel) - e7b6b68 estree: [**BREAKING**] `ESTree` config use methods not consts (#23573) (overlookmotel) ### 🚀 Features - 556cc6d data_structures: Add `CodeBuffer::as_str` method (#23571) (overlookmotel) - 38c4b06 parser: Add friendly error for adjacent JSX elements (#23378) (sapphi-red) - 53509a8 minifier: Treeshake pure typed arrays and Set/Map array literals (#23469) (Dunqing) - 09762d9 minifier: Inline const value for read-only vars (#22593) (Dunqing) ### 🐛 Bug Fixes - 20375f9 react_compiler: Keep imports referenced only by a computed key (#23586) (Boshen) - 31bfd9b minifier: Keep Object introspection calls on a possible Proxy (#23483) (Dunqing) - 837a395 parser: Treat a line comment after ':' as leading, not trailing (#23515) (Dunqing) - e409fe0 minifier: Keep `new Map`/`WeakSet`/`WeakMap` with a string argument (#23470) (Dunqing) - ae02b4e ci/parser: Use `minimal` for vitest reporter (#23457) (camc314) ### ⚡ Performance - cf24329 mangler: Compile slot sort once instead of per CAPACITY (#23577) (Boshen) - 4058a6a parser: Reduce code bloat from verify_modifiers monomorphization (#23576) (Boshen) - 053b0c1 estree: Remove pointless `mem::take` (#23572) (overlookmotel) - dfb52b6 transformer: Pre-size statement vecs in TS enum & namespace lowering (#23516) (Yunfei He) - 970e09a minifier: Compute template-literal inline checks in a single pass (#23467) (Yunfei He) - 3170c0e semantic,mangler,minifier: Fix `Semantic::stats` node count and reuse stats in mangler builds (#23352) (Boshen) - d1fa6e0 minifier: Evaluate ternary branches once in minimize_conditional_expression (#23479) (Yunfei He) - 3fa8051 transformer: Pre-size JSX props vec to attribute count (#23466) (Yunfei He) - 488b382 react_compiler: Borrow binding names in prefilter instead of allocating (#23471) (Yunfei He) - bcb3894 minifier: Incremental scoping refresh, delete LiveUsageCollector (#23197) (Dunqing) ### 📚 Documentation - f68641e data_structures: Improve docs on safety contract (#23575) (overlookmotel) Co-authored-by: Boshen <1430279+Boshen@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What & why
minimize_conditional_expressionran two folds — the boolean fold (a ? true : false→!!a) andthe number fold (
a ? 1 : 0→+a) — and each independently calledexpr.consequent.evaluate_value(ctx)andexpr.alternate.evaluate_value(ctx). So every ternaryevaluated each branch twice (4
evaluate_valuecalls perConditionalExpression).evaluate_valueis a recursive constant-evaluation walk over the operand subtree. This hoists the tworesults and reuses them across both folds (4 → 2 calls per ternary).
minimize_conditional_expressionruns in
exit_expressionon everyConditionalExpression, across the fixed-point peephole loop, andternaries are common — so this is a hot path.
The boolean fold reads the cached values clone-free (an
as_ref()+Booleanvariant match,identical to
into_boolean); the number fold consumes them. Nothing mutates the operands between thetwo folds (the boolean fold only mutates inside arms that
return), so the cached values are valid.Allocations (measured)
cargo allocsshows a small system-allocation reduction, no regressions:(Arena allocs unchanged.) The primary effect is the halved
evaluate_valuework (instruction count) —CodSpeed will arbitrate that on this PR. The change is structured to never do more work than before.
Behavior-preserving
cargo test -p oxc_minifier(535 tests) passes; no snapshot drift beyond the allocation snapshot above.Prepared with AI assistance.