Skip to content

refactor(minifier): group symbol state#24640

Merged
graphite-app[bot] merged 1 commit into
mainfrom
codex/minifier-symbol-state-facade
Jul 18, 2026
Merged

refactor(minifier): group symbol state#24640
graphite-app[bot] merged 1 commit into
mainfrom
codex/minifier-symbol-state-facade

Conversation

@Dunqing

@Dunqing Dunqing commented Jul 18, 2026

Copy link
Copy Markdown
Member

MinifierState separately exposed dense per-pass values, sparse persistent metadata, and optional reachability state even though all three are indexed by semantic symbols. Its private-member stack also used symbol terminology despite storing names rather than SymbolIds.

This introduces a SymbolState facade that documents the three storage lifecycles and forwards their operations while leaving the reachability algorithm in its dedicated module. The class-local tracker is renamed around private-member usage so it cannot be confused with semantic symbol state.

The PR tip passed the full oxc_minifier suite independently. The completed stack also passed clippy with warnings denied, formatting, minsize and allocation regeneration, and just ready.

Stack

  1. fix(minifier): avoid stale pure function summaries #24636 — function-summary correctness
  2. refactor(minifier): model symbol effects explicitly #24637 — explicit symbol effect types
  3. refactor(minifier): group symbol reference counts #24638 — grouped reference counts
  4. refactor(minifier): merge persistent symbol metadata #24639 — merged persistent metadata
  5. refactor(minifier): group symbol state #24640 — SymbolState facade

Implemented with AI assistance (OpenAI Codex). The contributor remains responsible for reviewing, understanding, and submitting the changes under the repository AI usage policy.

Dunqing commented Jul 18, 2026

Copy link
Copy Markdown
Member Author

How to use the Graphite Merge Queue

Add either label to this PR to merge it via the merge queue:

  • 0-merge - adds this PR to the back of the merge queue
  • hotfix - for urgent changes, fast-track this PR to the front of the merge queue

You must have a Graphite account in order to use the merge queue. Sign up using this link.

An organization admin has enabled the Graphite Merge Queue in this repository.

Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue.

This stack of pull requests is managed by Graphite. Learn more about stacking.

@Dunqing Dunqing changed the title refactor(minifier): group minifier symbol state refactor(minifier): group symbol state Jul 18, 2026
@Dunqing Dunqing added the run-monitor-oxc Add to a PR to dispatch oxc-project/monitor-oxc CI against it label Jul 18, 2026
@oxc-guard

oxc-guard Bot commented Jul 18, 2026

Copy link
Copy Markdown
Contributor

@oxc-guard oxc-guard Bot removed the run-monitor-oxc Add to a PR to dispatch oxc-project/monitor-oxc CI against it label Jul 18, 2026
@codspeed-hq

codspeed-hq Bot commented Jul 18, 2026

Copy link
Copy Markdown

Merging this PR will not alter performance

✅ 57 untouched benchmarks
⏩ 19 skipped benchmarks1


Comparing codex/minifier-symbol-state-facade (f0893aa) with codex/minifier-persistent-symbol-metadata (00f1815)2

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.

  2. No successful run was found on codex/minifier-persistent-symbol-metadata (b38c502) during the generation of this report, so ec6ead1 was used instead as the comparison base. There might be some changes unrelated to this pull request in this report.

@Dunqing
Dunqing force-pushed the codex/minifier-persistent-symbol-metadata branch from 327de79 to baf8949 Compare July 18, 2026 07:11
@Dunqing
Dunqing force-pushed the codex/minifier-symbol-state-facade branch from c72d3ae to 96cfca6 Compare July 18, 2026 07:11
@armano2

armano2 commented Jul 18, 2026

Copy link
Copy Markdown
Contributor

could you add 2 missing edge cases, this will check if liveliness re-computation works correctly

    test_same("var alias = c; function c() { d() } function d() { c() } sideEffect(alias);");
    test("let alias = c; function c() { d() } function d() { c() } sideEffect();", "sideEffect();");

ohh, maybe not in this PR, but higher in stack

@Dunqing
Dunqing force-pushed the codex/minifier-symbol-state-facade branch from 96cfca6 to de61355 Compare July 18, 2026 15:38
@Dunqing

Dunqing commented Jul 18, 2026

Copy link
Copy Markdown
Member Author

could you add 2 missing edge cases, this will check if liveliness re-computation works correctly

    test_same("var alias = c; function c() { d() } function d() { c() } sideEffect(alias);");
    test("let alias = c; function c() { d() } function d() { c() } sideEffect();", "sideEffect();");

ohh, maybe not in this PR, but higher in stack

You meant #24125? It has been merged; feel free to send a PR to add these tests.

@Dunqing
Dunqing force-pushed the codex/minifier-symbol-state-facade branch from de61355 to f0893aa Compare July 18, 2026 16:25
@Dunqing
Dunqing force-pushed the codex/minifier-persistent-symbol-metadata branch from 00f1815 to b38c502 Compare July 18, 2026 16:25
@Dunqing Dunqing added the 0-merge Merge with Graphite Merge Queue label Jul 18, 2026

Dunqing commented Jul 18, 2026

Copy link
Copy Markdown
Member Author

Merge activity

`MinifierState` separately exposed dense per-pass values, sparse persistent metadata, and optional reachability state even though all three are indexed by semantic symbols. Its private-member stack also used symbol terminology despite storing names rather than `SymbolId`s.

This introduces a `SymbolState` facade that documents the three storage lifecycles and forwards their operations while leaving the reachability algorithm in its dedicated module. The class-local tracker is renamed around private-member usage so it cannot be confused with semantic symbol state.

