Skip to content

bundler: size each chunk's renamer name table from the chunk, not the whole bundle - #41861

Merged
Jarred-Sumner merged 1 commit into
mainfrom
claude/renamer-chunk-sized-name-table
Sep 7, 2026
Merged

Jarred-Sumner merged 1 commit into
mainfrom
claude/renamer-chunk-sized-name-table

Conversation

@Jarred-Sumner

Copy link
Copy Markdown
Collaborator

What does this PR do?

NumberRenamer::init (#41286) pre-sizes its root name table — the ids hash map and the slots vec — to root_names.len() + symbol_count / 4, where symbol_count is summed over symbols.symbols_for_source, i.e. every symbol in the bundle. That is fine for a single chunk, but with --splitting generateChunksInParallel builds a renamer for every chunk up front and each one stays alive in chunk.renamer until its chunk has been printed. So a build with N JS chunks reserves N bundle-sized tables at once.

On a large app that splits into ~800 chunks (~1M symbols), each chunk reserved a ~4.4 MB hashbrown table plus a ~3 MB Vec<NameSlot> and used a few hundred entries of it:

peak RSS (Linux, 4 cores)
before #41286 (858f4e7) 6.2 GB
main 7.7 GB
this PR 6.2 GB

Linux only pays for the pages that get touched (the control-byte memset plus scattered inserts, ~2 MB per chunk). On Windows mimalloc commits the whole block when it is allocated, so the same build carried several GB of extra commit charge and, on a machine with a small commit limit, bun build aborted with memory allocation of 4456464 bytes failed — 4,456,464 bytes being exactly this table (262,144 × 16-byte (StoreStr, u32) buckets + control bytes).

The fix sizes the table from the symbols of the files in the chunk (files_in_order), which is what the chunk's renamer can actually add. A non-splitting build has one chunk containing every file, so it reserves exactly what it did before and the #41286 benchmarks are unaffected.

How did you verify your code works?

Measured the build above with release builds of 858f4e7, main and this branch, pinned to 4 cores, two runs each (numbers in the table; run-to-run spread ±0.1 GB). Page-fault profiles of main attribute the extra faults to renamer::intern_into and hashbrown's control-byte memset under NumberRenamer::init; both disappear with this change. Output is byte-identical (capacity is only a hint; intern grows the table on demand).

… whole bundle

NumberRenamer::init pre-sized its root name table (ids + slots) to a
quarter of every symbol in the bundle. With --splitting every chunk gets
its own renamer and they are all alive until their chunk is printed, so a
build with N chunks reserved N bundle-sized tables: on a large app with
~800 chunks that was +1.5 GB of peak RSS on Linux and several GB of commit
charge on Windows (where the whole reservation is committed up front),
enough to fail the allocation outright on a small machine.

Size it from the symbols of the files in the chunk instead. A single-chunk
build reserves exactly what it did before.
@coderabbitai

coderabbitai Bot commented Sep 7, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Essentials

Run ID: 3bd8f058-c119-4262-ba27-719a84280abb

📥 Commits

Reviewing files that changed from the base of the PR and between 62838e1 and f177054.

📒 Files selected for processing (2)
  • src/bundler/linker_context/renameSymbolsInChunk.rs
  • src/js_printer/renamer.rs

Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review.


Walkthrough

The linker now computes the total symbols in each chunk and passes that value to NumberRenamer::init. The renamer sizes its name ID map and slot vector from the chunk-local count.

Changes

Chunk symbol renaming

Layer / File(s) Summary
Chunk count and renamer capacity
src/bundler/linker_context/renameSymbolsInChunk.rs, src/js_printer/renamer.rs
The linker sums symbols across chunk files and passes the total to NumberRenamer::init. The renamer uses the chunk count to size both name IDs and slots.

Suggested reviewers: robobun

Merge Risk: ⚪ Minimal · up to f1770

Chunk renamer capacity is now sized from symbols in each chunk, reducing memory use for splitting builds while retaining bundle-wide sizing for a single chunk. No current merge-blocking risk remains.

🚥 Pre-merge checks | ✅ 4
✅ Passed checks (4 passed)
Check name Status Explanation
Description check ✅ Passed The description includes both required sections. It clearly explains the chunk-local sizing change, its memory impact, non-splitting behavior, and verification results.
Title check ✅ Passed The title clearly and concisely describes the main change: each chunk's renamer name table is sized from chunk symbols instead of the entire bundle.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

Comment @coderabbitai help to get the list of available commands.

@claude claude Bot 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.

LGTM — capacity-hint-only change, output is unaffected.

What was reviewed:

  • Confirmed NumberRenamer::init has a single call site (this one), so no sibling callers to update.
  • Verified files_in_order holds source indices and symbols_for_source is indexed by source index — same pattern as the surrounding all_module_scopes[source_index] / ast_flags_col[source_index] loops in this function.
  • Both NameIds::with_capacity_and_hasher and Vec::with_capacity grow on demand, so under-sizing cannot change renamer output; a single-chunk (non-splitting) build sums the same files as before.
Extended reasoning...

Overview

This PR touches two files: src/js_printer/renamer.rs adds a symbols_in_chunk: usize parameter to NumberRenamer::init and uses it (instead of summing the whole bundle's symbols_for_source) to size the ids hashmap and slots vec; src/bundler/linker_context/renameSymbolsInChunk.rs computes that count by summing symbols_view.symbols_for_source[i].len() over the chunk's files_in_order before calling init. The capacity formula root_names.len() + N / 4 is unchanged and now deduplicated into one local. A short doc comment on init records why the caller must supply the count.

Security risks

None. This changes only the initial capacity reservation of a hashmap and a vec inside the bundler's symbol renamer. No untrusted-input parsing, no I/O, no auth/crypto, no user-facing API surface.

Level of scrutiny

Low-to-moderate. The change is ~15 lines and purely affects allocation sizing hints — HashMap::with_capacity_and_hasher and Vec::with_capacity both grow on demand, so even a wrong estimate cannot alter renamer output (the PR states output is byte-identical, which follows structurally). I checked that NumberRenamer::init has exactly one caller in the tree, so there are no sibling sites left with the old signature. The files_in_order → symbols_for_source[i as usize] indexing matches the established pattern used four other times in the same function for other per-source-index tables, and symbols_for_source is indexed by source_index elsewhere in renamer.rs. For a non-splitting build the single chunk contains every file, so the reserved capacity equals what it was before and the #41286 benchmark path is unchanged.

Other factors

The PR description supplies before/after peak-RSS measurements (6.2 GB → 7.7 GB regression on main, back to 6.2 GB with this change) and a page-fault attribution, satisfying the Performance section's "before/after numbers" requirement. No behavioral test is added, but capacity hints are not observable in output, so REVIEW.md's "every behavioral change ships a test" does not apply here — existing bundler correctness tests cover the renamer's output. No CODEOWNERS entry covers src/bundler/ or src/js_printer/. No prior reviews or outstanding objections on the timeline. Exit reason was dry_streak with zero findings.

@Jarred-Sumner
Jarred-Sumner merged commit b52d3e5 into main Sep 7, 2026
10 of 12 checks passed
@Jarred-Sumner
Jarred-Sumner deleted the claude/renamer-chunk-sized-name-table branch September 7, 2026 23:58
Jarred-Sumner pushed a commit that referenced this pull request Sep 14, 2026
### Problem

- `NumberRenamer.names` is `names[source_index][inner_index]`. Each
chunk has a 24-byte slot per file of the bundle: 215 MB of the 405 MB
peak for 800 entry points x 13 modules. Every chunk's renamer is alive
until its chunk is printed.
- A row grows to the inner index of each symbol it names, and a chunk
also names the bindings it imports (`pin_top_level_symbol`,
`src/js_printer/renamer.rs:886`). 800 chunks import one function
declared after 200,000 constants: 2,121 MB peak RSS. A `require_*`
wrapper is the last symbol of its file.

### Fix

- A symbol of another chunk's file already has one name in the linker's
`cross_chunk_names`. `pin_top_level_symbol` only reserves it in the root
scope, and `name_for_symbol` reads it from the linker. No copy per
chunk.
- `names` has one row per file of the chunk. A `u32` table maps a source
index to its row. With `--splitting` each file is in one chunk, so the
linker builds one table for all chunks.
- 2,121 MB is now 276 MB, 405 MB is now 200 MB. `diff -r` of 4,933
output files: no difference. One intended change: a chunk no longer
reserves a name for a binding it imports and never uses.
- Verified: two new tests in `test/bundler/bundler_splitting.test.ts`.
More in Notes.

### Background

- The linker makes one renamer per chunk before it prints a chunk.
`NumberRenamer` is the one for builds without `--minify-identifiers`.
- A `Ref` is a source index plus an inner index: a file and a position
in its symbol table.
- `cross_chunk_names` (#40518) gives each binding that crosses chunks
one bundle-wide name.
- #41861 sized the `ids` and `slots` tables from the chunk, not `names`.

<details><summary>Notes</summary>

**History of this PR.** The first version kept the per-chunk names of
imported symbols in a `HashMap<Ref, NameStr>` and left the per-file
slots alone. Review asked for no per-chunk copy at all, for a dense
`names`, and for a structural test. This is that rework. Earlier
comments refer to the first version.

**Numbers.** Release builds of the same commit with and without the
change, Linux x64, peak RSS from `getrusage`. The project from the
report: `big.js` with 200,000 `export const` and one `export function
pick`, and N entry points that each import `pick`.

| entry chunks | `pick` last, before | `pick` last, after | `pick`
first, after |
| --- | --- | --- | --- |
| 100 | 463 MB | 232 MB | 215 MB |
| 200 | 700 MB | 241 MB | |
| 400 | 1,172 MB | 251 MB | 235 MB |
| 800 | 2,121 MB | 276 MB | 261 MB |
| 1,600 | 4,054 MB | 328 MB | 312 MB |

The CommonJS case: `big.cjs` with `module.exports = { pick, ... }` and
50,000 other symbols, 200 entry points that import `pick`,
`--splitting`. Before 160 MB, after 47 MB.

Other builds, median of 9 runs (15 for `typescript.js`), before and
after:

| build | peak RSS | wall (median) |
| --- | --- | --- |
| 800 entry points x 13 own modules, `--splitting` | 405 MB, 200 MB |
0.26 s, 0.21 s |
| the same, no `--splitting` | 404 MB, 195 MB | 0.26 s, 0.21 s |
| the same, `--splitting --minify-identifiers` (`MinifyRenamer`, not
changed) | 169 MB | |
| 3,000 modules, one chunk | 711 MB, 711 MB | 445 ms, 455 ms |
| 3,000 modules, 300 entry points, `--splitting` (562 files) | 840 MB,
700 MB | 487 ms, 449 ms |
| 3,000 modules, 300 entry points, no `--splitting` | 802 MB, 785 MB |
452 ms, 445 ms |
| `typescript.js`, one file | 127 MB, 129 MB | 324 ms, 329 ms |

The machine is shared, so medians of wall time move by a few percent
between runs. A second set of 21 runs of the 3,000-module single chunk
gave 446 ms and 445 ms.

**The lookup.** `row_of_source[source_index] - first_row` is the row if
it is less than `names.len()`. Rows are numbered in chunk order and then
in `files_in_chunk_order`, so the rows of a chunk are consecutive and
one subtraction tells if a file is in the chunk. `renamer_rows` returns
`None` when a file is in two chunks (several entry points without
`--splitting` that share a file, for example the runtime). Then each
renamer has a `hashbrown` map from source index to row that holds only
its own files. 800 entry points x 13 own modules plus one file that all
of them import, no `--splitting`: 401 MB before, 202 MB after. 300 entry
points that share 3,000 modules, no `--splitting`, median of 15 runs:
471 ms before, 472 ms after.

**The output change.** `add_top_level_symbol` returns early for a symbol
of another chunk's file. An imported binding that the chunk uses is
pinned before that, as before. A binding that a file of the chunk
imports and never uses (or uses only in code that tree shaking removes)
reserved its name on main. Now it does not:

```js
// shared.js (its own chunk)       // a-helper.js (chunk of a.js)      // a.js
export const x = 1;                 import { y } from "./shared.js";   import "./a-helper.js";
export const y = 2;                 console.log("helper");              import { x } from "./shared.js";
                                                                        const y = 5; console.log(x, y);
```

main prints `var y2 = 5` in the chunk of `a.js`. This branch prints `var
y = 5`. The unused `y` of `shared.js` is not printed in that chunk, so
nothing can collide with it.

**Links.** `name_for_symbol` looks an imported symbol up by the ref its
links lead to. `assign_unminified` keys `cross_chunk_names` by the
export's ref. The two differ for `export var x = 1; var x = 2;`, where
the parser links the first `x` to the second (#42409 describes it). So
`assign_unminified` also adds the followed ref.
`splitting/CrossChunkNameOfRedeclaredExport` covers it: without that
insert it fails with `"x" has already been declared`.

**Tests.** `splitting/ChunkRenamerTablesHoldOnlyTheChunksOwnFiles`
builds two entry points that import from an ES module and from a
CommonJS module, each with 1,000 other symbols. With
`BUN_DEBUG_ChunkRenamer=1` a debug build logs, per chunk, the file
count, the row count and the name slot count of its renamer. The test
asserts that each entry chunk has 1 row and fewer than 50 name slots (it
has 4). The log is a scoped debug log, so the test is skipped on release
builds, like `test/js/bun/resolve/tsconfig-extends-leak.test.ts`. The
RSS test of the first version is gone.

**Suites run with the debug build.** All of
`test/bundler/bundler_splitting.test.ts`,
`test/bundler/esbuild/{splitting,default,importstar,importstar_ts,dce,ts}.test.ts`,
`test/bundler/bundler_{edgecase,regressions,cjs2esm,naming,minify,compile_splitting,html}.test.ts`,
`test/bake/dev/{bundle,esm}.test.ts`.
`test/bundler/bun-build-api.test.ts` has three tests that time out on
this loaded debug build, so I ran it with the two release builds: 66
pass on both.

**Not changed.** The mimalloc heap that each renamer owns (`arena`).
`MinifyRenamer`.

</details>

<!-- robobun:evidence:begin -->

---

**[human-review]** gate passed · iteration 0 · 6 files touched

<details><summary>fails on main (without fix)</summary>

```console
ASAN without fix: 1 failed, 4 skipped
$ BUN_DEBUG_QUIET_LOGS=1 bun scripts/build.ts --profile=debug --quiet test "--reporter=junit" "--reporter-outfile=/tmp/pr_gate.xml" test/bundler/bundler_splitting.test.ts
bun test v1.4.3 (09bb546)

test/bundler/bundler_splitting.test.ts:
(pass) bundler > splitting/DynamicImportCSSFile [1256.76ms]
(pass) bundler > splitting/DynamicImportMultipleCSSImports [808.91ms]
(pass) bundler > splitting/StaticAndDynamicCSSImports [763.80ms]
(pass) bundler > splitting/NestedDynamicImportWithCSS [734.52ms]
(pass) bundler > splitting/SharedCSSBetweenChunks [699.30ms]
(pass) bundler > splitting/DynamicImportChainWithCSS [936.04ms]
(pass) bundler > splitting/ConditionalDynamicImportWithCSS [727.08ms]
(pass) bundler > splitting/MultipleEntryPointsWithSharedCSS [1087.03ms]
(pass) bundler > splitting/DynamicImportWithOnlyCSSNoJS [777.53ms]
(pass) bundler > splitting/CircularDynamicImportsWithCSS [711.07ms]
(pass) bundler > splitting/DeadDynamicImportTargetGetsNoChunk-cli [753.99ms]
(pass) bundler > splitting/DeadDynamicImportTargetGetsNoChunk-api [592.91ms]
(pass) bundler > splitting/DeadDynamicImportTargetNoSplittingReference [684.20ms]
(pass) bundler > splitting/DeadDynamicImportTa
... (truncated)

release without fix: 4 skipped
bun test v1.4.3-canary.1 (6bec1ef)

test/bundler/bundler_splitting.test.ts:
(pass) bundler > splitting/DynamicImportCSSFile [26.43ms]
(pass) bundler > splitting/DynamicImportMultipleCSSImports [14.04ms]
(pass) bundler > splitting/StaticAndDynamicCSSImports [14.61ms]
(pass) bundler > splitting/NestedDynamicImportWithCSS [13.20ms]
(pass) bundler > splitting/SharedCSSBetweenChunks [15.64ms]
(pass) bundler > splitting/DynamicImportChainWithCSS [14.41ms]
(pass) bundler > splitting/ConditionalDynamicImportWithCSS [14.44ms]
(pass) bundler > splitting/MultipleEntryPointsWithSharedCSS [19.35ms]
(pass) bundler > splitting/DynamicImportWithOnlyCSSNoJS [13.03ms]
(pass) bundler > splitting/CircularDynamicImportsWithCSS [15.17ms]
(pass) bundler > splitting/DeadDynamicImportTargetGetsNoChunk-cli [11.63ms]
(pass) bundler > splitting/DeadDynamicImportTargetGetsNoChunk-api [13.74ms]
(pass) bundler > splitting/DeadDynamicImportTargetNoSplittingReference [13.31ms]
(pass) bundler > splitting/DeadDynamicImportTargetKeptWithoutTreeShaking [12.21ms]
(pass) bundler > splitting/LiveDynamicImportTargetStillGetsChunk [16.91ms]
(pass) bundler > splitting/DynamicImportChainLiveness [17.31ms]
(
... (truncated)
```

</details>

<details><summary>passes on PR (with fix)</summary>

```console
ASAN with fix: 4 skipped
$ BUN_DEBUG_QUIET_LOGS=1 bun scripts/build.ts --profile=debug --quiet test "--reporter=junit" "--reporter-outfile=/tmp/pr_gate.xml" test/bundler/bundler_splitting.test.ts
bun test v1.4.3 (09bb546)

test/bundler/bundler_splitting.test.ts:
(pass) bundler > splitting/DynamicImportCSSFile [1234.67ms]
(pass) bundler > splitting/DynamicImportMultipleCSSImports [657.65ms]
(pass) bundler > splitting/StaticAndDynamicCSSImports [621.68ms]
(pass) bundler > splitting/NestedDynamicImportWithCSS [666.60ms]
(pass) bundler > splitting/SharedCSSBetweenChunks [821.23ms]
(pass) bundler > splitting/DynamicImportChainWithCSS [851.81ms]
(pass) bundler > splitting/ConditionalDynamicImportWithCSS [707.22ms]
(pass) bundler > splitting/MultipleEntryPointsWithSharedCSS [918.65ms]
(pass) bundler > splitting/DynamicImportWithOnlyCSSNoJS [607.51ms]
(pass) bundler > splitting/CircularDynamicImportsWithCSS [725.79ms]
(pass) bundler > splitting/DeadDynamicImportTargetGetsNoChunk-cli [711.70ms]
(pass) bundler > splitting/DeadDynamicImportTargetGetsNoChunk-api [505.03ms]
(pass) bundler > splitting/DeadDynamicImportTargetNoSplittingReference [608.86ms]
(pass) bundler > splitting/DeadDynamicImportTar
... (truncated)

release with fix: 4 skipped
$ bun scripts/build.ts --profile=release
[configured] bun-profile → bun (stripped) in 990ms (unchanged)
ninja: Entering directory `/workspace/bun/build/release'
[0/5] cargo bun_runtime → libbun_runtime.a
�[1m�[92m   Compiling�[0m bun_core v0.0.0 (/workspace/bun/src/bun_core)
�[1m�[92m   Compiling�[0m bun_errno v0.0.0 (/workspace/bun/src/errno)
�[1m�[92m   Compiling�[0m bun_ptr v0.0.0 (/workspace/bun/src/ptr)
�[1m�[92m   Compiling�[0m bun_boringssl_sys v0.0.0 (/workspace/bun/src/boringssl_sys)
�[1m�[92m   Compiling�[0m bun_safety v0.0.0 (/workspace/bun/src/safety)
�[1m�[92m   Compiling�[0m bun_base64 v0.0.0 (/workspace/bun/src/base64)
�[1m�[92m   Compiling�[0m bun_cares_sys v0.0.0 (/workspace/bun/src/cares_sys)
�[1m�[92m   Compiling�[0m bun_zlib_sys v0.0.0 (/workspace/bun/src/zlib_sys)
�[1m�[92m   Compiling�[0m bun_zstd v0.0.0 (/workspace/bun/src/zstd)
�[1m�[92m   Compiling�[0m bun_picohttp v0.0.0 (/workspace/bun/src/picohttp)
�[1m�[92m   Compiling�[0m bun_brotli v0.0.0 (/workspace/bun/src/brotli)
�[1m�[92m   Compiling�[0m bun_output v0.0.0 (/workspace/bun/src/output)
�[1m�[92m   Compiling�[0m bun_collections v0.0.0 (/workspace/bun/src/collections)
�[1m�[92m   Co
... (truncated)
```

</details>

<details><summary>diff hotspot</summary>

```
src/bundler/LinkerContext.rs                       |   5 +-
 src/bundler/linker_context/crossChunkNames.rs      |   5 +
 .../linker_context/generateChunksInParallel.rs     |   7 ++
 src/bundler/linker_context/renameSymbolsInChunk.rs |  57 +++++++++--
 src/js_printer/renamer.rs                          | 111 ++++++++++++++++-----
 test/bundler/bundler_splitting.test.ts             |  67 +++++++++++++
 6 files changed, 218 insertions(+), 34 deletions(-)
```

</details>

**gate history** · 2 passed · 0 rejected · iteration 0

<details><summary>evidence per changed file</summary>

```
file                                                    reads  edits  tests
src/bundler/LinkerContext.rs                                0      0     34
src/bundler/linker_context/crossChunkNames.rs               1      0     34
src/bundler/linker_context/generateChunksInParallel.rs      1      0     34
src/bundler/linker_context/renameSymbolsInChunk.rs          1      1     34
src/js_printer/renamer.rs                                   6      7     34
test/bundler/bundler_splitting.test.ts                      5      4     34
```

</details>

<!-- robobun:evidence:end -->
usrbinkat pushed a commit to usrbinkat/bun that referenced this pull request Sep 15, 2026
…#42489)

### Problem

- `NumberRenamer.names` is `names[source_index][inner_index]`. Each
chunk has a 24-byte slot per file of the bundle: 215 MB of the 405 MB
peak for 800 entry points x 13 modules. Every chunk's renamer is alive
until its chunk is printed.
- A row grows to the inner index of each symbol it names, and a chunk
also names the bindings it imports (`pin_top_level_symbol`,
`src/js_printer/renamer.rs:886`). 800 chunks import one function
declared after 200,000 constants: 2,121 MB peak RSS. A `require_*`
wrapper is the last symbol of its file.

### Fix

- A symbol of another chunk's file already has one name in the linker's
`cross_chunk_names`. `pin_top_level_symbol` only reserves it in the root
scope, and `name_for_symbol` reads it from the linker. No copy per
chunk.
- `names` has one row per file of the chunk. A `u32` table maps a source
index to its row. With `--splitting` each file is in one chunk, so the
linker builds one table for all chunks.
- 2,121 MB is now 276 MB, 405 MB is now 200 MB. `diff -r` of 4,933
output files: no difference. One intended change: a chunk no longer
reserves a name for a binding it imports and never uses.
- Verified: two new tests in `test/bundler/bundler_splitting.test.ts`.
More in Notes.

### Background

- The linker makes one renamer per chunk before it prints a chunk.
`NumberRenamer` is the one for builds without `--minify-identifiers`.
- A `Ref` is a source index plus an inner index: a file and a position
in its symbol table.
- `cross_chunk_names` (oven-sh#40518) gives each binding that crosses chunks
one bundle-wide name.
- oven-sh#41861 sized the `ids` and `slots` tables from the chunk, not `names`.

<details><summary>Notes</summary>

**History of this PR.** The first version kept the per-chunk names of
imported symbols in a `HashMap<Ref, NameStr>` and left the per-file
slots alone. Review asked for no per-chunk copy at all, for a dense
`names`, and for a structural test. This is that rework. Earlier
comments refer to the first version.

**Numbers.** Release builds of the same commit with and without the
change, Linux x64, peak RSS from `getrusage`. The project from the
report: `big.js` with 200,000 `export const` and one `export function
pick`, and N entry points that each import `pick`.

| entry chunks | `pick` last, before | `pick` last, after | `pick`
first, after |
| --- | --- | --- | --- |
| 100 | 463 MB | 232 MB | 215 MB |
| 200 | 700 MB | 241 MB | |
| 400 | 1,172 MB | 251 MB | 235 MB |
| 800 | 2,121 MB | 276 MB | 261 MB |
| 1,600 | 4,054 MB | 328 MB | 312 MB |

The CommonJS case: `big.cjs` with `module.exports = { pick, ... }` and
50,000 other symbols, 200 entry points that import `pick`,
`--splitting`. Before 160 MB, after 47 MB.

Other builds, median of 9 runs (15 for `typescript.js`), before and
after:

| build | peak RSS | wall (median) |
| --- | --- | --- |
| 800 entry points x 13 own modules, `--splitting` | 405 MB, 200 MB |
0.26 s, 0.21 s |
| the same, no `--splitting` | 404 MB, 195 MB | 0.26 s, 0.21 s |
| the same, `--splitting --minify-identifiers` (`MinifyRenamer`, not
changed) | 169 MB | |
| 3,000 modules, one chunk | 711 MB, 711 MB | 445 ms, 455 ms |
| 3,000 modules, 300 entry points, `--splitting` (562 files) | 840 MB,
700 MB | 487 ms, 449 ms |
| 3,000 modules, 300 entry points, no `--splitting` | 802 MB, 785 MB |
452 ms, 445 ms |
| `typescript.js`, one file | 127 MB, 129 MB | 324 ms, 329 ms |

The machine is shared, so medians of wall time move by a few percent
between runs. A second set of 21 runs of the 3,000-module single chunk
gave 446 ms and 445 ms.

**The lookup.** `row_of_source[source_index] - first_row` is the row if
it is less than `names.len()`. Rows are numbered in chunk order and then
in `files_in_chunk_order`, so the rows of a chunk are consecutive and
one subtraction tells if a file is in the chunk. `renamer_rows` returns
`None` when a file is in two chunks (several entry points without
`--splitting` that share a file, for example the runtime). Then each
renamer has a `hashbrown` map from source index to row that holds only
its own files. 800 entry points x 13 own modules plus one file that all
of them import, no `--splitting`: 401 MB before, 202 MB after. 300 entry
points that share 3,000 modules, no `--splitting`, median of 15 runs:
471 ms before, 472 ms after.

**The output change.** `add_top_level_symbol` returns early for a symbol
of another chunk's file. An imported binding that the chunk uses is
pinned before that, as before. A binding that a file of the chunk
imports and never uses (or uses only in code that tree shaking removes)
reserved its name on main. Now it does not:

```js
// shared.js (its own chunk)       // a-helper.js (chunk of a.js)      // a.js
export const x = 1;                 import { y } from "./shared.js";   import "./a-helper.js";
export const y = 2;                 console.log("helper");              import { x } from "./shared.js";
                                                                        const y = 5; console.log(x, y);
```

main prints `var y2 = 5` in the chunk of `a.js`. This branch prints `var
y = 5`. The unused `y` of `shared.js` is not printed in that chunk, so
nothing can collide with it.

**Links.** `name_for_symbol` looks an imported symbol up by the ref its
links lead to. `assign_unminified` keys `cross_chunk_names` by the
export's ref. The two differ for `export var x = 1; var x = 2;`, where
the parser links the first `x` to the second (oven-sh#42409 describes it). So
`assign_unminified` also adds the followed ref.
`splitting/CrossChunkNameOfRedeclaredExport` covers it: without that
insert it fails with `"x" has already been declared`.

**Tests.** `splitting/ChunkRenamerTablesHoldOnlyTheChunksOwnFiles`
builds two entry points that import from an ES module and from a
CommonJS module, each with 1,000 other symbols. With
`BUN_DEBUG_ChunkRenamer=1` a debug build logs, per chunk, the file
count, the row count and the name slot count of its renamer. The test
asserts that each entry chunk has 1 row and fewer than 50 name slots (it
has 4). The log is a scoped debug log, so the test is skipped on release
builds, like `test/js/bun/resolve/tsconfig-extends-leak.test.ts`. The
RSS test of the first version is gone.

**Suites run with the debug build.** All of
`test/bundler/bundler_splitting.test.ts`,
`test/bundler/esbuild/{splitting,default,importstar,importstar_ts,dce,ts}.test.ts`,
`test/bundler/bundler_{edgecase,regressions,cjs2esm,naming,minify,compile_splitting,html}.test.ts`,
`test/bake/dev/{bundle,esm}.test.ts`.
`test/bundler/bun-build-api.test.ts` has three tests that time out on
this loaded debug build, so I ran it with the two release builds: 66
pass on both.

**Not changed.** The mimalloc heap that each renamer owns (`arena`).
`MinifyRenamer`.

</details>

<!-- robobun:evidence:begin -->

---

**[human-review]** gate passed · iteration 0 · 6 files touched

<details><summary>fails on main (without fix)</summary>

```console
ASAN without fix: 1 failed, 4 skipped
$ BUN_DEBUG_QUIET_LOGS=1 bun scripts/build.ts --profile=debug --quiet test "--reporter=junit" "--reporter-outfile=/tmp/pr_gate.xml" test/bundler/bundler_splitting.test.ts
bun test v1.4.3 (09bb546)

test/bundler/bundler_splitting.test.ts:
(pass) bundler > splitting/DynamicImportCSSFile [1256.76ms]
(pass) bundler > splitting/DynamicImportMultipleCSSImports [808.91ms]
(pass) bundler > splitting/StaticAndDynamicCSSImports [763.80ms]
(pass) bundler > splitting/NestedDynamicImportWithCSS [734.52ms]
(pass) bundler > splitting/SharedCSSBetweenChunks [699.30ms]
(pass) bundler > splitting/DynamicImportChainWithCSS [936.04ms]
(pass) bundler > splitting/ConditionalDynamicImportWithCSS [727.08ms]
(pass) bundler > splitting/MultipleEntryPointsWithSharedCSS [1087.03ms]
(pass) bundler > splitting/DynamicImportWithOnlyCSSNoJS [777.53ms]
(pass) bundler > splitting/CircularDynamicImportsWithCSS [711.07ms]
(pass) bundler > splitting/DeadDynamicImportTargetGetsNoChunk-cli [753.99ms]
(pass) bundler > splitting/DeadDynamicImportTargetGetsNoChunk-api [592.91ms]
(pass) bundler > splitting/DeadDynamicImportTargetNoSplittingReference [684.20ms]
(pass) bundler > splitting/DeadDynamicImportTa
... (truncated)

release without fix: 4 skipped
bun test v1.4.3-canary.1 (6bec1ef)

test/bundler/bundler_splitting.test.ts:
(pass) bundler > splitting/DynamicImportCSSFile [26.43ms]
(pass) bundler > splitting/DynamicImportMultipleCSSImports [14.04ms]
(pass) bundler > splitting/StaticAndDynamicCSSImports [14.61ms]
(pass) bundler > splitting/NestedDynamicImportWithCSS [13.20ms]
(pass) bundler > splitting/SharedCSSBetweenChunks [15.64ms]
(pass) bundler > splitting/DynamicImportChainWithCSS [14.41ms]
(pass) bundler > splitting/ConditionalDynamicImportWithCSS [14.44ms]
(pass) bundler > splitting/MultipleEntryPointsWithSharedCSS [19.35ms]
(pass) bundler > splitting/DynamicImportWithOnlyCSSNoJS [13.03ms]
(pass) bundler > splitting/CircularDynamicImportsWithCSS [15.17ms]
(pass) bundler > splitting/DeadDynamicImportTargetGetsNoChunk-cli [11.63ms]
(pass) bundler > splitting/DeadDynamicImportTargetGetsNoChunk-api [13.74ms]
(pass) bundler > splitting/DeadDynamicImportTargetNoSplittingReference [13.31ms]
(pass) bundler > splitting/DeadDynamicImportTargetKeptWithoutTreeShaking [12.21ms]
(pass) bundler > splitting/LiveDynamicImportTargetStillGetsChunk [16.91ms]
(pass) bundler > splitting/DynamicImportChainLiveness [17.31ms]
(
... (truncated)
```

</details>

<details><summary>passes on PR (with fix)</summary>

```console
ASAN with fix: 4 skipped
$ BUN_DEBUG_QUIET_LOGS=1 bun scripts/build.ts --profile=debug --quiet test "--reporter=junit" "--reporter-outfile=/tmp/pr_gate.xml" test/bundler/bundler_splitting.test.ts
bun test v1.4.3 (09bb546)

test/bundler/bundler_splitting.test.ts:
(pass) bundler > splitting/DynamicImportCSSFile [1234.67ms]
(pass) bundler > splitting/DynamicImportMultipleCSSImports [657.65ms]
(pass) bundler > splitting/StaticAndDynamicCSSImports [621.68ms]
(pass) bundler > splitting/NestedDynamicImportWithCSS [666.60ms]
(pass) bundler > splitting/SharedCSSBetweenChunks [821.23ms]
(pass) bundler > splitting/DynamicImportChainWithCSS [851.81ms]
(pass) bundler > splitting/ConditionalDynamicImportWithCSS [707.22ms]
(pass) bundler > splitting/MultipleEntryPointsWithSharedCSS [918.65ms]
(pass) bundler > splitting/DynamicImportWithOnlyCSSNoJS [607.51ms]
(pass) bundler > splitting/CircularDynamicImportsWithCSS [725.79ms]
(pass) bundler > splitting/DeadDynamicImportTargetGetsNoChunk-cli [711.70ms]
(pass) bundler > splitting/DeadDynamicImportTargetGetsNoChunk-api [505.03ms]
(pass) bundler > splitting/DeadDynamicImportTargetNoSplittingReference [608.86ms]
(pass) bundler > splitting/DeadDynamicImportTar
... (truncated)

release with fix: 4 skipped
$ bun scripts/build.ts --profile=release
[configured] bun-profile → bun (stripped) in 990ms (unchanged)
ninja: Entering directory `/workspace/bun/build/release'
[0/5] cargo bun_runtime → libbun_runtime.a
�[1m�[92m   Compiling�[0m bun_core v0.0.0 (/workspace/bun/src/bun_core)
�[1m�[92m   Compiling�[0m bun_errno v0.0.0 (/workspace/bun/src/errno)
�[1m�[92m   Compiling�[0m bun_ptr v0.0.0 (/workspace/bun/src/ptr)
�[1m�[92m   Compiling�[0m bun_boringssl_sys v0.0.0 (/workspace/bun/src/boringssl_sys)
�[1m�[92m   Compiling�[0m bun_safety v0.0.0 (/workspace/bun/src/safety)
�[1m�[92m   Compiling�[0m bun_base64 v0.0.0 (/workspace/bun/src/base64)
�[1m�[92m   Compiling�[0m bun_cares_sys v0.0.0 (/workspace/bun/src/cares_sys)
�[1m�[92m   Compiling�[0m bun_zlib_sys v0.0.0 (/workspace/bun/src/zlib_sys)
�[1m�[92m   Compiling�[0m bun_zstd v0.0.0 (/workspace/bun/src/zstd)
�[1m�[92m   Compiling�[0m bun_picohttp v0.0.0 (/workspace/bun/src/picohttp)
�[1m�[92m   Compiling�[0m bun_brotli v0.0.0 (/workspace/bun/src/brotli)
�[1m�[92m   Compiling�[0m bun_output v0.0.0 (/workspace/bun/src/output)
�[1m�[92m   Compiling�[0m bun_collections v0.0.0 (/workspace/bun/src/collections)
�[1m�[92m   Co
... (truncated)
```

</details>

<details><summary>diff hotspot</summary>

```
src/bundler/LinkerContext.rs                       |   5 +-
 src/bundler/linker_context/crossChunkNames.rs      |   5 +
 .../linker_context/generateChunksInParallel.rs     |   7 ++
 src/bundler/linker_context/renameSymbolsInChunk.rs |  57 +++++++++--
 src/js_printer/renamer.rs                          | 111 ++++++++++++++++-----
 test/bundler/bundler_splitting.test.ts             |  67 +++++++++++++
 6 files changed, 218 insertions(+), 34 deletions(-)
```

</details>

**gate history** · 2 passed · 0 rejected · iteration 0

<details><summary>evidence per changed file</summary>

```
file                                                    reads  edits  tests
src/bundler/LinkerContext.rs                                0      0     34
src/bundler/linker_context/crossChunkNames.rs               1      0     34
src/bundler/linker_context/generateChunksInParallel.rs      1      0     34
src/bundler/linker_context/renameSymbolsInChunk.rs          1      1     34
src/js_printer/renamer.rs                                   6      7     34
test/bundler/bundler_splitting.test.ts                      5      4     34
```

</details>

<!-- robobun:evidence:end -->
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.

1 participant