refactor(treeshake): split include_statements.rs into focused modules#10095
Conversation
How to use the Graphite Merge QueueAdd 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. |
1bfa680 to
2d421d2
Compare
b1d7dde to
fdb678e
Compare
068c181 to
0343dd3
Compare
0343dd3 to
a344d7d
Compare
✅ Deploy Preview for rolldown-rs canceled.
|
a344d7d to
2bacc75
Compare
Merging this PR will not alter performance
Comparing Footnotes
|
Merge activity
|
…#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
2bacc75 to
3325bf7
Compare
…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
…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
…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
## [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]>

Description
Part 1 of the tree-shaking inclusion refactor stack.
include_statements.rshad 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,evalhandling, 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:
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,preserveModulesre-export interface preservation)include_module/include_symbol/include_statementbecomes 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 dedupesinclude_declaring_statements, which replaces five hand-rolled copies of the "a used binding keeps its declaring statements" loop.itertools::Itertoolsimport that wasm targets need forzip_eq(rolldown_utils::rayonshimspar_iter()to a plainIteratorthere); caught by the browser/WASI CI jobs.No behavior change: byte-identical output across the integration suite (zero snapshot diffs), clippy clean,
wasm32-wasip1-threadschecked.Stack: #10095 → #10096 → #10097 → #10098