The PR tip passed the full `oxc_minifier` suite independently. The completed stack also passed clippy with warnings denied, formatting, minsize and allocation regeneration, and `just ready`.

## Stack

1. #24636 — function-summary correctness
2. #24637 — explicit symbol effect types
3. #24638 — grouped reference counts
4. #24639 — merged persistent metadata
5. **#24640 — SymbolState facade**

Implemented with AI assistance (OpenAI Codex). The contributor remains responsible for reviewing, understanding, and submitting the changes under the repository AI usage policy.
graphite-app Bot pushed a commit that referenced this pull request Jul 18, 2026
Pure-function summaries trusted resolved read and write references, but redeclarations, direct `eval`, and classic Script globals can replace a binding without producing such a reference. A later call could therefore be folded according to an earlier empty declaration and lose runtime side effects.

## Example

```js
function foo() {}
eval("foo = () => side_effect()")
foo()
```

Bindings with redeclarations, bindings visible to direct eval, and classic Script-root bindings do not receive a reusable summary; the eval restriction is intentionally non-sticky so removing the final eval can enable the optimization in a later pass. Module and CommonJS bindings retain the existing fold.

The PR tip passed the full `oxc_minifier` suite independently. The completed stack also passed clippy with warnings denied, formatting, minsize and allocation regeneration, and `just ready`.

## Stack

1. **#24636 — function-summary correctness**
2. #24637 — explicit symbol effect types
3. #24638 — grouped reference counts
4. #24639 — merged persistent metadata
5. #24640 — SymbolState facade

Implemented with AI assistance (OpenAI Codex). The contributor remains responsible for reviewing, understanding, and submitting the changes under the repository AI usage policy.
graphite-app Bot pushed a commit that referenced this pull request Jul 18, 2026
Pure-call summaries and member-write hazards encoded semantic states through nested `Option` values and independent bitflags. Reviewers had to reconstruct which combinations were valid and which implications producers must maintain.

This introduces `FunctionSummary` for reusable call proofs and an ordered `MemberWriteEffect` for `None < Hazard < MayMutatePrototype`. Prototype mutation is therefore hazardous by construction, the removed bitflag dependency is no longer needed, and producers and consumers share named contracts. Behavior is unchanged relative to #24636.

The PR tip passed the full `oxc_minifier` suite independently. The completed stack also passed clippy with warnings denied, formatting, minsize and allocation regeneration, and `just ready`.

## Stack

1. #24636 — function-summary correctness
2. **#24637 — explicit symbol effect types**
3. #24638 — grouped reference counts
4. #24639 — merged persistent metadata
5. #24640 — SymbolState facade

Implemented with AI assistance (OpenAI Codex). The contributor remains responsible for reviewing, understanding, and submitting the changes under the repository AI usage policy.
graphite-app Bot pushed a commit that referenced this pull request Jul 18, 2026
`SymbolValue` exposed three related counters directly, so every consumer had to coordinate read, write, and member-write-target invariants itself.

This groups them in `ReferenceCounts` and exposes factual queries such as `has_single_read` and `has_only_member_write_target_reads`. The existing 48-byte `SymbolValue` layout is preserved, and the refactor does not change minified output.

The PR tip passed the full `oxc_minifier` suite independently. The completed stack also passed clippy with warnings denied, formatting, minsize and allocation regeneration, and `just ready`.

## Stack

1. #24636 — function-summary correctness
2. #24637 — explicit symbol effect types
3. **#24638 — grouped reference counts**
4. #24639 — merged persistent metadata
5. #24640 — SymbolState facade

Implemented with AI assistance (OpenAI Codex). The contributor remains responsible for reviewing, understanding, and submitting the changes under the repository AI usage policy.
graphite-app Bot pushed a commit that referenced this pull request Jul 18, 2026
Pure-call summaries and member-write effects used separate sparse hash maps even though both are keyed by `SymbolId` and remain valid across peephole iterations.

This stores both values in one `PersistentSymbolMetadata` entry. Summary replacement and monotone member-effect strengthening remain independent, and clearing a dynamic-binding summary updates an existing entry without allocating or discarding its member effect.

The regenerated allocation snapshot records fewer system allocations: checker 155→152, pdf 2469→2467, antd 1044→1038, and kitchen-sink 1872→1867. `minsize.snap` remains byte-identical.

The PR tip passed the full `oxc_minifier` suite independently. The completed stack also passed clippy with warnings denied, formatting, minsize and allocation regeneration, and `just ready`.

## Stack

1. #24636 — function-summary correctness
2. #24637 — explicit symbol effect types
3. #24638 — grouped reference counts
4. **#24639 — merged persistent metadata**
5. #24640 — SymbolState facade

Implemented with AI assistance (OpenAI Codex). The contributor remains responsible for reviewing, understanding, and submitting the changes under the repository AI usage policy.
@graphite-app
graphite-app Bot force-pushed the codex/minifier-persistent-symbol-metadata branch from b38c502 to 7492af4 Compare July 18, 2026 16:54
@graphite-app
graphite-app Bot requested a review from overlookmotel as a code owner July 18, 2026 16:54
@graphite-app
graphite-app Bot force-pushed the codex/minifier-symbol-state-facade branch from f0893aa to 4eae149 Compare July 18, 2026 16:55
@graphite-app graphite-app Bot removed the 0-merge Merge with Graphite Merge Queue label Jul 18, 2026
Base automatically changed from codex/minifier-persistent-symbol-metadata to main July 18, 2026 17:00
@graphite-app
graphite-app Bot merged commit 4eae149 into main Jul 18, 2026
31 checks passed
@graphite-app
graphite-app Bot deleted the codex/minifier-symbol-state-facade branch July 18, 2026 17:01
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