Skip to content

heap-snapshot: keep node ids stable across v8 snapshots - #34085

Open
robobun wants to merge 1 commit into
mainfrom
farm/27dc1b63/heap-snapshot-stable-ids
Open

robobun wants to merge 1 commit into
mainfrom
farm/27dc1b63/heap-snapshot-stable-ids

Conversation

@robobun

@robobun robobun commented Jul 13, 2026 •

Copy link
Copy Markdown
Collaborator

What

Heap-snapshot node ids are not stable across snapshots: the same live object gets a fresh id on every v8.writeHeapSnapshot() / Bun.generateHeapSnapshot("v8") call. This breaks every heap-diff tool that keys on the V8 id-stability contract (DevTools Comparison view, memlab): between any two snapshots the entire heap appears freed-and-reallocated.

globalThis.__ID_STABILITY_TAG__ = { marker: "id-stability-probe" };
const id1 = findTagId(Bun.generateHeapSnapshot("v8"));
globalThis.__pad__ = new Array(50000).fill("pad");
const id2 = findTagId(Bun.generateHeapSnapshot("v8"));
// node:  77759 77759   (same id, object survived)
// bun:   7436  16139   (fresh id every snapshot)

Cause

functionGenerateHeapSnapshot (and the sibling v8 entry points in BunHeapProfiler.cpp / JSWorker.cpp) called heapProfiler.clearSnapshots() before constructing BunV8HeapSnapshotBuilder. That nulls m_previousSnapshot, so the id-reuse lookup in analyzeNodeInternal never hits and every cell is assigned a fresh HeapSnapshotBuilder::getNextObjectIdentifier():

// BunV8HeapSnapshotBuilder.cpp
, m_previousSnapshot(profiler.mostRecentSnapshot())   // always null after clearSnapshots()
...
if (auto existing = m_previousSnapshot ? m_previousSnapshot->nodeForCell(cell) : std::nullopt)
    identifier = existing->identifier;   // never taken

Fix

Drop the three clearSnapshots() calls so the builder can find surviving cells in the previous snapshot. Heap::removeDeadHeapSnapshotNodes() already prunes dead entries from the retained chain on every GC, so a freed-and-reused address cannot alias an old id. The retained HeapSnapshot only stores cells that were new in that snapshot (delta storage), so repeated snapshots do not grow unboundedly.

Verification

Added id-stability mode to the v8 heap-snapshot fixture: tags an object on globalThis, takes two snapshots (string and arraybuffer paths), and asserts the tagged object's id is identical in both.

Before: id1=888 id2=6824 stable=false
After: id1=1234 id2=1234 stable=true


[review] gate passed · iteration 0 · 5 files touched

fails on main (without fix)
ASAN without fix: 1 FAILED
$ BUN_DEBUG_QUIET_LOGS=1 bun scripts/build.ts --profile=debug --quiet test "--reporter=junit" "--reporter-outfile=/tmp/mechgate.xml" "test/js/bun/util/v8-heap-snapshot.test.ts"
info: syncing channel updates for nightly-2026-05-06-x86_64-unknown-linux-gnu
info: latest update on 2026-05-06 for version 1.97.0-nightly (e95e73209 2026-05-05)
info: component rust-src is up to date
info: checking for self-update (current version: 1.29.0)
bun test v1.4.0 (a7aa5f182)

