fix(code-splitting): make strict transitive init registration deterministic and constant-time#10320
Merged
Conversation
Merging this PR will not alter performance
Comparing Footnotes
|
graphite-app
Bot
force-pushed
the
codex/vite-chunk-execution-order-plan
branch
from
July 17, 2026 08:56
2ee23cc to
1d86ffe
Compare
hyfdev
force-pushed
the
codex/strict-transitive-init-determinism
branch
from
July 20, 2026 14:07
56f76bc to
d427f05
Compare
✅ Deploy Preview for rolldown-rs canceled.
|
hyfdev
force-pushed
the
codex/strict-transitive-init-determinism
branch
from
July 22, 2026 03:38
d427f05 to
4039bf3
Compare
IWANABETHATGUY
approved these changes
Jul 22, 2026
Member
Author
Merge activity
|
…nistic and constant-time (#10320) ## Summary Two independent, cheap fixes to the strict-execution-order cross-chunk link pass (review items 3 and 4): - **Cross-target determinism.** `add_transitive_esm_init_depended_symbols` iterated an `FxHashMap` via `.values()`, whose bucket order follows FxHash layout. FxHash is unseeded but hashes differently on 32-bit vs 64-bit, so that order — which flows into the chunk's `depended_symbols` `FxIndexSet` and thence into imported-symbol rename order (the `$1`/`$2` suffixes) in `deconflict_chunk_symbols` — could diverge between native and wasm32/WASI builds. Now iterated sorted by the owning `StmtInfoIdx`, pinning one order everywhere. It is the single hash-ordered site three review passes converged on; every other hash-ordered effect is already sorted before use. - **Quadratic strict-mode hot loop.** `module_is_in_live_chunk` did a linear `chunk.modules.contains` scan, called for every canonical referenced symbol of every included statement under strict — quadratic on large chunks. Replaced with an O(1) `module_to_chunk` + liveness check. The O(1) rewrite is verified semantically equivalent: `add_module_to_chunk` sets `module_to_chunk[m]` and pushes `m` onto `c.modules` together, and every site that later removes a module from a chunk's `modules` list preserves the invariant (`module_to_chunk[m] == Some(c) && c live ⟹ c.modules ∋ m`) by pairing the removal with clearing `module_to_chunk[m]` (unused-runtime sweep), reassigning it to the destination chunk via `add_module_to_chunk` (runtime relocation, runtime→target merge, `apply_common_chunk_merges`), or marking the emptied chunk `Removed`. No site leaves `module_to_chunk` pointed at a still-live chunk that no longer contains the module, so no removed-module set is needed. The invariant is documented in the function's doc comment. ## Behavior / snapshot impact **None.** Zero fixture snapshot changes. The determinism fix is invisible on native 64-bit (its value is native-vs-wasm parity, which the native suite cannot exercise); the O(1) change is a pure performance rewrite with identical results. ## Validation - `cargo test -p rolldown` — 1868 passed; snapshot-stable (zero deltas). The only failures are 5 pre-existing environmental ones unrelated to this change (`cjs_module_lexer_compat` ×3 and `npm_packages/util_deprecate` need an installed `cjs-module-lexer`; `test262` needs the test262 submodule). - `cargo fmt --all --check` — clean. - `cargo clippy -p rolldown --all-targets -- -D warnings` — clean. --- Follow-up to #10104; addresses the two "cheap pre-merge fixes" (items 3 and 4) from the review discussion: #10104 (comment)
graphite-app
Bot
force-pushed
the
codex/strict-transitive-init-determinism
branch
from
July 22, 2026 06:58
4039bf3 to
1ed58f4
Compare
19 tasks
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
Two independent, cheap fixes to the strict-execution-order cross-chunk link pass (review items 3 and 4):
add_transitive_esm_init_depended_symbolsiterated anFxHashMapvia.values(), whose bucket order follows FxHash layout. FxHash is unseeded but hashes differently on 32-bit vs 64-bit, so that order — which flows into the chunk'sdepended_symbolsFxIndexSetand thence into imported-symbol rename order (the$1/$2suffixes) indeconflict_chunk_symbols— could diverge between native and wasm32/WASI builds. Now iterated sorted by the owningStmtInfoIdx, pinning one order everywhere. It is the single hash-ordered site three review passes converged on; every other hash-ordered effect is already sorted before use.module_is_in_live_chunkdid a linearchunk.modules.containsscan, called for every canonical referenced symbol of every included statement under strict — quadratic on large chunks. Replaced with an O(1)module_to_chunk+ liveness check.The O(1) rewrite is verified semantically equivalent:
add_module_to_chunksetsmodule_to_chunk[m]and pushesmontoc.modulestogether, and every site that later removes a module from a chunk'smoduleslist preserves the invariant (module_to_chunk[m] == Some(c) && c live ⟹ c.modules ∋ m) by pairing the removal with clearingmodule_to_chunk[m](unused-runtime sweep), reassigning it to the destination chunk viaadd_module_to_chunk(runtime relocation, runtime→target merge,apply_common_chunk_merges), or marking the emptied chunkRemoved. No site leavesmodule_to_chunkpointed at a still-live chunk that no longer contains the module, so no removed-module set is needed. The invariant is documented in the function's doc comment.Behavior / snapshot impact
None. Zero fixture snapshot changes. The determinism fix is invisible on native 64-bit (its value is native-vs-wasm parity, which the native suite cannot exercise); the O(1) change is a pure performance rewrite with identical results.
Validation
cargo test -p rolldown— 1868 passed; snapshot-stable (zero deltas). The only failures are 5 pre-existing environmental ones unrelated to this change (cjs_module_lexer_compat×3 andnpm_packages/util_deprecateneed an installedcjs-module-lexer;test262needs the test262 submodule).cargo fmt --all --check— clean.cargo clippy -p rolldown --all-targets -- -D warnings— clean.Follow-up to #10104; addresses the two "cheap pre-merge fixes" (items 3 and 4) from the review discussion: #10104 (comment)