Repository navigation
bundler: report the emitted size of each chunk as metafile outputs[..].bytes - #42760
Conversation
…].bytes The metafile fragment of a chunk was built before code() ran, from IntermediateOutput::get_size(). That sum leaves out every path and HTML manifest code() writes over a placeholder, the debugId comment code() adds, and the sourceMappingURL comment appended after it. Record the length of the emitted buffer on the chunk where the output is made, and build the fragment in metafile_builder::generate, which runs after generate_chunks_in_parallel. get_size() and Chunk::metafile_chunk_json have no other use and are removed. With that borrow gone every borrowed node of the joiner in generate() is 'static, so the unsafe detach_lifetime() goes too.
|
Warning Review limit reached
On-demand reviews are free for the next 6 days. After that, they cost $0.25 per reviewed file. Or wait 6 minutes for your next included review. View limit detailsLimit details: You’ve used all 10 included reviews currently available. Review configuration: ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Essentials Run ID: 📒 Files selected for processing (5)
Comment |
|
Status
|
There was a problem hiding this comment.
LGTM — a clean root-cause fix that records the emitted buffer length at each of the three emission sites and reads it after emission.
What was reviewed:
- Verified
metafile_builder::generateruns aftergenerate_chunks_in_parallelat both call sites inbundle_v2.rs, and thatwrite_output_files_to_diskis invoked from within it, sofinal_output_sizeis set before it is read. - Checked that
generate_chunk_jsonno longer reads any state (intermediate_output) consumed between the old and new call sites; every field it reads survives to thegeneratecall. - Confirmed the
detach_lifetime()removal is sound: the only remainingpush_staticinputs areb"…"literals andImportKind::label() -> &'static [u8]; the fragment now goes throughpush_owned. - Test file already imports
bunExe/bunEnv(line 865), so the new CLI test resolves; subprocess pipes are drained concurrently and the matrix covers all sourcemap modes × on-disk/in-memory plus the standalone-HTML path for the third assignment.
Extended reasoning...
Overview
This PR fixes outputs[..].bytes in the bundler metafile so it reports the actual emitted file size rather than the pre-substitution intermediate size. It replaces Chunk::metafile_chunk_json: Box<[u8]> with Chunk::final_output_size: usize, sets that field at each of the three places a chunk's final buffer is produced (generateChunksInParallel.rs standalone-HTML scc loop, the in-memory insert_for_chunk path, and writeOutputFilesToDisk.rs), and defers generate_chunk_json until metafile_builder::generate, which both callers in bundle_v2.rs already invoke after generate_chunks_in_parallel. Dead code is removed in the same commit: IntermediateOutput::get_size(), the pre-loop that populated metafile_chunk_json, and the unsafe { j.detach_lifetime() } with its SAFETY comment.
Security risks
None. The change touches only metafile JSON assembly and a usize field on Chunk; no user input parsing, no filesystem path handling beyond what already existed, no new unsafe (one unsafe is removed). The removed detach_lifetime was the only lifetime escape hatch in the function, and I confirmed every remaining push_static argument is 'static (ImportKind::label() returns &'static [u8]), so the borrow checker now enforces what the old SAFETY comment claimed.
Level of scrutiny
Moderate. This is a self-contained bug fix in a diagnostic output path (the metafile), not runtime behavior. The main risk was a state-ordering regression from moving generate_chunk_json to run later — I traced every field it reads (final_rel_path, files_with_parts_in_chunk, cross_chunk_imports, entry_point, content.css_chunks, c.graph.meta) and none is consumed or mutated between the old and new call sites; the only consumed field was intermediate_output, which the new code no longer reads. Error handling improved slightly: unwrap_or_default() (which produced invalid JSON on error) is replaced with ? propagation into the existing "Failed to generate metafile" warning.
Other factors
Test coverage is thorough per REVIEW.md's variant-matrix rule: test.each over four sourcemap modes × on-disk/in-memory, copied assets from JS and CSS (with a 128 KB asset to defeat data-URL inlining), HTML entry points, standalone compile: true HTML (the only path exercising the scc assignment), and the CLI --metafile/--metafile-md flags. Tests use tempDir, bunExe/bunEnv (already imported mid-file at line 865), Buffer.alloc(n, fill), drain stdout/stderr/exited concurrently, and assert structured toEqual on { reported, actual } maps. No CODEOWNERS entry covers the changed paths. No prior review comments or outstanding objections on the PR.
|
Updated 5:04 PM PT - Sep 14th, 2026
✅ @robobun, your commit 93429ba1977cb69458efd084da72b12ffedd6a49 passed in 🧪 To try this PR locally: bunx bun-pr 42760That installs a local version of the PR into your bun-42760 --bun |
Problem
bun build --metafile,Bun.build({ metafile: true })),outputs[path].bytesis smaller than the emitted file when the chunk references another output. On 1.4.3m0.jsis 130 bytes on disk, 98 in the metafile.bun.d.tsdocuments the field as "Size of the output file in bytes".generate_chunk_json(src/bundler/linker_context/MetafileBuilder.rs:75). It ran beforecode()and wroteIntermediateOutput::get_size()(src/bundler/Chunk.rs:610), which adds only the literal pieces. It leaves out every path and HTML manifest thatcode()writes, and the//# debugIdand//# sourceMappingURLcomments.Fix
Chunk::final_output_sizerecords the length of the emitted buffer. Each place that makes the final buffer sets it: two ingenerateChunksInParallel.rs(standalone HTML, in-memory) and one inwriteOutputFilesToDisk.rs.metafile_builder::generatebuilds each chunk fragment. Both callers run it aftergenerate_chunks_in_parallel, so the size is known. esbuild defers this fragment in the same way.get_size(),Chunk::metafile_chunk_json, and theunsafe detach_lifetime()ingenerate. The fragment was its only borrow that was not'static.test/bundler/metafile.test.ts, 15 new cases, all fail on 1.4.3. Also theesbuild/metafile,bundler_compile_splitting,standalone,bundler_htmlandhtml-import-manifestsuites.Background
IntermediateOutput::Piecesholds the printed text, split at each placeholder.code()joins the pieces and writes the final path at each split.BuildArtifact.Notes
Repro (1.4.3 and main at 5fce36e):
bytesbeforebytesafter./m0.js(2 imports)./m0-5begjve3.js(1 import)./m1.js(1 import)./m0-8x1hvvzq.js(no import)Each import costs 16 bytes, the length of
./m0-xxxxxxxx.js. Other cases measured on 1.4.3:--sourcemap=linked:m0.jsis 207 on disk, the metafile said 98.--sourcemap=inline: 611 on disk, 98 in the metafile.--sourcemap=external: 176 on disk (the//# debugIdcomment, whichcode()writes), 98 in the metafile.import logo from "./logo.svg"(file loader): 98 on disk, 79 in the metafile.index.htmlis 218 on disk, 160 in the metafile.server.jsnow matches, 889 bytes.esbuild 0.25.1 on a split build with source maps reports the exact file size (
a.js: 146 on disk, 146 in the metafile). Its linker passeslen(outputContents)tojsonMetadataChunkCallback(finalOutputSize)after it appends thesourceMappingURLcomment.The new tests compare
outputs[key].byteswith the size of each emitted file:sourcemapmode (none,linked,inline,external), with anoutdir(the disk loop) and without (the in-memory loop).data:URL.compilewithtarget: "browser"). This is the only case that reaches the third assignment. The test compares thebytesof each inlined chunk with the text between its tags in the document.--metafileand--metafile-md. The test reads the[ENTRY: ... (N bytes)]lines of the report.With any one of the three assignments removed, the chunks of that path report 0 and its cases fail.
More builds checked by hand against the fixed binary, all equal to the file on disk:
--bannerand--footer, a shebang entry with--target=bun,--minifywith--sourcemap=linked,--format=cjs --bytecode,--public-path,--entry-naming '[dir]/[name]-[hash].[ext]'with entry points in subdirectories, and a CSS entry point with an@importand a 200 KBurl()asset.Why a field on the chunk and not
OutputFile.size: a standalone HTML build gives each inlined chunk an empty placeholderOutputFileand then filters those out of the list. The index of a chunk and the index of its output file match only by adebug_assert, and not in that mode.What changes for a user:
bytesgrows by the paths, by thedebugIdcomment, and by thesourceMappingURLcomment. Withsourcemap: "inline"that comment holds the whole map. The number now equalsBuildArtifact.size.--metafile-mdreport uses the same numbers for "Total output size", "Bundle size", the chunk sizes and the "% of Total" column.--compile --target=browserinlines the JS and CSS chunks into the document.index.htmlnow reports its real size (403, was 203). An inlined chunk keeps itsoutputsentry, as before. Itsbytesis the size of the inlined code.--compileembeds each chunk in the executable.bytesis the size of the embedded module.generate_chunk_jsonnow failsgeneratewith the existing "Failed to generate metafile" warning. Before,unwrap_or_default()left an empty fragment and the JSON was not valid. The only fallible call is a write to aVec.Not changed here, tracked in #42758:
.map,.jscand copied asset files have nooutputsentry. This is a different defect (a missing key, not a wrong value) and it adds keys that users see.inputs[..].bytesInOutputcounts a placeholder at its own length. A fix needs a count per compile result after the substitution, as in esbuild'saccurateFinalByteCount.Self-reviewed: 5 concerns raised, 5 addressed. A test for the standalone HTML assignment. The
unsafethat this change made needless. Stricter test assertions (statSync, exact key lists, one combined CLI assertion). ThedebugIdwording in the commit message. A tracker for the gaps above.Open PRs that touch
MetafileBuilder.rsbut not this field: #38341, #38624, #36974, #38666. None reads state that is gone whengenerateruns. #38666 has a small text conflict inChunk.rswhere this PR deletesget_size().no test proof · iteration 0 · platform-specific test(s) that do not run on this machine, deferring to CI, which covers all platforms: test/bundler/metafile.test.ts