test/js/bun/util/v8-heap-snapshot.test.ts:
(pass) v8 heap snapshot [3541.86ms]
(pass) v8 heap snapshot arraybuffer [3019.23ms]
(pass) v8 heap snapshot arraybuffer matches string output [1396.87ms]
(pass) v8.getHeapSnapshot() [3203.53ms]
(pass) v8.writeHeapSnapshot() [4134.46ms]
(pass) v8.writeHeapSnapshot() with path [3096.14ms]
89 |   expect(await runFixture("write-path", { args: [path] })).toEqual({ returnedPath: path, ...structure });
90 | });
91 | 
92 | test("v8 heap snapshot node ids are stable across snapshots", async () => {
93 |   const { id1, id2, found, stable } = await runFixture("id-stability");
94 |   expect({ found, stable, id1, id2 }).toEqual({ found: true, stable: true, id1, id2: id1 });
       
... (truncated)

release without fix: 2 FAILED
bun test v1.4.0-canary.1 (1498d7b77)

test/js/bun/util/v8-heap-snapshot.test.ts:
(pass) v8 heap snapshot [67.61ms]
(pass) v8 heap snapshot arraybuffer [66.06ms]
(pass) v8 heap snapshot arraybuffer matches string output [35.76ms]
(pass) v8.getHeapSnapshot() [53.79ms]
(pass) v8.writeHeapSnapshot() [59.88ms]
(pass) v8.writeHeapSnapshot() with path [50.28ms]
89 |   expect(await runFixture("write-path", { args: [path] })).toEqual({ returnedPath: path, ...structure });
90 | });
91 | 
92 | test("v8 heap snapshot node ids are stable across snapshots", async () => {
93 |   const { id1, id2, found, stable } = await runFixture("id-stability");
94 |   expect({ found, stable, id1, id2 }).toEqual({ found: true, stable: true, id1, id2: id1 });
                                           ^
error: expect(received).toEqual(expected)

  {
    "found": true,
    "id1": 843,
-   "id2": 843,
-   "stable": true,
+   "id2": 6822,
+   "stable": false,
  }

- Expected  - 2
+ Received  + 2

      at <anonymous> (/workspace/bun/test/js/bun/util/v8-heap-snapshot.test.ts:94:39)
(fail) v8 heap snapshot node ids are stable across snapshots [38.34ms]
 96 | 
 97 | test("v8 heap snapshot labels Web St
... (truncated)
passes on PR (with fix)
ASAN with fix: all passed
$ BUN_DEBUG_QUIET_LOGS=1 bun scripts/build.ts --profile=debug --quiet test "--reporter=junit" "--reporter-outfile=/tmp/mechgate.xml" "test/js/bun/util/v8-heap-snapshot.test.ts"
info: syncing channel updates for nightly-2026-05-06-x86_64-unknown-linux-gnu
info: latest update on 2026-05-06 for version 1.97.0-nightly (e95e73209 2026-05-05)
info: component rust-src is up to date
info: checking for self-update (current version: 1.29.0)
bun test v1.4.0 (a7aa5f182)

test/js/bun/util/v8-heap-snapshot.test.ts:
(pass) v8 heap snapshot [3410.54ms]
(pass) v8 heap snapshot arraybuffer [2896.17ms]
(pass) v8 heap snapshot arraybuffer matches string output [1394.38ms]
(pass) v8.getHeapSnapshot() [3140.28ms]
(pass) v8.writeHeapSnapshot() [4022.11ms]
(pass) v8.writeHeapSnapshot() with path [3059.05ms]
(pass) v8 heap snapshot node ids are stable across snapshots [1479.27ms]
(pass) v8 heap snapshot labels Web Streams internal edges [1275.10ms]

 8 pass
 0 fail
 19 expect() calls
Ran 8 tests across 1 file. [22.74s]
__F:0:S:0

release with fix: all passed
$ bun scripts/build.ts --profile=release
info: syncing channel updates for nightly-2026-05-06-x86_64-unknown-linux-gnu
info: latest update on 2026-05-06 for version 1.97.0-nightly (e95e73209 2026-05-05)
info: component rust-src is up to date
info: checking for self-update (current version: 1.29.0)
[configured] bun-profile → bun (stripped) in 696ms (unchanged)
ninja: Entering directory `/workspace/bun/build/release'
[1/9] gen cpp.rs (cppbind)
[1/9] cargo bun_bin → libbun_rust.a (--target x86_64-unknown-linux-gnu)
info: syncing channel updates for nightly-2026-05-06-x86_64-unknown-linux-gnu
info: latest update on 2026-05-06 for version 1.97.0-nightly (e95e73209 2026-05-05)
info: component rust-src is up to date
info: component rust-std is up to date

info: checking for self-update (current version: 1.29.0)
  nightly-2026-05-06-x86_64-unknown-linux-gnu unchanged - rustc 1.97.0-nightly (e95e73209 2026-05-05)

�[1m�[92m    Blocking�[0m waiting for file lock on build directory
�[1m�[92m    Finished�[0m `release` profile [optimized + debuginfo] target(s) in 1m 12s
[2/9] gen BunObject.lut.h
Generating /workspace/bun/build/release/codegen/BunObject.lut.h from /workspace/
... (truncated)
diff hotspot
src/jsc/bindings/BunHeapProfiler.cpp         |  1 -
 src/jsc/bindings/BunObject.cpp               |  1 -
 src/jsc/bindings/webcore/JSWorker.cpp        |  1 -
 test/js/bun/util/v8-heap-snapshot-fixture.ts | 45 ++++++++++++++++++++++++++++
 test/js/bun/util/v8-heap-snapshot.test.ts    |  5 ++++
 5 files changed, 50 insertions(+), 3 deletions(-)

gate history · 1 passed · 0 rejected · iteration 0

evidence per changed file
file                                          reads  edits  tests
src/jsc/bindings/BunHeapProfiler.cpp              1      1      0
src/jsc/bindings/BunObject.cpp                    1      1      0
src/jsc/bindings/webcore/JSWorker.cpp             1      1      0
test/js/bun/util/v8-heap-snapshot-fixture.ts      1      2      0
test/js/bun/util/v8-heap-snapshot.test.ts         1      2      0

Bun.generateHeapSnapshot("v8"), v8.writeHeapSnapshot(), v8.getHeapSnapshot()
and Worker#getHeapSnapshot() all called heapProfiler.clearSnapshots() before
constructing BunV8HeapSnapshotBuilder. That nulls m_previousSnapshot and
defeats the id-reuse logic the builder already implements: every surviving
object got a fresh id on every snapshot, so DevTools' Comparison view and
memlab saw the whole heap as freed-and-reallocated between any two snapshots.

Drop the clearSnapshots() calls so the builder can look up surviving cells in
the previous snapshot. Heap::removeDeadHeapSnapshotNodes() already prunes dead
entries from the retained chain on every GC.
@robobun

robobun commented Jul 13, 2026 •

Copy link
Copy Markdown
Collaborator Author
Updated 4:10 PM PT - Jul 13th, 2026

❌ @robobun, your commit a7aa5f1 has 1 failures in Build #72529 (All Failures):


🧪   To try this PR locally:

bunx bun-pr 34085

That installs a local version of the PR into your bun-34085 executable, so you can run:

bun-34085 --bun

@coderabbitai

coderabbitai Bot commented Jul 13, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 448f0659-3a52-4156-b8e3-782cd3d07ea5

📥 Commits

Reviewing files that changed from the base of the PR and between eb0d38b and a7aa5f1.

📒 Files selected for processing (5)
  • src/jsc/bindings/BunHeapProfiler.cpp
  • src/jsc/bindings/BunObject.cpp
  • src/jsc/bindings/webcore/JSWorker.cpp
  • test/js/bun/util/v8-heap-snapshot-fixture.ts
  • test/js/bun/util/v8-heap-snapshot.test.ts
💤 Files with no reviewable changes (3)
  • src/jsc/bindings/BunHeapProfiler.cpp
  • src/jsc/bindings/BunObject.cpp
  • src/jsc/bindings/webcore/JSWorker.cpp

Walkthrough

Heap snapshot entry points now preserve existing profiler snapshots while ensuring profiler initialization. A new fixture mode and test verify that V8 node IDs remain stable across multiple snapshots.

Changes

Heap snapshot stability

Layer / File(s) Summary
Preserve profiler state during snapshot generation
src/jsc/bindings/BunHeapProfiler.cpp, src/jsc/bindings/BunObject.cpp, src/jsc/bindings/webcore/JSWorker.cpp
V8, Bun, and worker snapshot paths ensure heap profiler initialization and stop clearing existing snapshots before generation.
Validate stable snapshot node IDs
test/js/bun/util/v8-heap-snapshot-fixture.ts, test/js/bun/util/v8-heap-snapshot.test.ts
The fixture compares a tagged global object's node ID across two snapshots, and the test asserts that the ID remains stable.
🚥 Pre-merge checks | ✅ 4
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarizes the main change: stabilizing V8 heap-snapshot node IDs across snapshots.
Description check ✅ Passed The description covers what changed and how it was verified, even though the section headings differ from the template.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

Comment @coderabbitai help to get the list of available commands.

@robobun

robobun commented Jul 13, 2026 •

Copy link
Copy Markdown
Collaborator Author

CI build #72529: the new v8-heap-snapshot.test.ts id-stability test and all other heap-snapshot tests pass on every lane. All remaining failures are either pre-existing on main or tracked flakes, none in heap-snapshot code:

test status lane
test-worker-message-port-transfer-terminate.js pre-existing on main x64-asan
bake/deinitialization.test.ts tracked flake windows aarch64
bake/dev-and-prod.test.ts tracked flake windows x64-baseline
bun-jsc.test.ts (CPU sampling profiler) tracked flake x64-asan
spawn/spawn.test.ts tracked flake windows x64-baseline
test-repl-close.js tracked flake windows x64-baseline
cli/install/bun-run-dir.test.ts tracked flake
cli/run/no-orphans.test.ts tracked flake
net/net-mongodb-pattern-leak.test.ts tracked flake
test-https-timeout.js tracked flake

Ready for review.

@claude claude Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Beyond the inline note on the snapshot chain, I also checked the non-v8 Bun.generateHeapSnapshot() / "jsc" path in BunObject.cpp — removing clearSnapshots() there does not turn its output into a delta (JSC's HeapSnapshotBuilder::json() still emits the full node set), so the default format is unaffected.

Extended reasoning...

The three-line deletion is well-reasoned and the id-stability test covers both the string and arraybuffer v8 entry points. The main residual risk was that functionGenerateHeapSnapshot shares the removed clearSnapshots() call between the v8 and JSC-format branches, so the JSC HeapSnapshotBuilder now also sees a prior snapshot; verifiers confirmed its json() still serializes all live nodes rather than only the delta, so repeated Bun.generateHeapSnapshot() calls remain complete. Deferring rather than approving because the change alters HeapProfiler retention semantics (per-GC chain walk, inline nit) and lives in the JSC bindings — worth a maintainer glance.

Comment thread src/jsc/bindings/BunObject.cpp Resolved
@robobun

robobun commented Sep 12, 2026

Copy link
Copy Markdown
Collaborator Author

Cost data for the retained snapshot chain that this PR depends on. I measured it while tracing a CI flake. The Cost section of #29557 prices the chain in bytes only, not in GC time.

While a snapshot is registered in the VM's HeapProfiler, Heap::didFinishCollection calls removeDeadHeapSnapshotNodes after every collection, Eden included. That is MarkedSpace::forEachDeadCell over the whole heap, with one binary search in the snapshot per dead cell. The cost does not decay. It stops when clearSnapshots() runs.

Measured on main (release build, linux x64). One snapshot is taken, nothing else changes:

before after generateHeapSnapshot("v8") after generateHeapSnapshot()
median edenGC(), 300k live objects 0.27 ms 2.2 ms 2.5 ms
one expect() with BUN_GARBAGE_COLLECTOR_LEVEL=1 (one GC each), 1M live objects 0.83 to 0.88 ms 22.7 ms 29.9 ms

Main pays this today and gets no stable ids for it. The clear-before calls that this PR removes run before the next build, and nothing clears after a build.

This is the test/js/bun/css/color.test.ts timeout on darwin x64 (ansi-16 never emits a 256-color escape, "test timed out", 263 of the last 400 builds). shell-blocking-pipe.test.ts calls generateHeapSnapshot("v8") inside a bun test --parallel worker. The same worker later runs color.test.ts, which makes 2197 expect() calls in one test. CI sets BUN_GARBAGE_COLLECTOR_LEVEL=1, so that is 2197 collections, each with a walk over a 1 GB heap.

I reproduced it on an Intel Mac mini from the CI pool with the shard's own batch (769 files, --parallel=4, the runner's environment). The test took 232 s, 270 s and 234 s in three of four runs. The limit is 90 s. With the in-process snapshot removed from the batch it took 2.6 s, 4.5 s and 7.0 s. sample on the stuck worker shows the main thread in JSC::Heap::runEndPhase and JSC::MarkedBlock::Handle::isLive. #38940 moves that snapshot into a fixture process, so it also removes this flake.

Two changes in oven-sh/WebKit would keep the chain and remove most of the cost:

  1. Call removeDeadHeapSnapshotNodes only when the scope is CollectionScope::Full. The builder's own Full collection marked the cell of every node, and an Eden collection keeps mark bits. So no node can die in an Eden collection, and the Eden walk finds nothing.
  2. Make the Full prune walk the snapshot's own nodes and test each cell, not every dead cell in MarkedSpace.

I had a Bun-side change that clears the snapshots after each build. I did not open it, because it removes the chain that this PR and #29557 want to keep.

@robobun

robobun commented Sep 14, 2026

Copy link
Copy Markdown
Collaborator Author

The first of the two fork changes from my comment above is now a PR: oven-sh/WebKit#656. Heap::didFinishCollection prunes a retained snapshot only after a full collection.

Measured in the jsc shell (linux x64, 4.5 million live cells), with one snapshot held by the profiler:

fork main (3cf2a3dfd2) oven-sh/WebKit#656
edenGC(), no snapshot 0.42 ms 0.27 to 0.45 ms
edenGC(), snapshot held 22.0 to 24.3 ms 0.38 to 0.44 ms
fullGC(), snapshot held 94 ms 91 to 95 ms

It keeps the id chain, so it does not decide anything for this PR. It removes the eden cost that this PR would otherwise make permanent for every process that takes one V8 snapshot.

The test/js/bun/css/color.test.ts timeout on darwin x64 is now at 282 of the last 400 builds. #38940 (test only) still removes it on its own.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant