fix(code-splitting): re-derive chunk exec order after the runtime sweep#10334
Merged
Merged
Conversation
hyfdev
force-pushed
the
codex/flag-off-runtime-sweep-chunk-order
branch
from
July 17, 2026 08:18
39bb381 to
4957c8a
Compare
This was referenced Jul 17, 2026
hyfdev
force-pushed
the
codex/flag-off-runtime-sweep-chunk-order
branch
from
July 18, 2026 05:35
4957c8a to
cbe7f25
Compare
hyfdev
force-pushed
the
codex/flag-off-runtime-sweep-chunk-order
branch
from
July 18, 2026 05:48
cbe7f25 to
f8ec80d
Compare
✅ Deploy Preview for rolldown-rs ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
hyfdev
force-pushed
the
codex/flag-off-runtime-sweep-chunk-order
branch
3 times, most recently
from
July 19, 2026 12:49
40e86d8 to
0f69617
Compare
hyfdev
marked this pull request as ready for review
July 19, 2026 12:51
Merging this PR will not alter performance
Comparing Footnotes
|
shulaoda
approved these changes
Jul 19, 2026
Member
Merge activity
|
IWANABETHATGUY
approved these changes
Jul 19, 2026
…ep (#10334) ## Summary Flag-off builds could emit cross-chunk imports — and therefore run side effects — in a different order than `main`, on the default path. When the post-chunking runtime sweep drops the runtime module out of a shared chunk it co-hosts with user modules, that chunk was left with an execution order keyed on the removed runtime (exec order `0`) instead of its first surviving module, so it sorted ahead of chunks it should have followed. This restores `main`'s ordering by re-deriving chunk execution orders after the sweep. ## Verified mechanism - Before #10104 (`1d86ffe06`), `sweep_unused_runtime_module` ran in `generate_chunks` **before** chunk exec-order assignment (there was a comment requiring it). #10104 moved the sweep into `finalize_chunk_plan`, which runs **after** assignment, so the regression is live on `main` today. - The Common-chunk comparator keys on `modules[0]`'s exec order; `sort_chunk_modules` always makes the runtime (module exec order `0`) `modules[0]`. So a chunk hosting the runtime keys on `0`. - When the walk-back (`find_entry_level_external_module`) invalidates the runtime demand (the #9374 star-re-export-to-external shape) and the sweep strips the runtime from a still-live shared chunk, the chunk keeps `exec_order` derived from the now-absent runtime. `rebuild_sorted_chunk_idx_vec(true)` only re-sorts the stale values; `renumber_live_chunks` is strict-only. That stale key feeds both `sorted_chunk_idx_vec` and the cross-chunk import sort (`compute_cross_chunk_links.rs:110/124`), so an entry emits `import "./C.js"` before a chunk that should run first. ## Fix Chunk exec orders must already be assigned before order analysis runs (the on-demand actual-order simulation sorts sibling chunks by `exec_order`), so assignment cannot simply move after the sweep. Instead: - Extract the assignment loop into `assign_chunk_exec_orders` (`code_splitting.rs`); keep the provisional call where it was in `generate_chunks`. - Re-run it in `finalize_chunk_plan` when the sweep actually removed the runtime from a chunk — the same condition that already triggered `rebuild_sorted_chunk_idx_vec(true)` — then rebuild the sorted vec. **Final ordering contract (documented on `assign_chunk_exec_orders`).** Flag-off, `analyze_execution_order` returns `None`, so no lowering and no `renumber_live_chunks` run; the re-run is the sole authority and reproduces exactly what `main` produced by sweeping before assignment. Under strict, the sweep block only runs when `required_runtime_helpers()` is empty (no order wrapper demands the runtime); `renumber_live_chunks` (strict-only, after lowering) merely *compacts* existing `exec_order` values to close tombstone gaps and never re-derives them from `modules[0]`. The re-run happens after lowering, so whichever of the two touches `exec_order` last leaves every live chunk both densely numbered and keyed on its correct lead module — they never contend. ## Behavior / snapshot impact - **New fixture only.** `topics/runtime/sweep_shared_chunk_exec_order` — three entries; `re.js` (`export * from 'external'`) makes tree-shaking include the runtime for `__reExport`, and the runtime folds into `re.js`'s shared `{entry_a, entry_b}` chunk. The walk-back flattens the star re-export to a chunk-level `export * from "external"`, so the post-order sweep drops the runtime from that still-live shared chunk. `entry_a` also imports the earlier side-effect chunk `s_ac.js` (shared `{entry_a, entry_c}`). - **Main-parity proof.** The fixture was built against three states. Pre-#10104 `main` (`ce98ae798`) emits `import "./s_ac.js"` before `import "./re.js"`; current `main` emits `import "./re.js"` first (the runtime-hosting chunk keeping stale exec order `0`) — a real side-effect-order change on the default path; this branch restores the pre-#10104 order. The committed snapshot is therefore the pre-regression output, not an expectation written to match the fix. Reverting only the two source files onto this branch reproduces the failure, so the fixture discriminates. - **No existing snapshot changes.** No other fixture failed under `INSTA_UPDATE=no`, which is what a changed snapshot would do, so no existing snapshot changed. ## Validation - `cargo test -p rolldown` — 1877 passed here against 1876 on `main`, the difference being exactly the new fixture. Both runs share the same 5 environmental failures (`cjs_module_lexer_compat` ×3 + `npm_packages/util_deprecate` need `pnpm install`; `test262_module_code` needs the submodule). Run with `INSTA_UPDATE=no`: `.insta.yaml` sets `update: 'always'`, so a local run without it rewrites snapshots in place and reports success regardless — worth knowing for anyone reproducing the revert-check above. - `cargo fmt --all --check` — clean. - `cargo clippy -p rolldown --all-targets -- -D warnings` — clean. --- Follow-up to #10104; addresses item 3 of #10104 (comment)
graphite-app
Bot
force-pushed
the
codex/flag-off-runtime-sweep-chunk-order
branch
from
July 19, 2026 15:25
0f69617 to
ab2a338
Compare
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.
Summary
Flag-off builds could emit cross-chunk imports — and therefore run side effects — in a different order than
main, on the default path. When the post-chunking runtime sweep drops the runtime module out of a shared chunk it co-hosts with user modules, that chunk was left with an execution order keyed on the removed runtime (exec order0) instead of its first surviving module, so it sorted ahead of chunks it should have followed. This restoresmain's ordering by re-deriving chunk execution orders after the sweep.Verified mechanism
1d86ffe06),sweep_unused_runtime_moduleran ingenerate_chunksbefore chunk exec-order assignment (there was a comment requiring it). feat(code-splitting): wrap strict execution order modules on demand #10104 moved the sweep intofinalize_chunk_plan, which runs after assignment, so the regression is live onmaintoday.modules[0]'s exec order;sort_chunk_modulesalways makes the runtime (module exec order0)modules[0]. So a chunk hosting the runtime keys on0.find_entry_level_external_module) invalidates the runtime demand (the [Bug]: rolldown/runtime is false-positively injected #9374 star-re-export-to-external shape) and the sweep strips the runtime from a still-live shared chunk, the chunk keepsexec_orderderived from the now-absent runtime.rebuild_sorted_chunk_idx_vec(true)only re-sorts the stale values;renumber_live_chunksis strict-only. That stale key feeds bothsorted_chunk_idx_vecand the cross-chunk import sort (compute_cross_chunk_links.rs:110/124), so an entry emitsimport "./C.js"before a chunk that should run first.Fix
Chunk exec orders must already be assigned before order analysis runs (the on-demand actual-order simulation sorts sibling chunks by
exec_order), so assignment cannot simply move after the sweep. Instead:assign_chunk_exec_orders(code_splitting.rs); keep the provisional call where it was ingenerate_chunks.finalize_chunk_planwhen the sweep actually removed the runtime from a chunk — the same condition that already triggeredrebuild_sorted_chunk_idx_vec(true)— then rebuild the sorted vec.Final ordering contract (documented on
assign_chunk_exec_orders). Flag-off,analyze_execution_orderreturnsNone, so no lowering and norenumber_live_chunksrun; the re-run is the sole authority and reproduces exactly whatmainproduced by sweeping before assignment. Under strict, the sweep block only runs whenrequired_runtime_helpers()is empty (no order wrapper demands the runtime);renumber_live_chunks(strict-only, after lowering) merely compacts existingexec_ordervalues to close tombstone gaps and never re-derives them frommodules[0]. The re-run happens after lowering, so whichever of the two touchesexec_orderlast leaves every live chunk both densely numbered and keyed on its correct lead module — they never contend.Behavior / snapshot impact
topics/runtime/sweep_shared_chunk_exec_order— three entries;re.js(export * from 'external') makes tree-shaking include the runtime for__reExport, and the runtime folds intore.js's shared{entry_a, entry_b}chunk. The walk-back flattens the star re-export to a chunk-levelexport * from "external", so the post-order sweep drops the runtime from that still-live shared chunk.entry_aalso imports the earlier side-effect chunks_ac.js(shared{entry_a, entry_c}).main(ce98ae798) emitsimport "./s_ac.js"beforeimport "./re.js"; currentmainemitsimport "./re.js"first (the runtime-hosting chunk keeping stale exec order0) — a real side-effect-order change on the default path; this branch restores the pre-feat(code-splitting): wrap strict execution order modules on demand #10104 order. The committed snapshot is therefore the pre-regression output, not an expectation written to match the fix. Reverting only the two source files onto this branch reproduces the failure, so the fixture discriminates.INSTA_UPDATE=no, which is what a changed snapshot would do, so no existing snapshot changed.Validation
cargo test -p rolldown— 1877 passed here against 1876 onmain, the difference being exactly the new fixture. Both runs share the same 5 environmental failures (cjs_module_lexer_compat×3 +npm_packages/util_deprecateneedpnpm install;test262_module_codeneeds the submodule). Run withINSTA_UPDATE=no:.insta.yamlsetsupdate: 'always', so a local run without it rewrites snapshots in place and reports success regardless — worth knowing for anyone reproducing the revert-check above.cargo fmt --all --check— clean.cargo clippy -p rolldown --all-targets -- -D warnings— clean.Follow-up to #10104; addresses item 3 of #10104 (comment)