Embed the source hash next to bytecode so compiled modules load without reading their source - #39399
Embed the source hash next to bytecode so compiled modules load without reading their source#39399robobun wants to merge 1 commit into
Conversation
…ut reading their source JSC builds the bytecode cache key from SourceProvider::hash(). Zig::SourceProvider never set m_hash, so the key fell through to StringImpl::hash(), which walks the whole source string. In a compiled executable that faults every page of the embedded source into memory for a module that, thanks to the bytecode cache, is never parsed. The build step already has the hash (it keys the bytecode with it), so return it from the bytecode generators, store it in CompiledModuleGraphFile, carry it through ResolvedSource and install it as m_hash when the provider is created with a bytecode cache. Only modules embedded as Latin1 record it, since that is the only encoding where the runtime string is the exact bytes the bytecode was keyed on; everything else stays 0 and hashes at runtime as before. The CommonJS module-wrapper override paths replace source_code, so they clear the hash.
|
Updated 8:47 PM PT - Aug 16th, 2026
✅ @robobun, your commit 7ce47bc8d8a00eed4064bedfbcd400157da60632 passed in 🧪 To try this PR locally: bunx bun-pr 39399That installs a local version of the PR into your bun-39399 --bun |
|
Status: closed unmerged. The maintainer who requested this measured it on macOS arm64 (see below): the pages the hash walks are clean file-backed pages that macOS does not count in |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (14)
Included review availability: Your plan includes up to 5 reviews per rolling hour; 0 remain after this review. WalkthroughChangesBytecode source-hash propagation
Possibly related PRs
Suggested reviewers: Merge Risk: ⚪ Minimal · up to This change avoids reading embedded module source when valid bytecode is available, with no actionable merge-blocking risk remaining after normal checks and review. 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
Comment |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
I reviewed this PR and didn't find any bugs. Because it changes an FFI signature, adds a field to two #[repr(C)] structs shared across Rust/C++, and threads state through ~8 layers of the bundler/standalone-graph/JSC path, a human look would still be worthwhile.
What was reviewed:
ResolvedSource/CompiledModuleGraphFilelayout: newu32field keeps 4-byte alignment with no padding; Rust and C++ sides updated in lockstep;m_hashis default-initialized to 0 in ZigSourceProvider.h so the non-bytecode branch still falls through correctly.to_bytesrefactor: hoisting theencodingcomputation is behavior-preserving (depends only onoutput_file.loaderandbuf_bytes, neither mutated in between);string_builderappend order is unchanged.- Hash correctness: only propagated for
Encoding::Latin1where runtime bytes match build-time bytes; bothhasOverriddenModuleWrapperpaths that rewritesource_codezero the hash; a wrong hash can only cause a cache miss, never wrong bytecode.
Extended reasoning...
Overview
Performance optimization for bun build --compile --bytecode: embed the JSC SourceProvider::hash() computed at build time next to each module's bytecode, so at runtime the bytecode cache key can be built without hashing (and thus faulting in) the entire embedded source string. The hash is captured in the C++ bytecode-generation functions via a new out-param, threaded through GeneratedBytecode → bytecode OutputFile → CompiledModuleGraphFile → File → ResolvedSource → SourceProvider::m_hash. 14 files touched across bundler, standalone graph, JSC bindings (Rust + C++), and runtime hooks, plus a Linux-only test measuring RSS via /proc/self/smaps.
Security risks
None identified. The new field is a u32 used only as a cache-key input; a mismatched hash produces a cache miss (JSC compares hash + length + flags + name + origin against what's stored inside the bytecode blob), never execution of wrong bytecode. No user-controlled input reaches new parsing or allocation logic.
Level of scrutiny
Medium-high. The mechanism is straightforward and fail-safe (0 = "compute at runtime", the pre-existing behavior), but the change spans an FFI ABI boundary (new out-param on two extern "C" functions, new trailing field on the shared ResolvedSource struct) and modifies the #[repr(C)] on-disk record format of the standalone module graph. These are exactly the kinds of changes where a subtle mismatch between Rust and C++ layouts, or a missed initialization site, produces hard-to-debug corruption. I verified: m_hash = 0 is default-initialized in ZigSourceProvider.h:64; the Rust ResolvedSource::default() and all output_file::Options construction sites initialize the new field to 0; the CompiledModuleGraphFile struct remains padding-free (56 bytes, 4-byte aligned) so the read_unaligned / bytes-cast serialization stays sound; and the graph format is only ever read by the same binary that wrote it, so no version bump is needed.
Other factors
The PR description is exceptionally thorough and the test is well-constructed: it asserts both the RSS delta (< 4 MB vs. 8 MB without the fix, comfortable margin) and that both modules still hit the bytecode cache (proving the embedded hash matches JSC's stored key). The to_bytes refactor in StandaloneModuleGraph.rs was checked line-by-line for behavior preservation. The two paths in JSCommonJSModule.cpp that rewrite source_code correctly zero the hash. NodeCompileCache was updated to discard the hash it doesn't need. Given the breadth (14 files, cross-language ABI change, on-disk format change) this warrants a maintainer's eyes even though I found nothing wrong.
|
Closing after measuring on macOS arm64 (release builds of
The pages the source hash walks are clean file-backed pages of the executable, which macOS does not count in |
|
Makes sense, thanks for measuring. For the record, the Linux side of the same setup (8 MiB CommonJS module, delta across |
2 participants
Problem
bun build --compile --bytecodeexecutable faults the module's entire embedded source into memory even though the bytecode cache means the source is never parsed. Measured on a CommonJS module with 8 MiB of source:require()grows the Rss of the executable's data mapping by 8128 KB (the whole source).Zig::SourceProvider::m_hash(src/jsc/bindings/ZigSourceProvider.h:64) is declared but never assigned, soSourceProvider::hash()(ZigSourceProvider.cpp:269) falls through toStringImpl::hash(), which walks the whole source string. JSC calls it from theSourceCodeKeyconstructor (SourceCodeKey.h:85) before it consults the bytecode cache. Whenmodule_infois present this is the only thing on the load path that reads the source at all, so on the bytecode path it is also pure CPU cost (and on signed macOS binaries each first-touched page additionally pays signature verification).Fix
generateCachedModuleByteCodeFromSourceCode/generateCachedCommonJSProgramByteCodeFromSourceCodereturnsourceCode.provider()->hash()through a new out parameter. That is exactly the value the runtime provider has to return for the stored key to match.bun_jsc::cached_bytecode->dispatch::GeneratedBytecode-> the bytecodeOutputFile(bytecode_source_hash) ->CompiledModuleGraphFile::bytecode_source_hash(au32; the graph format is only read by the binary that wrote it, same as every other field) ->File->ResolvedSource::bytecode_source_hash->provider->m_hashinSourceProvider::create, set only on the branch that attaches a bytecode cache.Encoding::Latin1, because that is the only case where the string the runtime hands JSC is byte-for-byte what the bytecode was keyed on.Binarymodules (any non-ASCII byte in the chunk) are decoded as UTF-8 into a different string; they stay at 0 and keep hashing at runtime, which is their existing behaviour (their bytecode already misses today; that is a separate pre-existing issue, handed off).hasOverriddenModuleWrapperpaths in JSCommonJSModule.cpp replacesource_code, so they zero the hash; a wrong hash can only ever produce a cache miss (JSC compares hash and length against what is stored in the bytecode), never the wrong bytecode for a source that was built correctly.if (m_hash)inhash()relies on.require()s the second one at runtime, and checks the Rss delta of the executable's writable mappings in/proc/self/smaps, plus that both modules still report[Disk Cache] Cache hit(which proves the embedded hash matches the key stored in the bytecode). Debug build: delta 64 to 128 KB, passes. Release 1.4.0 without the fix: delta 8128 KB, fails. Linux only because it reads/proc/self/smaps; CommonJS because debug builds re-parse ESM modules to cross-checkmodule_info, which reads the source regardless.compile/HelloWorldWithProcessVersionsBunfails on main too in debug builds, unrelated, handed off), test/regression/issue/26298.test.ts, bundler_bun, bundler_banner, the standalone madvise test, and the node compile-cache scripts that go through the changed__bun_jsc_generate_cached_bytecodehelper. Checked manually withBUN_JSC_verboseDiskCache=1that ESM--compilebuilds, non-compile--bytecodeoutput and non-ASCII (Binary) modules behave exactly as before.Background
bun build --bytecoderuns JSC's parser and bytecode generator at build time and serializes the result (CachedBytecode). At runtime Bun hands that blob to JSC throughSourceProvider::cachedBytecode(), and JSC'sCodeCachedecodes it instead of parsing, provided theSourceCodeKeystored inside the blob matches the one computed for the runtime source.SourceCodeKey: JSC's cache key for a piece of source. It is built fromprovider->hash()(xor'd with a few parse-mode flags) and compared by hash, length, flags, name and origin host.provider->hash()is normallyStringImpl::hash(), a lazily computed, cached hash of every character in the string; this PR makes Bun's provider return the value computed at build time instead of computing it again.Zig::SourceProvider: Bun'sJSC::SourceProvidersubclass. It wraps theResolvedSourcethat Rust fills in for a module (source string, optional bytecode, optionalmodule_info, origin path used for the bytecode key).bun build --compileappends to the bun executable. Per module it stores name, contents, optional bytecode,module_infoand the origin path in a#[repr(C)]record (CompiledModuleGraphFile); at runtime the OS maps it straight from the executable andFile::to_wtf_string()wraps the Latin1 contents zero-copy, so whatever touches the string decides which pages get faulted in.hint_source_pages_dont_need()(the madvise after entry evaluation) is left as is; with this change there is much less for it to drop.module_info: the module's import/export table, precomputed at build time so JSC can skip the analysis parse of ESM modules too. Debug builds still run that parse to cross-check it, which is why the test uses CommonJS.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/bundler_compile.test.ts