Skip to content

perf(minifier): evaluate ternary branches once in minimize_conditional_expression#23479

Merged
camc314 merged 2 commits into
oxc-project:mainfrom
hyf0:perf/minifier-conditional-eval-once
Jun 16, 2026
Merged

perf(minifier): evaluate ternary branches once in minimize_conditional_expression#23479
camc314 merged 2 commits into
oxc-project:mainfrom
hyf0:perf/minifier-conditional-eval-once

Conversation

@hyf0

@hyf0 hyf0 commented Jun 16, 2026

Copy link
Copy Markdown
Contributor

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.

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.

@codspeed-hq

codspeed-hq Bot commented Jun 16, 2026

Copy link
Copy Markdown

Merging this PR will not alter performance

✅ 52 untouched benchmarks
⏩ 19 skipped benchmarks1


Comparing hyf0:perf/minifier-conditional-eval-once (ffe4deb) with main (e409fe0)

Open in CodSpeed

Footnotes

  1. 19 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports.

@hyf0 hyf0 marked this pull request as ready for review June 16, 2026 08:30
@camc314 camc314 added the A-minifier Area - Minifier label Jun 16, 2026
@camc314 camc314 force-pushed the perf/minifier-conditional-eval-once branch from 5d591ee to 7214c4b Compare June 16, 2026 08:40

@camc314 camc314 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks!

@camc314 camc314 merged commit d1fa6e0 into oxc-project:main Jun 16, 2026
36 checks passed
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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-minifier Area - Minifier

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants