Skip to content

refactor(treeshake): single-source the own-export classification shared with the lazy-barrel loader#10098

Merged
graphite-app[bot] merged 1 commit into
mainfrom
refactor/treeshake-4-shared-export-origin
Jul 6, 2026
Merged

refactor(treeshake): single-source the own-export classification shared with the lazy-barrel loader#10098
graphite-app[bot] merged 1 commit into
mainfrom
refactor/treeshake-4-shared-export-origin

Conversation

@IWANABETHATGUY

@IWANABETHATGUY IWANABETHATGUY commented Jul 2, 2026

Copy link
Copy Markdown
Member

Description

Part 4 of the tree-shaking inclusion refactor stack (stacked on #10097).

The lazy-barrel loader and tree shaking each hand-rolled the same "is this export the module's own declaration or a re-export of an import?" lookup:

  • loader: try_extract_lazy_barrel_info builds BarrelInfo::local from it, and take_needed_records loads every plain import record of a barrel as soon as one of its own exports is requested
  • shaker: compute_body_demand_keys builds the body-demand keys from it, and a module's gated side-effect statements join as soon as one of its own exports is used

Those two sides must agree: if they classified an export differently, a retained statement could reference an import record that was never loaded — a free identifier at runtime. That disagreement class is exactly the #9806 bug family (#9691, #9806, #9961, #9964, #10013, #10048).

The classification now lives in one place — EcmaView::classify_export → ExportOrigin { Own, ReExport(&NamedImport) } in rolldown_common — consumed by both sides, so the agreement holds by construction and the invariant is documented at the definition site instead of in two mirror comments.

Scope note: the demand semantics (take_needed_records on the loader side vs body-demand sweeping on the shaker side) remain parallel implementations documented against each other; unifying those is future work.

No behavior change: byte-identical output across the integration suite, zero snapshot diffs.

Stack: #10095#10096#10097#10098

IWANABETHATGUY commented Jul 2, 2026

Copy link
Copy Markdown
Member Author

How to use the Graphite Merge Queue

Add the label graphite: merge-when-ready to this PR to add it to 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.

@IWANABETHATGUY
IWANABETHATGUY force-pushed the refactor/treeshake-3-body-demand-bit branch from 2b07c2c to 3534078 Compare July 2, 2026 13:23
@IWANABETHATGUY
IWANABETHATGUY force-pushed the refactor/treeshake-4-shared-export-origin branch 2 times, most recently from f42f241 to 7c7b0d0 Compare July 2, 2026 14:02
@IWANABETHATGUY
IWANABETHATGUY force-pushed the refactor/treeshake-3-body-demand-bit branch from 3534078 to d8c011d Compare July 2, 2026 14:02
@IWANABETHATGUY
IWANABETHATGUY force-pushed the refactor/treeshake-4-shared-export-origin branch from 7c7b0d0 to aeda0fc Compare July 2, 2026 14:07
@IWANABETHATGUY
IWANABETHATGUY force-pushed the refactor/treeshake-3-body-demand-bit branch from d8c011d to 0aec1aa Compare July 2, 2026 14:07
@IWANABETHATGUY
IWANABETHATGUY force-pushed the refactor/treeshake-4-shared-export-origin branch from aeda0fc to 96a69dd Compare July 2, 2026 15:40
@IWANABETHATGUY
IWANABETHATGUY force-pushed the refactor/treeshake-3-body-demand-bit branch 2 times, most recently from 566c3c0 to 68a076c Compare July 2, 2026 16:39
@IWANABETHATGUY
IWANABETHATGUY force-pushed the refactor/treeshake-4-shared-export-origin branch 2 times, most recently from 2be9b57 to 2de7f30 Compare July 2, 2026 18:17
@IWANABETHATGUY
IWANABETHATGUY force-pushed the refactor/treeshake-3-body-demand-bit branch from 68a076c to b88ce97 Compare July 2, 2026 18:17
graphite-app Bot pushed a commit that referenced this pull request Jul 2, 2026
…#10095)

### Description

Part 1 of the tree-shaking inclusion refactor stack.

`include_statements.rs` had grown to ~1200 lines interleaving the inclusion fixpoint driver, the three mutually recursive inclusion functions, dynamic-entry topological ordering, the body-demand gating model, and several standalone passes — with 10+ orthogonal policies (CJS bailouts, constant/enum inlining bypasses, JSON self-reference tracking, `eval` handling, HMR namespace registration, property-write retention) inlined directly into the recursion. Reviewing any change meant re-deriving which clause belonged to which concern; every bug fix added another inline branch.

Pure code motion, in three commits:

- **Split into focused modules**
  - `dynamic_entries.rs` — dynamic-entry topological ordering (tarjan), aliveness determination, per-iteration processing (~200 lines unrelated to inclusion mechanics)
  - `on_demand.rs` — the body-demand gating model (`side_effects_included_on_demand`, `compute_on_demand_side_effect_stmts`, `is_gated_side_effect_stmt`)
  - `passes.rs` — standalone passes around the fixpoint (CJS bailout export inclusion, runtime-helper collection/inclusion, `preserveModules` re-export interface preservation)
- **Extract named policies** — each inline block of `include_module` / `include_symbol` / `include_statement` becomes a named function with a doc contract (`sweep_side_effect_statements`, `include_side_effectful_dependencies`, `is_bypassed_inlined_constant`, `drain_body_demand_stmts`, `follow_cjs_namespace_alias`, `note_namespace_inclusion_reason`, `demand_esm_init_wrapper`, `note_json_self_reference`, `include_property_write_referencing_stmts`, `scan_import_records_for_cjs_bailout`, `is_inlined_enum_member_access`), so the three skeletons read as a sequence of policies. Also dedupes `include_declaring_statements`, which replaces five hand-rolled copies of the "a used binding keeps its declaring statements" loop.
- **wasm build fix** — the move dropped an incidental `itertools::Itertools` import that wasm targets need for `zip_eq` (`rolldown_utils::rayon` shims `par_iter()` to a plain `Iterator` there); caught by the browser/WASI CI jobs.

No behavior change: byte-identical output across the integration suite (zero snapshot diffs), clippy clean, `wasm32-wasip1-threads` checked.

Stack: #10095#10096#10097#10098
@graphite-app
graphite-app Bot force-pushed the refactor/treeshake-3-body-demand-bit branch from b88ce97 to 1ce181e Compare July 2, 2026 20:49
@graphite-app
graphite-app Bot force-pushed the refactor/treeshake-4-shared-export-origin branch from 2de7f30 to a63c2cd Compare July 2, 2026 20:49
graphite-app Bot pushed a commit that referenced this pull request Jul 3, 2026
…st engine (#10096)

### Description

Part 2 of the tree-shaking inclusion refactor stack (stacked on #10095).

Replaces the `include_module` ↔ `include_symbol` ↔ `include_statement` mutual recursion with an explicit worklist engine. Edge producers push typed work items — `WorkItem::Module(idx)` (structural inclusion: sweep side-effect statements, evaluate side-effectful deps), `WorkItem::Symbol(ref, reason)` (a symbol became used: retain its declaration, wrapper, owner module), `WorkItem::Statement(idx, stmt)` (include one statement and follow its references) — onto a queue that the public `include_*` entry points drain to empty before returning. Handlers and internal helpers only push; only the public entry points drain, so drains never nest.

Why:

- no stack-depth limit on deep module graphs (the recursion could overflow)
- inclusion order becomes a visible queue discipline instead of an implicit call stack — the class of traversal-ordering bugs becomes inspectable and testable
- every inclusion flows through one loop, giving a single choke point for future reason/provenance tracking

Correctness argument: the final inclusion sets are a monotone closure over the pushed edges (all handler dedup is idempotent bit-setting; reason flags are ORed), so drain order affects only traversal order, never results. LIFO drain mirrors the depth-first shape of the recursion it replaces. Confirmed empirically: byte-identical output across the integration suite, zero snapshot diffs.

Public API is unchanged — `passes.rs`, `dynamic_entries.rs`, and `chunk_optimizer.rs` keep calling the same `include_*` functions, apart from the one new context field.

#### Review follow-up: queue-invariant hardening

The drain-before-return invariant made helper boundaries subtle — especially `include_declaring_statements`, which was enqueue-only yet callable from outside the engine (driver entry loop, dynamic entries), legitimately pre-seeding the queue across a public entry. Addressed:

- split it: `enqueue_declaring_statements` (private, engine-internal, used by handlers) vs `include_declaring_statements` (`pub(super)`, now drains like every other public entry) — so "queue empty between public entries" is a true invariant, not a mostly-true one
- `debug_assert!` at every public entry point that the queue is empty on entry and after draining; the full fixture suite runs with these live
- `WorkItem` and the `pending` field narrowed to `pub(in crate::stages)` (the minimum: `chunk_optimizer` constructs `IncludeContext` with a struct literal), documented as not-API

Stack: #10095#10096#10097#10098
@graphite-app
graphite-app Bot force-pushed the refactor/treeshake-3-body-demand-bit branch 2 times, most recently from 227e7e4 to 1b15096 Compare July 3, 2026 03:02
@graphite-app
graphite-app Bot force-pushed the refactor/treeshake-4-shared-export-origin branch from a63c2cd to 20db47f Compare July 3, 2026 03:02
@graphite-app
graphite-app Bot changed the base branch from refactor/treeshake-3-body-demand-bit to graphite-base/10098 July 3, 2026 06:01
graphite-app Bot pushed a commit that referenced this pull request Jul 3, 2026
…a stmt multimap (#10097)

### Description

Part 3 of the tree-shaking inclusion refactor stack (stacked on #10096).

#### Before

The body-demand machinery stored, for **every** body-demand key of a module, a full copy of that module's gated side-effect statement list (`FxHashMap<SymbolRef, Vec<(ModuleIdx, StmtInfoIdx)>>`), drained by map removal. That one mutable map entangled three different things: the **demand edge** (key → module), the **gate predicate** (which statements are gated), and the **progress state** (encoded as entry presence/absence). A module with N own exports carried N copies of the same statement list, and "has this module's body already joined?" was emergent from which keys happened to have been removed.

#### After

The rule being implemented — *a `sideEffects: false` module's gated statements join once its body is demanded* — is a boolean event on a **module**, and the state now says exactly that. Module inclusion is two explicit bits:

- `is_module_included_vec` — structural inclusion (wrapper/interop resolvable, side-effectful deps evaluated); unchanged
- `body_demand_swept` — the module's gated side-effect statements have joined because one of its own exports / its namespace object was actually used

and each of the three entangled concerns lives where it belongs:

- **Demand edge** — `body_demand_keys: FxHashMap<SymbolRef, ModuleIdx>` ("whose body does using this symbol demand?"), immutable and shared across both `IncludeContext` constructions (link stage and chunk-optimizer replay); no more `&mut` threading. Every key's canonical is owned by the module itself (own exports and namespace refs never link across modules). Before, `entry(key).or_default().extend(..)` would have silently merged two modules' statements if that invariant ever broke; now it's an explicit `debug_assert!` exercised by the whole suite.
- **Gate predicate** — the gated statements are enumerated at demand time from the immutable `stmt_infos`, via the same `is_gated_side_effect_stmt` used when building the key map. One source of truth; no O(keys × gated stmts) materialized copies.
- **Progress** — `body_demand_swept: FxHashSet<ModuleIdx>`, owned per run. `insert()` returning `false` short-circuits, so the sweep happens exactly once per **module**. Before, `remove(&key)` only guaranteed once per *key*: demanding a second export of the same module re-enqueued all its statements (harmless only because `include_statement` is idempotent — you had to know that to trust it). The generate-stage replay gets shared edges plus a fresh swept set: graph-scoped facts vs run-scoped progress.

#### Why `FxHashMap`/`FxHashSet` rather than `IndexBitSet`

`is_module_included_vec` is already an `IndexBitSet` — dense domain, touched for essentially every module on the hot path. The two body-demand structures sit on the other side of that line: they are sparse, usually **empty**. A module contributes keys only if it is user-declared side-effect-free, ESM, non-entry, without `eval`, **and** has a top-level side-effecting statement that reads module-level bindings — the rare #9806-family pattern.

- `body_demand_keys` can't be dense-indexed at all: `SymbolRef` is a two-dimensional key (`owner` × `SymbolId`), so a dense encoding would cost O(total symbols in the graph). The map *is* probed once per symbol in `include_symbol`, but a `get` on an empty `FxHashMap` is a few instructions — the common case stays effectively free.
- `body_demand_swept` is constructed fresh per `IncludeContext` (twice in the link stage, again in the chunk-optimizer replay). `IndexBitSet::new(modules.len())` would allocate and zero the full module range each time to record what is almost always nothing; `FxHashSet::default()` allocates nothing until the first insert, and it is only touched after a `body_demand_keys` hit. Its cardinality is bounded by the number of gated modules, so both structures are zero-cost together in the common case.
- Neither structure is ever iterated (membership test / probe only), so hash iteration order cannot leak into output.

#### Explored and rejected

Deriving `is_module_included` from the statement bits. The `SymbolIncludeReason::SimulatedFacadeChunk` path (chunk optimizer) deliberately includes statements **without** structurally including the module, so a derived bit is a behavior change, not a refactor.

No behavior change: byte-identical output across the integration suite, zero snapshot diffs.

Stack: #10095#10096#10097#10098
@graphite-app
graphite-app Bot force-pushed the refactor/treeshake-4-shared-export-origin branch from 20db47f to 98f99ed Compare July 3, 2026 06:05
@graphite-app
graphite-app Bot force-pushed the graphite-base/10098 branch from 1b15096 to 37073a1 Compare July 3, 2026 06:05
@graphite-app
graphite-app Bot changed the base branch from graphite-base/10098 to main July 3, 2026 06:06
@graphite-app
graphite-app Bot force-pushed the refactor/treeshake-4-shared-export-origin branch from 98f99ed to e117209 Compare July 3, 2026 06:06
@netlify

netlify Bot commented Jul 3, 2026

Copy link
Copy Markdown

Deploy Preview for rolldown-rs canceled.

Name Link
🔨 Latest commit 04fcbc1
🔍 Latest deploy log https://app.netlify.com/projects/rolldown-rs/deploys/6a4b2299ec9e15000823def1

@IWANABETHATGUY

Copy link
Copy Markdown
Member Author

Could you take a look, @shulaoda? Since the code between tree shaking and lazyBarrel has been consolidated, I'm not sure it is necessary.

@IWANABETHATGUY
IWANABETHATGUY marked this pull request as ready for review July 3, 2026 06:14
@codspeed-hq

codspeed-hq Bot commented Jul 3, 2026

Copy link
Copy Markdown

Merging this PR will not alter performance

✅ 7 untouched benchmarks
⏩ 10 skipped benchmarks1


Comparing refactor/treeshake-4-shared-export-origin (e117209) with main (37073a1)

Open in CodSpeed

Footnotes

  1. 10 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.

shulaoda commented Jul 6, 2026

Copy link
Copy Markdown
Member

Merge activity

  • Jul 6, 3:35 AM UTC: The merge label 'graphite: merge-when-ready' was detected. This PR will be added to the Graphite merge queue once it meets the requirements.
  • Jul 6, 3:35 AM UTC: shulaoda added this pull request to the Graphite merge queue.
  • Jul 6, 3:39 AM UTC: Merged by the Graphite merge queue.

…ed with the lazy-barrel loader (#10098)

### Description

Part 4 of the tree-shaking inclusion refactor stack (stacked on #10097).

The lazy-barrel loader and tree shaking each hand-rolled the same "is this export the module's own declaration or a re-export of an import?" lookup:

- loader: `try_extract_lazy_barrel_info` builds `BarrelInfo::local` from it, and `take_needed_records` loads every plain import record of a barrel as soon as one of its **own** exports is requested
- shaker: `compute_body_demand_keys` builds the body-demand keys from it, and a module's gated side-effect statements join as soon as one of its **own** exports is used

Those two sides must agree: if they classified an export differently, a retained statement could reference an import record that was never loaded — a free identifier at runtime. That disagreement class is exactly the #9806 bug family (#9691, #9806, #9961, #9964, #10013, #10048).

The classification now lives in one place — `EcmaView::classify_export → ExportOrigin { Own, ReExport(&NamedImport) }` in `rolldown_common` — consumed by both sides, so the agreement holds by construction and the invariant is documented at the definition site instead of in two mirror comments.

Scope note: the demand *semantics* (`take_needed_records` on the loader side vs body-demand sweeping on the shaker side) remain parallel implementations documented against each other; unifying those is future work.

No behavior change: byte-identical output across the integration suite, zero snapshot diffs.

Stack: #10095#10096#10097#10098
@graphite-app
graphite-app Bot force-pushed the refactor/treeshake-4-shared-export-origin branch from e117209 to 04fcbc1 Compare July 6, 2026 03:35
@graphite-app
graphite-app Bot merged commit 04fcbc1 into main Jul 6, 2026
34 checks passed
@graphite-app
graphite-app Bot deleted the refactor/treeshake-4-shared-export-origin branch July 6, 2026 03:39
@rolldown-guard rolldown-guard Bot mentioned this pull request Jul 8, 2026
shulaoda added a commit that referenced this pull request Jul 8, 2026
## [1.1.5] - 2026-07-08

### 🚀 Features

- detect top-level import-binding reads as execution-order sensitive (#10180) by @hyf0
- sourcemap_filenames: add a sourcemapFileNames option (#9271) by @V1OL3TF0X
- binding: record plugin hook result kind in tracing spans (#10154) by @IWANABETHATGUY
- linking: skip side-effect-free modules in per-entry reachability (#10111) by @IWANABETHATGUY
- improve error message for unresolved virtual imports (#10156) by @sapphi-red
- add descriptive metadata to plugin API (#10106) by @sapphi-red
- add `--configLoader=native` option (#10118) by @sapphi-red

### 🐛 Bug Fixes

- improve invalid annotation warnings (#10185) by @hyf0
- keep deduplicated asset filenames stable once they can be observed (#10191) by @shulaoda
- sourcemap_filenames: use public option name in pattern errors (#10188) by @IWANABETHATGUY
- sourcemap_filenames: hash prepared sourcemap content (#10178) by @hyf0
- tree-shake unused circular declarators exported via export list (#10166) by @IWANABETHATGUY
- dev: don't panic when an HMR rebuild hits an unresolved import (#10162) by @shulaoda
- propagate errors from output.globals function (#9880) by @shulaoda
- dev: revert cache mutations when a partial scan fails (#10110) by @shulaoda
- dev: update importer relationships of cached modules in incremental build (#10107) by @shulaoda
- hmr: fall back to full reload when a changed module is not registered as executed (#10132) by @shulaoda
- chunk-optimizer: follow entry facade edges in runtime placement cycle check (#10101) by @hyf0
- dev: ignore watcher events after close (#10113) by @hyf0
- emit async wrapper for TLA modules under onDemandWrapping (#10086) by @IWANABETHATGUY
- gate sideEffects:false modules' side effects on body demand (#10080) by @IWANABETHATGUY
- rolldown_plugin_vite_resolve: return empty object for `browser: false` mapped modules (#10082) by @sapphi-red
- reset the word-boundary state on newline in Hires::Boundary sourcemaps (#10025) by @shulaoda
- trim an emptied chunk's outro/intro instead of skipping past it (#10029) by @shulaoda
- test each edited chunk's own start against indent exclude ranges (#10026) by @shulaoda
- preserve sourcemap mappings for indented lines when a CJS module shares the chunk (#10074) by @hyf0

### 🚜 Refactor

- separate tree-shaking side effects from execution order sensitivity (#10168) by @hyf0
- type construct_vite_preload_call to take an ObjectPattern (#10135) by @shulaoda
- treeshake: single-source the own-export classification shared with the lazy-barrel loader (#10098) by @IWANABETHATGUY
- dev: reuse Vite's bundledDev server (#10081) by @h-a-n-a
- clippy: ban std HashMap/HashSet in favour of FxHashMap/FxHashSet (#10108) by @Boshen
- treeshake: make body demand a second module bit instead of a stmt multimap (#10097) by @IWANABETHATGUY
- seal used_symbol_refs by construction after its last writer (#10091) by @hyf0
- treeshake: replace inclusion mutual recursion with a worklist engine (#10096) by @IWANABETHATGUY
- treeshake: split include_statements.rs into focused modules (#10095) by @IWANABETHATGUY
- drop redundant is_user_defined filter on partitioned entries (#10050) by @shulaoda
- project the retained export interface out of used_symbol_refs (#10089) by @hyf0
- track used external symbols separately from used_symbol_refs (#10088) by @hyf0
- make module namespace inclusion an explicit linking metadata field (#10087) by @hyf0
- rename statement evaluation metadata (#10078) by @hyf0

### 📚 Documentation

- virtual modules user-facing id convention (#10155) by @sapphi-red
- cli: clarify disabling boolean/object flags like codeSplitting (#10153) by @IWANABETHATGUY
- chore: remove Vite+ alpha banner (#10105) by @mdong1909
- write down the used_symbol_refs contract (#10090) by @hyf0
- dev/lazy: update design and implementation (#10079) by @h-a-n-a

### ⚡ Performance

- ast_scanner: stop order-sensitivity checks once a module is flagged (#10190) by @IWANABETHATGUY
- return impl ExactSizeIterator from slice-backed accessors (#10133) by @Boshen
- binding: box dev and watcher napi futures (#10103) by @Boshen

### 🧪 Testing

- move string_wizard replace unit tests to the JS magic-string suite (#10176) by @IWANABETHATGUY
- dev: assert incremental scan state matches a fresh full build after each HMR step (#10115) by @shulaoda
- dev: restore runtime assertions of delete_file_not_used_anymore (#10112) by @shulaoda
- dev: fix flaky dev server tests in CI (#10152) by @h-a-n-a
- add regression test for #10099 (lazyBarrel drops default-import binding but keeps its property reads) (#10109) by @IWANABETHATGUY

### ⚙️ Miscellaneous Tasks

- deploy website to Void via GitHub OIDC (#10192) by @Boshen
- deps: update oxc to 0.139.0 (#10161) by @shulaoda
- deps: update test262 submodule for tests (#10160) by @rolldown-guard[bot]
- rolldown_plugin_utils: remove dead asset-url and css scaffolding (#10131) by @shulaoda
- deps: revert vite-plus to v0.2.1 (#10148) by @shulaoda
- deps: update github actions (#10141) by @renovate[bot]
- deps: update dependency rust to v1.96.1 (#10145) by @renovate[bot]
- deps: update npm packages (#10142) by @renovate[bot]
- deps: update rust crates (#10143) by @renovate[bot]
- deps: update napi to v3.10.3 (#10121) by @renovate[bot]
- rolldown_utils: remove unused time module (#10138) by @shulaoda
- remove dead CopyModulePlugin::is_active method (#10129) by @shulaoda
- remove dead LazyCompilationContext::is_lazy_module method (#10128) by @shulaoda
- remove dead BuildDiagnostic::downcast_ref method (#10127) by @shulaoda
- deps: update dependency vite-plus to v0.2.2 (#10084) by @renovate[bot]
- deps: update rust crate oxc_sourcemap to v8.1.0 (#10122) by @renovate[bot]
- deps: update crate-ci/typos action to v1.48.0 (#10124) by @renovate[bot]
- enable more clippy restriction lints (#10114) by @Boshen
- deps: update rust dependencies (#10100) by @Boshen
- deps: update oxc resolver to v11.23.0 (#10083) by @renovate[bot]

### ◀️ Revert

- Revert "chore(deps): revert vite-plus to v0.2.1" (#10157) by @h-a-n-a
- "fix(hmr): fall back to full reload when a changed module is not registered as executed (#10132)" (#10151) by @shulaoda

### ❤️ New Contributors

* @V1OL3TF0X made their first contribution in [#9271](#9271)

Co-authored-by: shulaoda <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants