node:v8: heap statistics shape, cachedDataVersionTag, ERR_NOT_BUILDING_SNAPSHOT (+4 tests) - #34841
Closed
cirospaciari wants to merge 1 commit into
Closed
node:v8: heap statistics shape, cachedDataVersionTag, ERR_NOT_BUILDING_SNAPSHOT (+4 tests)#34841cirospaciari wants to merge 1 commit into
cirospaciari wants to merge 1 commit into
Conversation
…G_SNAPSHOT getHeapStatistics gains total_allocated_bytes so the key set matches node's exactly. getHeapSpaceStatistics and getHeapCodeStatistics report JSC's real totals in node's shape instead of throwing: JSC has one undivided heap, so its used and capacity numbers appear under old_space and the other twelve V8 space names carry zeros; the names are required by shape-dependent consumers, the numbers are real. getCppHeapStatistics validates its argument with node's message and returns the empty statistics node itself reports when cppgc holds nothing. setFlagsFromString validates and records the flags instead of throwing, and cachedDataVersionTag derives a stable uint32 from the runtime version plus the recorded flags, so the tag changes across a setFlagsFromString call as node's does. startupSnapshot callbacks throw node's ERR_NOT_BUILDING_SNAPSHOT. writeHeapSnapshot path handling is deliberately untouched: PR #34084 already fixes it. queryObjects is not implemented; it needs real heap iteration by prototype chain, which JSC does not expose. Adds test-v8-stats, test-v8-version-tag, test-v8-startup-snapshot-api and test-cppheap-stats from Node v26.3.0, verbatim.
Collaborator
|
Updated 1:18 PM PT - Jul 20th, 2026
❌ @cirospaciari, your commit 5442bd9 has 3 failures in
🧪 To try this PR locally: bunx bun-pr 34841That installs a local version of the PR into your bun-34841 --bun |
Contributor
|
Found 2 issues this PR may fix:
🤖 Generated with Claude Code |
Contributor
|
This PR may be a duplicate of:
🤖 Generated with Claude Code |
Member
Author
|
Folded into #34660 (branch merged) as part of PR consolidation — same commits and tests, fewer PRs. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Four small
node:v8API gaps, each verified against the node v26.3.0 binary before implementation.getHeapStatistics()total_allocated_bytes= JSC heapCapacity, a real number)getHeapSpaceStatistics()/getHeapCodeStatistics()getCppHeapStatistics()ERR_INVALID_ARG_VALUEmessage), empty statistics — node itself reports empty when cppgc holds nothingcachedDataVersionTag()setFlagsFromString, which the test assertssetFlagsFromString()startupSnapshotcallbacksERR_NOT_BUILDING_SNAPSHOTThe one judgment call:
getHeapSpaceStatisticsmust return exactly V8's 13 space names (the test and shape-dependent consumers require them), but JSC has one undivided heap. JSC's real used/capacity numbers are reported underold_space; the other twelve names carry zeros. The names are shape, the numbers are real — nothing is fabricated.Deliberately skipped:
writeHeapSnapshotBuffer/URL paths (PR #34084 already fixes exactly this — duplicating would conflict),queryObjects(needs real heap iteration by prototype chain, which JSC doesn't expose — an honest implementation is a new native heap-walk binding),test-v8-flags(%IsSminatives syntax) andtest-v8-serdes(native V8 Serializer classes).No overlap with #34550 (GCProfiler untouched) or #34523 (its
test-v8-flag-type-check.jspasses against this implementation; one trivial import-line adjacency whenever the second lands).Verification
All 4 tests byte-identical to upstream, pass on node (ceiling), fail on unpatched bun (control), pass 3× each on this build with the CI invocation. 12 tamper mutations (3 per file), every one fails. Regressions:
test/js/node/v8/48/48,stubs.test.js378/378,v8-heap-snapshot.test.ts7/7, all pre-existing vendored v8/worker heap tests green. tsc clean in touched files.