Repository navigation
Conversation
The loop that builds the exports object of a CSS module called `E::Object::put` for each local name. `put` searches all earlier properties for the key, so N names cost N(N-1)/2 comparisons. The keys are the keys of `local_scope`, a map, so the search never finds one. The loop now calls `E::Object::append_property` and reserves the capacity once. A `debug_assert!` checks that each key is the `local_scope` key of its entry. The output does not change.
|
Status: ready for review. How I reproduced it. I generated a PR: #45824 |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
WalkthroughThe linker now reserves capacity and appends CSS-module exports after checking key-to-symbol mappings in debug builds. A new bundler test covers export-key ordering, deduplication, composed values, and hash consistency. ChangesCSS-module exports
Suggested reviewers: Priority: ➖ Normal Merge Risk: ⚪ Minimal · up to
|
| Check name | Status | Explanation |
|---|---|---|
| Title check |  |
The title clearly and concisely describes the main change: using append behavior to build CSS module exports without a per-name lookup. |
| Description check | |
The description provides a detailed explanation of the problem, fix, behavior impact, performance results, testing, and limitations. It does not use the exact template headings, but it includes the re… |
| Linked Issues check | |
Check skipped because no linked issues were found for this pull request. |
| Out of Scope Changes check | |
Check skipped because no linked issues were found for this pull request. |
Full details: Description check
-
Explanation
The description provides a detailed explanation of the problem, fix, behavior impact, performance results, testing, and limitations. It does not use the exact template headings, but it includes the required information for what the PR does and how it was verified.
✨ Finishing Touches: 💡 1
-
🛠️ Fix failing CI checks: 💡
-
- Commit to this branch
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
Comment @coderabbitai help to get the list of available commands.
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
LGTM — a small, behavior-preserving swap of put for append_property whose no-duplicate-keys precondition holds structurally.
What was reviewed:
- Key uniqueness:
add_symbol_for_nameinsrc/css/css_parser.rspushes a symbol only on alocal_scopemiss and setsoriginal_nameto the map key, and nothing else inserts intolocal_scope, so each iterated entry yields a distinct key regardless of thedebug_assert!compiling out. putvsappend_propertyfor a fresh key: sameown_key_property_flags, same keyloc(the value's loc isstmt.loc), so__proto__still becomes an own property; the dropped?is safe becauseputnever actually returned an error.ensure_total_capacityonVec<Property, AstAlloc>is the infallibleVecExtmethod; the reservation ahead of the fallibleBitSet::init_emptyonly matters on OOM.- New test asserts exact
Object.entriesorder and composed values in the existingcss-modules.test.ts; it passes on main, which is expected for a refactor with no behavior change.
Extended reasoning...
The change touches one loop in the CSS-modules branch of the bundler linker's lazy-export codegen (src/bundler/linker_context/generateCodeForLazyExport.rs), replacing a linear-scan dedupe insert with an unconditional append plus a capacity reservation, and adds one itBundled test. It touches no security-sensitive surface. Approval rests on the uniqueness invariant being enforced by the CSS parser's symbol creation (one symbol per local_scope key), on the two insert paths producing identical properties for new keys, and on the small size of the diff; no CODEOWNERS entry covers the changed files and the hunt exited on dry_streak with no findings.
|
Updated 12:11 PM PT - Oct 19th, 2026
✅ @robobun, your commit 66c7268d1f56fab77c3f5b48906431ef773693fc passed in 🧪 To try this PR locally: bunx bun-pr 45824That installs a local version of the PR into your bun-45824 --bun |
Problem
*.module.cssfile withE::Object::putfor each local name (src/bundler/linker_context/generateCodeForLazyExport.rs:357).putsearches all earlier properties (src/ast/e.rs:1398): N names cost N(N-1)/2 comparisons.Fix
E::Object::append_propertyand reserves the capacity once.local_scopemap. Adebug_assert!checks it.bulma.cssas a module (3,313 names): 404 M to 301 M. The output is the same for 17 fixtures and 2,000 random modules.test/bundler/css/css-modules.test.ts(new case),bundler_loader.test.tsandesbuild/css.test.ts. Self-reviewed: 12 concerns raised, 12 addressed.Behaviour change: none
Background
local_scopemaps each local name of a file to its symbol.E::Object: +8 B on each object literal. Considered thecomposesvisited set too: it costs 1 ms at 40,000 names, and bun test: Vitest compatibility #44833 deletes it.Downsides
.textgrows 512 B. Peak RSS at 20,000 names: 75.8 MB to 85.2 MB (cause in Notes).Notes
Method. Release builds (
bun scripts/build.ts --profile=release) of main ff8ad61 and of this branch, linux x64, in one build directory. The command isbun build entry.js --outdir out.perfis not installed on the machine, so a smallperf_event_openwrapper countsinstructions:uof the process and all its threads. Each instruction count is the minimum of 3 runs (1 run for the 40,000 cells of the two slow shapes). CPU is user + sys fromwait4, the minimum of 5 interleaved runs (3 runs for those cells). Peak RSS isru_maxrss, the median of 9 runs.Fixtures. Three module shapes and the same rules as plain CSS:
User instructions, in millions: main, then this PR.
composesCPU in ms: main, then this PR. The runs of one binary spread by 6 ms to 65 ms below 1 s of CPU, and by up to 161 ms above.
composesReal stylesheets. Files from
test/js/bun/css/files/, copied tostyles.module.cssand built with the sameentry.js. User instructions in millions, minimum of 9 runs. The CPU ofbulma.cssis 59 ms, then 54 ms (minimum of 9), which is less than the spread of the runs.bulma.csstachyons.cssbootstrap-4.csstailwind.cssuikit.cssThe output of these five, and of
materialize.css,foundation.min.css,animate.cssandpico.css, is the same on both binaries.Doubling. Instructions of the module build (rules that merge) minus the same rules as plain CSS. Each ratio is to the row above.
The ratio of this PR rises above 2 because the loop still clears the
composesbit set for each name (see "What stays").Calls, counted with gdb breakpoints, for a module of 2,000 names.
E::Object::put: 2,000 on main, 0 here.E::Object::as_property: 2,000 on main, 0 here. Grows of an AST-allocated vector (RawVecInner<AstAlloc>::finish_grow): 8,585 on main, 8,576 here. The reserve removes 9 grows of the property list. The plain CSS build makes 0, 0 and 486 of these calls on both binaries.Same output.
stdout,stderr, the exit code and each byte of each output file, compared between the two release builds.--format=esm,--format=cjs,--format=iifeand--minify, a 3,000 name module with thecomposesshapes of the new test (as a JS import in two formats, and as the entry point), rules that do not merge, 2,000 classes that each compose 16 classes, and a module with threecomposeserrors (exit code 1).:global(), names such as__proto__andfoo-bar, repeated names,composesfrom the same file, from another file and from global, cycles through the exported name, and rare errors. The entry is a default import, a namespace import, a dynamic import, or the module file. Flags are random formats, targets and minify options. 277 cases end with an error, with the same diagnostics.Size.
sizeon the releasebun:.text106,483,760 B on main, 106,484,272 B here.nm -Sonbun-profile:scan_imports_and_exports(the loop is inlined there) 45,328 B to 45,879 B. One drop function of 111 B is gone. No other symbol changes size.Peak RSS in MB: main, then this PR. With the default settings: 3,000 names 29.0, 28.8. 10,000 names 48.8, 52.7. 20,000 names 75.8, 85.2. 40,000 names 124.9, 127.4. With
MIMALLOC_PURGE_DELAY=0: 24.9, 24.9. 40.7, 39.5. 65.3, 62.4. 113.1, 106.0. So this PR allocates less. The likely cause of the default numbers: mimalloc returns freed pages after a delay, and main spends 0.1 s to 1.7 s more in the search before the build ends.What stays, with its owner.
composesvisited set. Before the walk of each name, the loop clears a bit set that has one bit for each name: n x n / 8 bytes ofmemset. A C loop that does the same writes takes 0.9 ms at 40,000 names (200 MB), 23 ms at 200,000 and 304 ms at 640,000. A draft of this PR replaced the bit set with epoch stamps. That saved 9 M of 1,218 M instructions at 40,000 names and 7% at 640,000, and it conflicted with bun test: Vitest compatibility #44833 and bundler: keep deep CSS, export star and composes graphs from overflowing the linker's stack #38966 in 3 places each. bun test: Vitest compatibility #44833 deletes this visitor, and itsComposesVisitorneeds no pass over all names. bundler: keep deep CSS, export star and composes graphs from overflowing the linker's stack #38966 keeps the bit set.StyleRuleKeyMap::remove_duplicate(src/css/rules/mod.rs:945) scans one bucket, becauseIdentOrRef::hash(src/css/values/ident.rs:363) hashes 2 of the 16 bytes of a symbol reference. A rule withcomposesnever merges either, becauseComposes::eqlcompares the source location (src/css/properties/css_modules.rs:110). css/values/ident.rs: hash only uses 2 bytes of u128 #30772 tracks the hash, and css: hash full Ref identity in IdentOrRef::hash #30784 is open for it. With a hash of the full reference on top of this PR (an experiment, not part of this PR), 20,000 rules that do not merge take 688.8 M instructions, and 40,000 take 1,380.8 M.mangle_local_cssmakes an arena and hashes the file path again for each local name (src/bundler/LinkerContext.rs:2614). It is linear. No PR owns it.composescycle that does not pass through the exported class overflows the stack, on main and here:.x { composes: a } .a { composes: a }exits with SIGSEGV. This PR does not touch the walk. bundler: keep deep CSS, export star and composes graphs from overflowing the linker's stack #38966 and bun test: Vitest compatibility #44833 mark a class before they walk it.debug_assert!indoStep5.rs:377searches the symbol uses of a part for each use. A debug build of bun takes 18 s for a 20,000 name module because of it (1.3 s as plain CSS).E::Object::putfor each key of an object whose size the input decides (INI, TOML,bun pm pkg). pm pkg get, pm view: append the found values without a lookup per member #45368 and toml: index table keys so large tables parse in linear time #37449 change some of them. This is the only such call insrc/bundler.Overlap with open PRs. A three-way merge of this branch with #44833, #38966, #42058 and #45487 has no conflict in the three files of this PR. #44743 moves the whole block into a new function, so the two changed lines must move with it.
The test.
css-module/ExportsObjectKeysAndComposedValuesbuilds a module of 100 plain names around a chain, a diamond, a cycle through the exported name, two classes that compose one class,from globaland two cross-filecomposes, a repeated class and an id that reuses a class name. It runs the bundle and compares the entries of the default export, in order. It takes about 1 s on the debug build. It passes on main, because this PR does not change the output. A test that fails on main needs a time limit or a counter in production code, and neither is wanted.bun/loader-css-module-proto-class-is-own-propertycovers the__proto__flag thatappend_propertysets likeput. Its comment namedE::Object::put, so this PR rewords it.Suites, on the debug build.
test/bundler/css/css-modules.test.ts15 pass.test/bundler/esbuild/css.test.ts56 pass, 1 todo.test/bundler/bundler_loader.test.ts59 pass.Why no key repeats.
Parser::add_symbol_for_name(src/css/css_parser.rs:3009) is the only writer oflocal_scope. On a new key it makes one symbol whoseoriginal_nameis that key. The loop reads the key back from that symbol. Thedebug_assert!checks thatlocal_scopemaps the key to the entry in hand.Where the loop runs.
generate_code_for_lazy_exporthas one call site (scanImportsAndExports.rs:517), reached fromLinkerContext::link:bun build,Bun.buildand production builds. Hot reload in the dev server skips the linker (bundle_v2.rs:6045).Other designs. A key index inside
E::Objectmakes each object literal 8 bytes larger (40 B to 48 B) and changes no line of this loop. A shared index type for allputloops repeats #37449 and #45368.