Compiled executables: alias embedded bytecode instead of copying it; smaller, page-friendly bytecode (WebKit#494) - #40201
Conversation
…aliases it instead of copying Pins WebKit to oven-sh/WebKit#494. The executable's bytecode section is mapped for the life of the process, so decoded instruction streams and expression info can point into it; the same PR lays the cache out so decoding pages in only what it reads and shrinks it ~36%. Adds a test that a --compile --bytecode executable's anonymous memory drops when the aliasing is on versus off in the same binary.
|
Updated 3:51 AM PT - Aug 24th, 2026
✅ @Jarred-Sumner, your commit 22432d8aab781ca747141cbd3be7944ee13a57eb passed in 🧪 To try this PR locally: bunx bun-pr 40201That installs a local version of the PR into your bun-40201 --bun |
… persistent regardless of isBuiltin; pin WebKit#494 head; test captures stderr
… from bytecode never reads the source text JSC keys the bytecode lookup on SourceCodeKey, whose hash is StringImpl::hash() of the whole module source; computing it faulted every source page back in right after the standalone graph madvised them out. The graph now carries the hash per module (Flags::HAS_SOURCE_HASHES, a u32 array after the module table; older payloads simply lack it) and Zig::SourceProvider returns it. Test: a compiled executable that imports a 4,000-function module without calling anything keeps under a quarter of its payload mapping resident (was ~95%).
… absolute 20 MB (the new cache format halves the bytecode)
…in the failure message
…apping depends on the agent's page-cache folio policy (dirty folios mapped wholesale on the ASAN agent), not on what the process reads; the anonymous-memory test covers the copy path
…ompact format, checksums)
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Beyond the inline nit, I also checked: bytecodeIsEmbeddedInExecutable is captured before the needsDeref = false mutation, and the only site that populates bytecode_cache with source_code_needs_deref: false is the standalone-graph path in jsc_hooks.rs (process-lifetime section bytes), so setPayloadIsPersistent() can't fire on a heap-owned buffer. The build-time hash uses output_file.value gated on module.encoding == Latin1, which is the same byte view to_wtf_string() hands JSC, and HAS_SOURCE_HASHES gates the read side so older graphs skip it.
Extended reasoning...
Verified the persistent-payload gate and the source-hash plumbing don't have lifetime or encoding mismatches; the reported finding is a stray comment left by e61ee8f. This is a coordinated WebKit bump with a bytecode-format change and new aliasing semantics, so a human should still look at it.
…ecutable section, retired compile-cache blob) instead of inferring it from needsDeref
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
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 (3)
Included review availability: 0 reviews are currently available. Your included PR review attempts over the past 7 days set your current allowance at 1 review per hour. WalkthroughChangesThe bundler generates bytecode for reachable internal modules and emits it in executable builds. Standalone graphs serialize source hashes and builtin bytecode. Runtime loading uses embedded bytecode when available and falls back to source compilation. Tests cover execution and memory behavior. Builtin bytecode pipeline
WebKit autobuild update
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
Comment |
There was a problem hiding this comment.
Commit 2384d71 addresses the last inline finding — all three node_compile_cache::fetch() sites and the standalone-graph path now set bytecode_cache_is_static. This run found no further issues, but given the change is memory-safety-adjacent (aliasing decoded instruction streams into a buffer that must outlive them) and is coupled to an unmerged WebKit bump, a human look is still warranted before landing.
Checked this run:
ResolvedSourcelayout matches betweenResolvedSource.rsandheaders-handwritten.h(new fields appended, same order/types).- Every site that sets
bytecode_cache_is_static: truepoints at process-lifetime memory (executable section orRETIRED_BLOBScompile-cache blob); default is the safefalse. HAS_SOURCE_HASHESread path infrom_bytesis gated on the flag, so old binaries without the hash table still parse.- Sidecar
.jscbytecode paths were checked and correctly leave the flagfalse(those buffers are freed viadestructorPtr).
Extended reasoning...
Overview
This PR wires Bun to WebKit#494's persistent-payload bytecode cache: adds bytecode_cache_is_static and source_code_hash to the ResolvedSource FFI struct (Rust + C++ header), calls CachedBytecode::setPayloadIsPersistent() in ZigSourceProvider::create when the flag is set, exports Bun__WTFStringHashLatin1 so bun build --compile can precompute the source hash, extends the standalone-module-graph binary format with a per-module hash table behind a new HAS_SOURCE_HASHES flag bit, sets the new static flag at four jsc_hooks.rs sites, bumps WEBKIT_VERSION to a #494 preview build, and adds a Linux-only anonymous-memory delta test.
Over three prior review rounds I flagged (1) a stray comment left after the dropped residency test, (2) the !needsDeref heuristic being narrower than its name/comment, and (3) two sibling compile-cache sites missing the flag. All three were addressed (commits 2de1d82, 4861e28, 2384d71). This run's finder pass raised only the sidecar .jsc path as a candidate, which verifiers refuted — those buffers are heap-owned with a real destructor, so false is correct there.
Security risks
None identified. The change is a performance optimization on trusted data (bytecode Bun itself generated at build time, or compile-cache blobs Bun itself wrote). No new user-controlled input reaches a parser or allocation.
Level of scrutiny
High. setPayloadIsPersistent() makes JSC alias decoded InstructionStream/ExpressionInfo directly into the payload rather than copying — a wrong true is a use-after-free of executable bytecode. The flag defaults to false (safe) and every true site was traced to process-lifetime memory, but this is exactly the class of change REVIEW.md's memory-safety section calls out for careful ownership audit. The FFI struct-layout change and the module-graph binary-format change are additional ABI surfaces where a mismatch fails silently.
Other factors
The PR is explicitly a draft pending the WebKit#494 merge (WEBKIT_VERSION currently points at a preview artifact; the description says to swap for the merge SHA before landing). The new test depends on a JSC option (BUN_JSC_useBorrowedBytecodeFromCache) that only exists in that WebKit build. A maintainer should confirm the WebKit side has landed and the version pin is finalized before merging. The change itself looks correct after the three fix-up commits, but the coupling to an external PR and the memory-aliasing nature put it outside what should merge without human sign-off.
…ring dedup, steps-backed metadata tables)
There was a problem hiding this comment.
I reviewed this revision and found no new issues — the three earlier findings (orphaned test comment, needsDeref-inferred predicate, missing bytecode_cache_is_static on the two sibling compile-cache paths) are all addressed. Because this bumps the WebKit engine version and gates JSC's alias-instead-of-copy path on a new process-lifetime invariant, a human look is still worthwhile before landing.
What was reviewed:
ResolvedSourceFFI struct: newsource_code_hash/bytecode_cache_is_staticfields appended in the same order/types on both the Rust (ResolvedSource.rs) and C++ (headers-handwritten.h) sides;Defaultsets both to safe fall-throughs.- All four
bytecode_cache_is_static: truesites traced to process-lifetime storage (executable section;RETIRED_BLOBSfor the threenode_compile_cache::fetchpaths). to_bytes/from_bytes: hash table written contiguously after the module table (capacity reservation usesoutput_files.len() ≥ modules.len();debug_assert_eq!on offset), read back viaslice_togated onHAS_SOURCE_HASHES;modules/module_filesare pushed in lockstep so the.zip()is aligned.m_hashalready defaults to 0 andhash()already falls through on 0, so writingsource_code_hash = 0is a no-op;Bun__WTFStringHashLatin1matches whatStringImpl::hash()computes for an 8-bit string.
Extended reasoning...
Overview
Bun-side integration for oven-sh/WebKit#494: bumps WEBKIT_VERSION, adds ResolvedSource::{source_code_hash, bytecode_cache_is_static} (Rust + C++ mirror), calls CachedBytecode::setPayloadIsPersistent() when the flag is set so JSC aliases instruction streams / expression info into the mapped payload instead of copying, exposes Bun__WTFStringHashLatin1 for build-time source hashing, extends the standalone-module-graph format with a per-module source-hash table behind a new HAS_SOURCE_HASHES flag, wires the flag/hash through the standalone-executable load path in jsc_hooks.rs, sets the static flag on all three NODE_COMPILE_CACHE fetch paths, and adds a Linux-only anonymous-memory delta test.
Security risks
None identified. Inputs are trusted (the process's own executable section and compile-cache blobs it produced). The memory-safety concern is lifetime, not adversarial input: setPayloadIsPersistent() promises JSC the payload outlives every decoded instruction stream. I traced each of the four true sites — the standalone-graph blob is a section of the running executable; the three compile-cache blobs are owned by NodeCompileCache's entry map and moved to RETIRED_BLOBS (never freed) on displacement. Default is false, so any un-audited construction site copies as before.
Level of scrutiny
High. This is (a) a JS-engine dependency bump whose payload-format changes I cannot review here, (b) a change that hands JSC a "you may alias this forever" promise where a wrong true is a latent UAF, and (c) a serialized-format extension to the compiled-executable graph. None of it is mechanical.
Other factors
Three earlier review rounds each produced a finding that was addressed (orphaned comment removed; implicit !needsDeref predicate replaced by an explicit field; the two sibling compile-cache paths now set the flag). The new test is a self-controlled A/B on the same binary via BUN_JSC_useBorrowedBytecodeFromCache=0, which sidesteps the ASAN/debug RSS-threshold flakiness that killed the earlier residency test. The PR description notes the WebKit hash may still need swapping for the merged SHA — worth confirming before merge.
### Problem - Bun's WebKit pin `aea1f010b6` is 412 upstream commits behind `8c4fd56347`, 90 of them in JavaScriptCore, WTF or bmalloc. oven-sh/WebKit#503 merges that range into the fork. - Upstream removed the `@newPromiseCapability` private builtin (`38027ff0ec`). Six call sites in Bun's bundled modules use it, so `node:events`, `node:util`, `node:dgram` and the HTTP server fail to load (`Private symbol not found: newPromiseCapability`). - #40263 and oven-sh/WebKit#501 were a parallel attempt at `55d9d9007f`, one WebCore-only commit ahead. Both are closed in favor of this pair. ### Fix - oven-sh/WebKit#503 is merged. `WEBKIT_VERSION` is `cb61607f1a4bae79d7701965062634dee9efb349`, its merge commit on the fork's main (release `autobuild-cb61607f1a4bae79d7701965062634dee9efb349`, 42 tarballs). That commit is the preview build this PR was tested against (`d2654c3b`) plus oven-sh/WebKit `a0a80b2276` (an optional depth bound on `recursivelyGenerateUnlinkedCodeBlockForProgram/ForModuleProgram`). - The six call sites create their promise with `$newPromise()` and settle it with `$resolvePromise` / `$rejectPromise`, or the `...WithFirstResolvingFunctionCallCheck` variants where a second settle is possible. `builtins.d.ts` follows. - `EncodeURIComponent.cpp` includes `<wtf/HexNumber.h>` itself (upstream `314133b7a6` no longer does). - Verified: `test/js/bun/jsc/webkit-upgrade-8c4fd56347.test.ts` pins four JavaScript-visible engine changes that fail at the current pin. The events, util and dgram tests cover the ported settlement paths. ### Background - Bun links a prebuilt JavaScriptCore from oven-sh/WebKit releases. `scripts/build/deps/webkit.ts` names the release tag. - Built-in modules (`src/js/`) go through JavaScriptCore's builtin compiler. A `$name` call becomes the private name `@name`, which has to exist in the engine. - `$newPromise` creates a pending promise. `$resolvePromise` / `$rejectPromise` settle it and require it to be pending. The `...WithFirstResolvingFunctionCallCheck` variants ignore calls after the first, like a Promise executor's functions. <details><summary>Notes</summary> - Duplicate resolution: oven-sh/WebKit#501 and #503 have the same structure (main at `62f427b86f`, then #488's head `d0fae3b3c9`, then upstream/main) and the same `CachedTypes.cpp` resolution (the two files differ in comments and an unused alias). WTF and bmalloc are identical. The only upstream difference is WebCore's `55d9d9007f` (`MediaElementAudioSourceNode` use-after-free), which the JSCOnly port does not compile. #503 was kept because this PR's CI run was green (Build #104490). The test file `webkit-upgrade-8c4fd56347.test.ts` is carried over from #40263. - oven-sh/WebKit#488 (upstream `baf4a9a7ec0b`) stopped merging after the fork's bytecode cache rework (#490, #493, #494, #497). The per-commit review of the upstream range (API and ABI changes, behavior changes, performance) and the conflict resolutions are in oven-sh/WebKit#503. The new conflict in this round is `CachedTypes.cpp`: the fork's new code block record layout against upstream moving the global-only fields (`features`, `lineCount`, source URL directives) to `UnlinkedGlobalCodeBlock` and deleting `m_jumpTargets`. - The ported call sites (the changes of #40054, carried over): `node:events` (`once`), `node:util` (`aborted`), `node:dgram` (`Symbol.asyncDispose`) and the HTTP server (CONNECT, Upgrade, the per-request completion promise). `builtins.d.ts` declares `$newPromise`, `$resolvePromiseWithFirstResolvingFunctionCallCheck` and `$rejectPromiseWithFirstResolvingFunctionCallCheck` and drops `$newPromiseCapability`. `@newPromise` is a bytecode intrinsic and `@resolvePromise` / `@rejectPromise` are link-time constants, so they exist in every engine build. `util.aborted` registers and unregisters its `FinalizationRegistry` entry with the same token (the promise). - Behavior changes in the upstream range that are visible from JavaScript: `Promise.try` follows the updated spec (`PromiseResolve` instead of `NewPromiseCapability`); the module map no longer caches fetch failures, so a second `import()` of a specifier whose load failed re-runs Bun's module loader instead of rejecting with the cached error; `Uint8Array.prototype.setFromBase64` on a zero-length target returns `{ read: 0, written: 0 }` without validating the input; `WebAssembly.Module.imports()/exports()` descriptors drop the non-standard `type` field; re-exported imported Wasm globals and tags keep object identity; a DFG `++`/`--` on an `int32` that overflows with an unused result now deoptimizes instead of wrapping (`7711916200`). The first, third, fourth and last of these are pinned by `test/js/bun/jsc/webkit-upgrade-8c4fd56347.test.ts`. - Performance changes of note: `SymbolTableEntry` no longer allocates a `WatchpointSet` per watched variable until the DFG watches it (`cea233cede`); `Object.assign` with several sources clones the first one through `objectCloneFast` (`96ca975b2a`); `JSON.parse` allocates arrays once at their final size; `TypedArray.prototype.sort()` without a comparator uses a radix sort for 2/4/8-byte element types; `Map`/`Set` `forEach` is inlined in the DFG and FTL; `RegExp` cells shrink from 96 to 80 bytes; `UnlinkedFunctionCodeBlock` shrinks from 216 to 192 bytes. - `src/jsc/bindings/NodeVMSyntheticModule.cpp` calls `SymbolTable::set(NoLockingNecessary, ...)`. After `cea233cede` only the locked overload exists. `NoLockingNecessary` converts to a `ConcurrentJSLocker`, so the call compiles unchanged. - The upstream change to Linux thread scheduling (per-QOS `sched_setattr` on every WTF thread, `SCHED_BATCH` compiler threads on hosts with 4 or fewer cores) is gated off for Bun in the fork: Bun's threads keep inheriting the process scheduling attributes. - Suites run on a local debug + ASAN build against the merged WebKit (`bun run build:local`): `test/js/bun/jsc`, `bun/jsc-stress` (116/116), `node/events`, `node/util`, `node/dgram`, `node/vm`, `node/module`, `bun/resolve`, `node/worker_threads`, `bun/wasm`, `web/url`, `web/atomics`, `node/http/node-http-connect`, `node/async_hooks`, `node/string_decoder`, `bundler/bundler_compile`, `bundler/bun-build-api`: 3,548 pass. The failures are 5 s timeouts under debug + ASAN, this machine's IPv6 multicast `ENODEV`, and one test that fails the same way at the current pin. `bun build --bytecode` output from that build loads and runs. A debug + ASAN build against the `autobuild-preview-pr-503-311eab61` prebuilt runs `test/js/bun/jsc/webkit-upgrade-8c4fd56347.test.ts`, `test/js/bun/jsc/webkit-upgrade-3722912f.test.ts`, `node/events/event-emitter.test.ts` and `node/util/test-aborted.test.ts`: 106 pass. - Every push to oven-sh/WebKit#503 produces a new preview tag (`autobuild-preview-pr-503-<first 8 of the head sha>`), and this PR's `WEBKIT_VERSION` follows it. CI lanes that fetch the prebuilt fail on the download until that tag's Actions run has published the release. - Rebase over #40201: Bun main moved its pin to the fork's `c148a12dd82b` and calls the bytecode APIs that release added (`EncoderStringTable`, persistent payloads). The `311eab61` preview predates them, so the branch could not rebase until oven-sh/WebKit#503 merged the fork's main (head `d2654c3b`, 0 commits behind). The rebase itself conflicted only on the `WEBKIT_VERSION` line. A debug + ASAN build against `autobuild-preview-pr-503-d2654c3b` passes `webkit-upgrade-8c4fd56347.test.ts`, `node/events/event-emitter.test.ts`, `node/util/test-aborted.test.ts`, `node/dgram` (except the IPv6 multicast `ENODEV` of this machine), `node/http/node-http-connect.test.ts`, `web/atomics`, `web/url`, `node/string_decoder` and `test/js/bun/jsc`. The compiled-executable bytecode paths of #40201 work against it: the aliasing run keeps 12 MB of instruction streams out of anonymous memory and 45 internal modules load from embedded bytecode. The failures on this machine are the DOMJIT hot loops and two `bun-build-compile` tests that exceed their timeouts under debug + ASAN (the compile alone takes 5 to 47 s here), and the nested `node-http-connect.node.mts` run that takes 5.0 s against a 5 s limit. - The `$newPromiseCapability` call in `src/node-fallbacks/events.js` (the browser polyfill, not a JSC builtin) is a pre-existing bug and was reported separately by #40054. </details> <!-- robobun:evidence:begin --> --- **no test proof** · iteration 2 · platform-specific test(s) that do not run on this machine, deferring to CI, which covers all platforms: test/js/node/dgram/node-dgram.test.js <!-- robobun:evidence:end -->
### What does this PR do? Reverts the numbered `./_N.js` chunk naming that #40201 introduced for chunks inside a compiled executable. Chunks use the same `chunk-<hash>.js` names as a normal build again. ### How did you verify your code works? Built an executable with two entry points and `splitting: true`; the embedded chunk names are `chunk-<hash>.js`. Ran `test/bundler/bun-build-compile.test.ts` locally; the only failures are the local ASAN `asan-dyld-shim.dylib` load errors when the compiled binary runs, which are unrelated to this change.
2 participants
What does this PR do?
bun build --compile --bytecodeexecutables get a smaller, faster bytecode section that JSC reads in place instead of copying.Bun side:
ZigSourceProvidermarks standalone-executable bytecode persistent so decoded instruction streams and expression info alias the mmapped section instead of heap copies.EncoderStringTableshared by every chunk and every embedded internal module: each ≥4-char non-symbol string is a 4-byte ordinal in every payload; the characters are written once as a graph section (Flags::HAS_BYTECODE_STRING_TABLE).DecoderStringTableonJSVMClientDatareads it with a demand-zero atom slot per ordinal.Flags::HAS_SOURCE_HASHES: each module'sSourceCodeKeyhash is baked in so a bytecode launch never pages in source text just to hash it.node:*,bun:*,internal:*) the bundle imports and everything they eagerlyrequire();InternalModuleRegistrydecodes instead of parsing../_12.js) instead ofchunk-<hash>.js.WebKit side (oven-sh/WebKit
c148a12dd82b):VM, not perDecoder.EncoderStringTable/DecoderStringTableandexternalStringTag;encodeBuiltinFunctiontakes the same table.Measured
57 npm packages (typescript, three, echarts, mathjs, tfjs, aws-sdk, firebase, antd, mui, prettier, babel, …) bundled with
--minifyto 36 MB,--compile --bytecode, importing 38 of them and running every module initializer. Medians; footprint =Bun.unsafe.memoryFootprint().macOS arm64, 1.4.0 vs this PR:
--bytecodeLinux x64, 1.4.0 vs this PR:
--bytecodemacOS
phys_footprintcounts anonymous memory only; the file-backed win is separate —vmmapshows the mmapped__BUNsection resident at 129 MB on main vs 41 MB on this PR (0 dirty in both).Where the startup time went
samply main-thread profile of the esm+split executable, main vs this PR (absolute samples at 8 kHz):
unlinkedCodeBlockFor(decode + fallback parse)CachedVector<CachedFunctionExecutable>Lexer::lex(parse fallback)CachedPtr<CachedInstructionStream>(copy)regionChecksumMatchesVarintReader::u32¹ skipped for persistent payloads as of this PR; before the skip it was 21.
How did you verify your code works?
test/bundler/bundler_compile.test.ts,test/bundler/bundler_compile_splitting.test.ts— every--compile --bytecodematrix cell.--compile --bytecode --splittingexecutable with 4,000 functions reports itsAnonymous:fromsmaps_rollup; the same binary withBUN_JSC_useBorrowedBytecodeFromCache=0is the control and must be several MB higher.JSTests/stressthrough the disk cache with the persistent-payload switch on and off, unchanged vs main.