Repository navigation
Conversation
|
No actionable comments were generated in the recent review.
|
| Layer / File(s) | Summary |
|---|---|
Track and merge barrel requests src/bundler/barrel_imports.rs |
Adds per-side tracked request storage and source-path indexing. Barrel optimization combines current and persisted requests, and build completion persists requests for tracked barrels. Removes per-alias persistence. |
Connect tracked requests to dev-server builds src/bundler/bundle_v2.rs, src/bundler/lib.rs, src/runtime/bake/dev_server/* |
Adds graph-side mapping and build-time request persistence. Replaces the separate dev-server maps and callbacks with TrackedBarrels and has_file. Updates cache checks, memory accounting, and a debug assertion. |
Test request retention across builds test/bake/dev/bundle.test.ts |
Adds client, server, and SSR regression tests for retained requests, deferred exports, and graph-specific barrel copies. |
Possibly related PRs
- oven-sh/bun#46742: Changes how HMR lowering preserves import records, which barrel optimization uses.
Suggested reviewers: alii, jarred-sumner
Priority:
Merge Risk: 
Minimal · up to f070c
-
No actionable correctness or availability issue was established. The change is ready to merge after normal checks, with the documented bundle-size tradeoff.
Pre-merge checks: 
4
-
Full details: Description check
-
Explanation
The description is complete and relevant. It explains the problem, fix, design, tradeoffs, known limitations, and verification results. It does not use the template headings exactly, but it provides the required information through equivalent sections such as “Problem,” “Fix,” and “How the state moves.”
-
- 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.
|
Updated 2:55 AM PT - Oct 25th, 2026
✅ @robobun, your commit f070caf032d50dbb6b6c5772dfc141e7f4a2e3b7 passed in 🧪 To try this PR locally: bunx bun-pr 46648That installs a local version of the PR into your bun-46648 --bun |
|
Status: ready for review. How this was reproduced. A dev server (
The sequence PR: #46648 |
|
A prototype of the follow-up that "Still open" names (do not parse and send a barrel again on each save of an importer) is on the branch What it does: Measured on release builds, main 5c6d4e2 against the branch. JS bytes in the hot update for one save of a file that imports the barrel:
Why it is not a PR:
|
… again A dev server build parses only the new and the changed files. A barrel that it parsed again chose its deferred records from the requests of that build and from a set of export names that only named imports wrote. A request for every export (import * as, export * from, require(), import()), a name that reached an inner barrel through an outer barrel, and a request made while the barrel was cached were lost. A later build then deferred a record that a module of an earlier build still reads, and that module read undefined. TrackedBarrels replaces the two DevServer maps. It has one entry per side of the module graph and per path, for each barrel that has deferred a record. Such a barrel is parsed in every build that reaches it, as before. When every file of a build is parsed, the requests of that build are added to the entries, each under the source index that its importer recorded it for. An entry that got a request for every export never defers again. A barrel that the dev server has bundled and does not track never defers.
The test removed two barrels that one module imports and then built a page that reads both. The dev server loads that importer again when the first barrel fails to open. Whether the second barrel gets a new copy in the same build depends on when its failure arrives, on main too. The test now removes one barrel for each build and reads the client bundle of each page.
Keep main's comment at the dev server block of the barrel pass and at the re-parse check of is_file_cached.
…y a walk The barrel pass looked for requests under another source index of its own path by a walk over every request slot of the build. With N barrels in one build that is N walks over about 3N slots. It now keeps the source indices of the build by the hash of their path and reads one chain per barrel.
SourcesByPath: the other indices of a path, and two paths with one hash.
248f5e3 to
f070caf
Compare
|
This comment is replaced by #46648 (comment). #46944 is closed. |
|
#46944 was the follow-up that targeted this branch. I closed it. This comment has what that work found about this PR. This branch alone answers worse than main. The barrel tests of #46944 (
Why #46944 is closed. Commit 1 kept the requests of a failed build in a second table of
Two decisions that this PR needs from a maintainer. More code in
What can be reused. The prototype branch has both commits and 41 devTests in Until those decisions are made, do not merge this PR alone. It fails the 15 tests above that main passes. |
Problem
"sideEffects": falsebarrel. After another route builds, it readsundefined(r2 B, thenr2 undefined).DevServer.barrel_needed_exports, which only named imports wrote (src/bundler/barrel_imports.rs:561). Requests for every export or through an outer barrel were lost.Fix
TrackedBarrelsreplaces the twoDevServermaps: one entry per graph side and path. Each build adds its requests once (persist_requests_of_tracked_barrels).test/bake/dev/bundle.test.ts(12 fail on main), 2 unit tests. Self-reviewed: 46 concerns raised, 41 addressed. Notes name the rest.Background
SourcesByPathfinds them.Downsides
import *is removed).Notes
Origin. A fuzz run of dev server sequences found the shape below. No user reported this exact shape. It is the second "Not changed" bullet of #46229.
New tests that fail on main (release build of main 179664e):
barrel optimization: ...)export *,export * as,import * as,import(),require())whole: undefined undefined undefined undefined undefinedouter: OWN undefinedsaved: A undefinedpersist_barrel_exportkey defect from the review of #45248)first: undefined/wholehas noWHOLE_Bwhole: undefinednamed: undefinedwhole: undefinedwhole: undefinedEXPORT_Cpage: EXPORT_ASix more new tests pass on main. They pin what this design must keep: a saved barrel defers again in the next build, a barrel that is parsed again still defers what nothing requested (server copy and SSR copy), a loaded outer barrel gets the exports that a later build loads (server, and an open page), and a name requested from a "use client" barrel survives a later build.
How the state moves.
TrackedBarrelshas one map per side of the dev server's module graph. The server and SSR copies of a file share one graph record and one list of imports, so they share one entry.Partialentry.is_file_cachedreturnNone. Every build that reaches the barrel parses it, as on main.Partial: the names of the entry count as requested.All: the barrel returns before the deferral loop.finish_from_bake_dev_servercallspersist_requests_of_tracked_barrelsonce. It adds each slot ofrequested_exportsto the entry of its path, on the side of the copy that has that source index. A request for every export makes the entryAll.Design notes.
Allentry is still parsed and sent in every build that reaches it. A re-export in a node_modules file, and everyexport *, is a value that the importer copies when it is evaluated. Only a new copy of an outer barrel gets the exports that an inner barrel loads later. A version that cached such a barrel leftundefinedin a loaded outer barrel until restart.SourcesByPathfinds those indices: a map from the hash of a path to its newest source index, and for each source the next older index with the same hash. Each build adds only its new sources. The first version of this PR walked every slot ofrequested_exportsfor each barrel (CPU table below).on_file_deleted), so its requests count for both sides.graph.entry_points.iter().any(..). It is main's code and is left as it is.has_filefor the server side is true only for a record that a build bundled (kindJs). A route file has a record of unknown kind from startup.finalize_bundlehas adebug_assert!: a copy with a deferred record has an entry.Downsides in detail.
import()andrequire()are such requests), a first build that took the barrel whole, a first build in which the file was not a pure barrel, and a first build of the client copy that failed. No save clears it. main deferred again on the next build that parsed the barrel.import *,import(),require(),export *): after the importer goes back to a named import, main prints the value. This branch shows the error of that file on every page that imports the barrel, until the server restarts.c.jstoc.jsx). The resolver still names the old path from its cached listing of the directory, and the read fails. On main only the page that requests that export gets the dead import. On this branch, after a request for every export, each page that imports the barrel gets it (server code: 500), until restart. The stale listing is a defect of main and has no fix there yet. dev server: read a directory again when a file that no build bundled fails to open #46846 is a draft for it: its prototype reads the directories again in the next build, and it waits for bundler: advance the resolver generation once per build instead of once per queue drain #38212 and resolver: keep cached DirEntry when a stale-generation refresh fails #36874. Two tests for this case (client and server code) pass onrobobun/761f31d9/barrel-requests-with-generation-prototype, which has this PR and that prototype (84 tests in the file, debug build). They fail on this branch alone, so they are not in this PR.Still open after this PR (each one is the same on main):
export *after another statement of the same path loses exports of a barrel target #46229. Two statements of one path plus a late request still printgot: C undefinedin the first build. The branchrobobun/fd59aa7e/hmr-merged-import-recordworks on it and changes the same function.undefinedreads that the fuzz still finds on this branch are of this kind, where inspected. Serve fresh exports when CommonJS modules hot-reload in the dev server #31944 marks every module of an update stale before it reloads any.bun test --changedscan #45938 adds the guard that such a switch needs), and the conditions for a later change that caches a barrel with anAllentry (2 concerns, it needs the reload order change first).SourcesByPath, and the side rule for a failed copy in the design notes. The other five are about the stale listing: they went to dev server: read a directory again when a file that no build bundled fails to open #46846 and to separate reports (an importer thaton_file_deletedskips because it no longer resolves, a package directory that is a symlink).has_filecounts a failure record. That part is new.undefineduntil the next build. The new boundary loop ofpersist_requests_of_tracked_barrelscorrects only the next build.TrackedBarrels::memory_costhas no test.barrel_imports.rs. The appended tests conflict.Measurements (main 5c6d4e2 against the first version of this branch, release builds, if a line names no other commit). Fixture A: a page with 50 named imports and no barrel. Fixture C: a page that imports one name from each of 20 barrels. Fixture L:
import { Camera } from 'lucide-react'(1.61.0).llvm-size, main 179664e): 107,556,528 -> 107,564,208 bytes (+7,680). BeforeSourcesByPathit was +4,608.DevServerHandlelink slots 11 -> 10.bun buildof fixture A,schedule_barrel_deferred_imports(gdb stepi): 34,949 -> 34,011 instructions for the file with 50 named imports, 2,722 -> 2,705 for each imported module.is_file_cachedper call (gdb stepi): no tracked barrel 206 -> 160 (file not in the graph) and 273 -> 226 (cached file). With 20 tracked barrels 188 -> 219 and 265 -> 301.on_parse_task_complete, first build (gdb): A 741 -> 650, C 2,678 -> 2,604 plus 60 in the end-of-build step.perf_event_open, interleaved runs): A median 740,526 -> 754,920 with a spread of 679k to 926k. C 1,732,934 -> 1,779,827 with a spread of 1.59M to 1.86M. L 11.73M -> 12.34M with a spread of 11.0M to 13.8M. Each difference is inside the spread./namedhasimport { Camera }and/wholehasimport * as:/namedships 622,618 bytes in 16 modules at first, and 3,285,979 bytes in 1,919 modules after/wholeis built, on both. Afterwhole.tsgoes back to a named import, main ships 622,618 bytes and this branch 3,285,979 bytes.perf,valgrind,bloatyorstrace. The numbers come from gdb,nm,llvm-sizeand aperf_event_openwrapper.CPU of the barrel pass at scale. A page that imports one name from each of N two-line
sideEffects: falsebarrels. CPU ms (process.cpuUsage, user plus system) of the first build and of a save of the page, release builds, 3 interleaved runs, median in parentheses. main is 179664e. "walk" is this PR beforeSourcesByPath(248f5e3). The first run of main in each group reads the files cold.sideEffectssideEffectsThe bytes served are equal on all three. The lookup is pinned by
a barrel that was not there to read keeps the requests of that build(it fails if the lookup returns nothing) and by two unit tests inbarrel_imports.rs(the other indices of a path after a second indexing pass, and two paths with one hash). CI runs the unit tests undercargo miri test -p bun_bundler.Fuzz. Random sequences of route loads, saves and failing routes over barrels, 356 seeds in 12 modes (HTML routes, server graph, one server graph with a client entry, separate SSR graph, each with and without nested barrels, "use client" barrels and app-level barrels). The two release builds ran the same seeds side by side.
undefinedread: 56 on main, 14 on this branch. No seed fails only on this branch.undefinedreads: 169 on main, 35 on this branch. 34 of the 35 are on main at the same step. The other one depends on timing: in 6 reruns main has it 6 times and this branch 2 times.debug_assert!did not fire, and the results match the release build.Suites run on the debug build of f070caf:
test/bake/dev/bundle.test.ts(72),test/bundler/bundler_barrel.test.ts(70),test/bake/dev/{css,ecosystem,esm,harness,hot,html,import-meta-inline,import-meta-inline-negative,incremental-graph-edge-deletion,plugins,react-response,request-cookies,server-sourcemap,sourcemap,ssg-pages-router,stress,vfile}.test.ts,test/bake/{app-options,deinitialization,dev-and-prod,framework-router,hmr-socket-protocol,serve-plugins-dev-server}.test.ts. On a release build:bundle.test.ts3 times, its 26 barrel tests 10 times,test/bake/dev/production.test.ts(21) andreact-spa.test.ts(10). On the debug build 10 tests ofproduction.test.tsand 1 ofreact-spa.test.tsreach the 5 s limit. They pass on release builds of this branch and of main.[human-review] gate passed · iteration 1 · 10 files touched
fails on main (without fix)
passes on PR (with fix)
diff hotspot
gate history · 1 passed · 1 rejected · iteration 1
evidence per changed file