Skip to content

refactor(rust): filter external modules from entries instead of mapping bit positions#8637

Merged
IWANABETHATGUY merged 4 commits intomainfrom
fix/filter-external-from-entries
Mar 13, 2026
Merged

refactor(rust): filter external modules from entries instead of mapping bit positions#8637
IWANABETHATGUY merged 4 commits intomainfrom
fix/filter-external-from-entries

Conversation

@Dunqing
Copy link
Copy Markdown
Collaborator

@Dunqing Dunqing commented Mar 11, 2026

Summary

Follow-up to #8596. Instead of using bit_to_chunk_idx to map bit positions to chunk indices (a band-aid that every consumer must remember), this filters external modules at the source so they never get entry bit positions.

  • module_loader.rs: Skip external modules when collecting dynamic_import_entry_ids
  • chunk_graph.rs: Remove bit_to_chunk_idx field
  • code_splitting.rs: Remove bit_to_chunk_idx pre-allocation/recording, add debug_assert for entry module normality
  • chunk_optimizer.rs: Use direct ChunkIdx::from_raw(bit) instead of bit_to_chunk_idx lookup
  • code-splitting.md: Update design doc to reflect new approach

This matches esbuild's approach where external modules never enter the entry list, ensuring bit positions directly equal chunk indices.

Test plan

  • cargo test -p rolldown --test integration -- "8595" passes
  • cargo test -p rolldown --test integration -- "circular_chunk_from_external" passes
  • Full CI

🤖 Generated with Claude Code

Copilot AI review requested due to automatic review settings March 11, 2026 08:18
@netlify
Copy link
Copy Markdown

netlify bot commented Mar 11, 2026

Deploy Preview for rolldown-rs canceled.

Name Link
🔨 Latest commit 786f663
🔍 Latest deploy log https://app.netlify.com/projects/rolldown-rs/deploys/69b35edd326d7d00077ad47d

@Dunqing Dunqing requested a review from IWANABETHATGUY March 11, 2026 08:19
@Dunqing Dunqing added the test: vite-tests Trigger Vite tests workflow on this PR label Mar 11, 2026
@Dunqing Dunqing changed the title refactor: filter external modules from entries instead of mapping bit positions refactor(rust): filter external modules from entries instead of mapping bit positions Mar 11, 2026
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Refactors code-splitting entry handling so external dynamic imports never become entry points, restoring the invariant that entry-bit positions map directly to ChunkIdx and allowing removal of the bit_to_chunk_idx workaround introduced in the prior fix.

Changes:

  • Filter external modules out of dynamic_import_entry_ids during module loading so they never enter link_output.entries.
  • Remove ChunkGraph::bit_to_chunk_idx and all associated allocation/recording logic.
  • Simplify chunk optimizer bit→chunk translation to ChunkIdx::from_raw(bit) and update the design doc accordingly.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.

Show a summary per file
File Description
meta/design/code-splitting.md Updates design doc to reflect “filter externals at source” and removes the old bit_to_chunk_idx invariant.
crates/rolldown/src/stages/generate_stage/code_splitting.rs Removes bit_to_chunk_idx setup and adds a debug assertion that entries are normal modules.
crates/rolldown/src/stages/generate_stage/chunk_optimizer.rs Switches from bit_to_chunk_idx lookup to direct ChunkIdx::from_raw(bit) mapping.
crates/rolldown/src/module_loader/module_loader.rs Prevents external dynamic imports from being collected as dynamic-import entry points.
crates/rolldown/src/chunk_graph.rs Deletes bit_to_chunk_idx from ChunkGraph and its constructor initialization.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Mar 11, 2026

Benchmarks Rust

group                                                        pr                                     target
-----                                                        --                                     ------
bundle/bundle@multi-duplicated-top-level-symbol              1.03     66.5±2.26ms        ? ?/sec    1.00     64.8±1.46ms        ? ?/sec
bundle/bundle@multi-duplicated-top-level-symbol-sourcemap    1.05     76.4±3.14ms        ? ?/sec    1.00     73.0±1.50ms        ? ?/sec
bundle/bundle@rome_ts                                        1.04    149.4±7.92ms        ? ?/sec    1.00    143.5±2.92ms        ? ?/sec
bundle/bundle@rome_ts-sourcemap                              1.05    171.4±7.12ms        ? ?/sec    1.00    163.3±3.09ms        ? ?/sec
bundle/bundle@threejs                                        1.01     62.5±3.48ms        ? ?/sec    1.00     62.1±2.84ms        ? ?/sec
bundle/bundle@threejs-sourcemap                              1.01     72.2±1.96ms        ? ?/sec    1.00     71.8±1.17ms        ? ?/sec
bundle/bundle@threejs10x                                     1.01    728.5±8.95ms        ? ?/sec    1.00    719.3±5.79ms        ? ?/sec
bundle/bundle@threejs10x-sourcemap                           1.00    817.2±4.53ms        ? ?/sec    1.00    820.6±5.22ms        ? ?/sec
scan/scan@rome_ts                                            1.03     70.7±1.67ms        ? ?/sec    1.00     69.0±1.50ms        ? ?/sec
scan/scan@threejs                                            1.02     25.1±1.42ms        ? ?/sec    1.00     24.7±1.36ms        ? ?/sec
scan/scan@threejs10x                                         1.01    245.9±3.63ms        ? ?/sec    1.00    244.3±3.56ms        ? ?/sec

@IWANABETHATGUY IWANABETHATGUY enabled auto-merge (squash) March 12, 2026 02:33
Dunqing and others added 3 commits March 13, 2026 08:16
… positions

Instead of mapping bit positions to chunk indices via `bit_to_chunk_idx`,
filter external modules at the source in module_loader.rs so they never
get entry bit positions. This ensures bit positions directly equal chunk
indices, eliminating the mapping layer entirely.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings March 13, 2026 00:17
@Dunqing Dunqing force-pushed the fix/filter-external-from-entries branch from 7dc486b to 552b889 Compare March 13, 2026 00:17
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.

Comments suppressed due to low confidence (1)

crates/rolldown/src/stages/generate_stage/code_splitting.rs:712

  • init_entry_point() enumerates all items in link_output.entries and only later continues for non-Module::Normal. If any external entry exists in entries, it will still consume an entry_index bit position, reintroducing gaps (bit positions != created ChunkIdx), but chunk_optimizer now assumes ChunkIdx::from_raw(bit) is always valid. To make the invariant robust, filter out non-normal entries before .enumerate() (and ensure entries_len matches the filtered count), or otherwise guarantee link_output.entries can never contain externals (including user-defined / emitted entries).
    // Create chunk for each static and dynamic entry
    for (entry_index, (&module_idx, entry_point)) in self
      .link_output
      .entries
      .iter()
      .flat_map(|(idx, entries)| entries.iter().map(move |e| (idx, e)))
      .enumerate()
    {
      let Module::Normal(module) = &self.link_output.module_table[module_idx] else {
        continue;
      };

User-defined and emitted entries are already guaranteed non-external by
`load_entry_module()` which rejects them with `entry_cannot_be_external`.
Added inline comment and updated design doc to explain this invariant.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@IWANABETHATGUY IWANABETHATGUY disabled auto-merge March 13, 2026 02:59
@IWANABETHATGUY IWANABETHATGUY merged commit 38b7369 into main Mar 13, 2026
26 of 30 checks passed
@IWANABETHATGUY IWANABETHATGUY deleted the fix/filter-external-from-entries branch March 13, 2026 03:00
This was referenced Mar 18, 2026
shulaoda added a commit that referenced this pull request Mar 18, 2026
## [1.0.0-rc.10] - 2026-03-18

### 🚀 Features

- add indentExclusionRanges property to MagicString (#8746) by @IWANABETHATGUY
- expose `oxcRuntimePlugin` (#8654) by @sapphi-red
- rust: make bundler generic over FileSystem for in-memory benchmarks (#8652) by @Boshen

### 🐛 Bug Fixes

- rolldown_plugin_vite_dynamic_import_vars: align dynamic import fast check with Vite (#8760) by @shulaoda
- renamer: handle existing bindings in nested scopes when finding unique names (#8741) by @drewolson
- pass `yarn_pnp` option where needed (#8736) by @sapphi-red
- preserve optional chaining in namespace member expr rewrite (#8712) by @Copilot
- correct UTF-16 index handling in native MagicString (#8693) by @IWANABETHATGUY
- mark failing doctests as ignore (#8700) by @Boshen
- prevent may_partial_namespace from leaking through include_module (#8682) by @IWANABETHATGUY
- ci: bump native-build cache key to invalidate stale napi-rs artifacts (#8678) by @Boshen
- `comments.annotation: false` breaking tree-shaking (#8657) by @IWANABETHATGUY
- validate filenames for NUL bytes from chunkFileNames/entryFileNames (#8644) by @IWANABETHATGUY
- dce-only minify should not set NODE_ENV to production (#8651) by @IWANABETHATGUY

### 🚜 Refactor

- rust: remove dead `CrossModuleOptimizationConfig::side_effects_free_function_optimization` (#8673) by @Dunqing
- rust: simplify `cross_module_optimization` by removing redundant scope tracking (#8672) by @Dunqing
- simplify string repeat in guess_indentor (#8753) by @IWANABETHATGUY
- consolidate custom magic-string tests into one file (#8696) by @IWANABETHATGUY
- extract CJS bailout checks from include_symbol (#8683) by @IWANABETHATGUY
- rust: remove `BindingIdentifierExt` to use `BindingIdentifier::symbol_id()` instead (#8667) by @Dunqing
- bench: add bench_preset helper and inline presets (#8658) by @Boshen
- rust: filter external modules from entries instead of mapping bit positions (#8637) by @Dunqing

### 📚 Documentation

- clarify watch mode behavior and its limitations (#8751) by @sapphi-red
- add external link icon to GitHub button in Hero section (#8731) by @thisisnkc
- guide: clarify that `inject` option is only conceptually similar to esbuild's one (#8743) by @sapphi-red
- meta/design: add `devtools.md` (#8663) by @hyf0
- add viteplus alpha announcement banner (#8668) by @shulaoda

### ⚡ Performance

- rolldown: some minor perf optimization found by autoresearch (#8730) by @Brooooooklyn
- replace Vec allocation with lazy iterator in find_hash_placeholders (#8703) by @Boshen
- replace TypedDashMap with TypedMap in CustomField (#8708) by @Boshen
- bench: remove scan benchmark binary to halve LTO link time (#8694) by @Boshen

### 🧪 Testing

- watch: increase timeout for error output (#8766) by @sapphi-red
- vite-tests: remove JS plugin tests (#8767) by @sapphi-red
- watch: add CLI exit code test (#8752) by @sapphi-red
- normalize paths on Windows even if `resolve.symlinks` is false (#8483) by @sapphi-red

### ⚙️ Miscellaneous Tasks

- correct comment in bundle-analyzer-plugin.ts (#8770) by @origami-z
- upgrade oxc to 0.120.0 (#8764) by @Boshen
- enable all test for `reset` category in MagicString.test.ts (#8749) by @IWANABETHATGUY
- deps: update test262 submodule for tests (#8742) by @sapphi-red
- deps: update oxc apps (#8734) by @renovate[bot]
- deps: update softprops/action-gh-release action to v2.6.1 (#8724) by @renovate[bot]
- deps: update npm packages (major) (#8722) by @renovate[bot]
- deps: update github-actions (major) (#8721) by @renovate[bot]
- deps: update softprops/action-gh-release action to v2.6.0 (#8720) by @renovate[bot]
- deps: update npm packages (#8718) by @renovate[bot]
- deps: update rust crates (#8717) by @renovate[bot]
- deps: update github-actions (#8716) by @renovate[bot]
- deps: update dependency oxlint-tsgolint to v0.17.0 (#8713) by @renovate[bot]
- deps: bump cargo-shear to v1.11.2 (#8711) by @Boshen
- use org level `CODE_OF_CONDUCT.md` (#8706) by @sapphi-red
- fix cache key mismatch and remove redundant cache saves (#8695) by @Boshen
- deps: update oxc apps (#8692) by @renovate[bot]
- deps: update oxc apps (#8649) by @renovate[bot]
- should do matrix out side of reusable workflows 2 (#8691) by @hyf0
- should do matrix out side of reusable workflows (#8690) by @hyf0
- deps: update dependency rolldown-plugin-dts to v0.22.5 (#8689) by @renovate[bot]
- upgrade oxc to 0.119.0 and oxc_resolver to 11.19.1 (#8686) by @Boshen
- correct if condition of `type-check` job (#8677) by @hyf0
- Gate CI type-check job on node changes (#8669) by @Copilot
- benchmark: improve codspeed build (#8665) by @Boshen
- deps: update oxc to v0.118.0 (#8650) by @renovate[bot]
- deps: update crate-ci/typos action to v1.44.0 (#8647) by @renovate[bot]
- deps: update oxc resolver to v11.19.1 (#8646) by @renovate[bot]
- deps: update dependency rust to v1.94.0 (#8648) by @renovate[bot]
- deps: update dependency rolldown-plugin-dts to v0.22.4 (#8645) by @renovate[bot]

### ◀️ Revert

- Revert "ci: Gate CI type-check job on node changes" (#8674) by @hyf0
- "chore(deps): update dependency rust to v1.94.0 (#8648)" (#8660) by @shulaoda

### ❤️ New Contributors

* @origami-z made their first contribution in [#8770](#8770)
* @drewolson made their first contribution in [#8741](#8741)
* @thisisnkc made their first contribution in [#8731](#8731)

Co-authored-by: shulaoda <165626830+shulaoda@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

test: vite-tests Trigger Vite tests workflow on this PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants