Skip to content

fix(code-splitting): make strict transitive init registration deterministic and constant-time#10320

Merged
graphite-app[bot] merged 1 commit into
mainfrom
codex/strict-transitive-init-determinism
Jul 22, 2026
Merged

fix(code-splitting): make strict transitive init registration deterministic and constant-time#10320
graphite-app[bot] merged 1 commit into
mainfrom
codex/strict-transitive-init-determinism

Conversation

@hyfdev

@hyfdev hyfdev commented Jul 16, 2026

Copy link
Copy Markdown
Member

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)

@hyfdev
hyfdev marked this pull request as ready for review July 17, 2026 08:35
Copilot AI review requested due to automatic review settings July 17, 2026 08:35

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@codspeed-hq

codspeed-hq Bot commented Jul 17, 2026

Copy link
Copy Markdown

Merging this PR will not alter performance

✅ 7 untouched benchmarks
⏩ 10 skipped benchmarks1


Comparing codex/strict-transitive-init-determinism (4039bf3) with main (aa73a68)

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.

@graphite-app
graphite-app Bot force-pushed the codex/vite-chunk-execution-order-plan branch from 2ee23cc to 1d86ffe Compare July 17, 2026 08:56
Base automatically changed from codex/vite-chunk-execution-order-plan to main July 17, 2026 09:00
@hyfdev
hyfdev force-pushed the codex/strict-transitive-init-determinism branch from 56f76bc to d427f05 Compare July 20, 2026 14:07
@netlify

netlify Bot commented Jul 20, 2026

Copy link
Copy Markdown

Deploy Preview for rolldown-rs canceled.

Name Link
🔨 Latest commit 1ed58f4
🔍 Latest deploy log https://app.netlify.com/projects/rolldown-rs/deploys/6a606a2a5c1b35000713b4d2

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@hyfdev
hyfdev force-pushed the codex/strict-transitive-init-determinism branch from d427f05 to 4039bf3 Compare July 22, 2026 03:38

hyfdev commented Jul 22, 2026

Copy link
Copy Markdown
Member Author

Merge activity

  • Jul 22, 6:58 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 22, 6:58 AM UTC: hyfdev added this pull request to the Graphite merge queue.
  • Jul 22, 7:04 AM UTC: Merged by the Graphite merge queue.

…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
graphite-app Bot force-pushed the codex/strict-transitive-init-determinism branch from 4039bf3 to 1ed58f4 Compare July 22, 2026 06:58
@graphite-app
graphite-app Bot merged commit 1ed58f4 into main Jul 22, 2026
33 of 34 checks passed
@graphite-app
graphite-app Bot deleted the codex/strict-transitive-init-determinism branch July 22, 2026 07:04
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