Repository navigation
BunV8HeapSnapshotBuilder: survive heaps past the Vector allocation cap #529
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -41,23 +41,17 @@ BunV8HeapSnapshotBuilder::BunV8HeapSnapshotBuilder(HeapProfiler& profiler) | |
| // calculateDistances BFS uses shortcut edges from root as user roots | ||
| // (distance=1) and assigns baseSystemDistance to anything reachable only | ||
| // through (GC roots). Synthetic IDs occupy 1..kSyntheticIdCount. | ||
| m_nodes.append({ | ||
| .id = 1, | ||
| .typeIndex = static_cast<unsigned>(V8NodeType::Synthetic), | ||
| m_nodes.append(Node { | ||
| .name = ""_s, | ||
| .selfSize = 0, | ||
| .edges = {}, | ||
| .traceLocation = std::nullopt, | ||
| .parentNodeId = std::nullopt, | ||
| }); | ||
| m_nodes.append({ | ||
| .id = 2, | ||
| .id = 1, | ||
| .typeIndex = static_cast<unsigned>(V8NodeType::Synthetic), | ||
| }); | ||
| m_nodes.append(Node { | ||
| .name = "(GC roots)"_s, | ||
| .selfSize = 0, | ||
| .edges = {}, | ||
| .traceLocation = std::nullopt, | ||
| .parentNodeId = kRootNodeIndex, | ||
| .id = 2, | ||
| .typeIndex = static_cast<unsigned>(V8NodeType::Synthetic), | ||
| }); | ||
|
|
||
| // Add empty string as first string (index 0) | ||
|
|
@@ -168,14 +162,11 @@ unsigned BunV8HeapSnapshotBuilder::analyzeNodeInternal(JSCell* cell) | |
| m_snapshot->appendNode(HeapSnapshotNode(cell, identifier)); | ||
| } | ||
|
|
||
| m_nodes.append({ | ||
| m_nodes.append(Node { | ||
| .cell = cell, | ||
| .selfSize = cell->estimatedSizeInBytes(m_profiler.vm()), | ||
| .id = identifier + kSyntheticIdCount, | ||
| .typeIndex = typeIndex, | ||
| .selfSize = cell->estimatedSizeInBytes(m_profiler.vm()), | ||
| .edges = {}, | ||
| .traceLocation = getTraceLocation(cell), | ||
| .parentNodeId = std::nullopt, | ||
| }); | ||
|
|
||
| if (cell->type() == GlobalObjectType) | ||
|
|
@@ -184,21 +175,32 @@ unsigned BunV8HeapSnapshotBuilder::analyzeNodeInternal(JSCell* cell) | |
| return index; | ||
| } | ||
|
|
||
| // m_edges stays a contiguous Vector because serialization sorts it. A Vector | ||
| // caps a single allocation at ~2GiB and CRASH()es past it, so record the | ||
| // overflow instead and let the serializers fail with a null result. | ||
| void BunV8HeapSnapshotBuilder::appendEdge(Edge&& edge) | ||
| { | ||
| if (m_overflowed.load(std::memory_order_relaxed)) | ||
| return; | ||
| if (!m_edges.tryAppend(WTF::move(edge))) [[unlikely]] | ||
| m_overflowed.store(true, std::memory_order_relaxed); | ||
| } | ||
|
|
||
| void BunV8HeapSnapshotBuilder::appendSyntheticRootEdges() | ||
| { | ||
| Edge rootToGcRoots {}; | ||
| rootToGcRoots.fromNodeId = kRootNodeIndex; | ||
| rootToGcRoots.toNodeId = kGcRootsNodeIndex; | ||
| rootToGcRoots.typeIndex = static_cast<unsigned>(V8EdgeType::Element); | ||
| rootToGcRoots.index = 0; | ||
| m_edges.append(WTF::move(rootToGcRoots)); | ||
| appendEdge(WTF::move(rootToGcRoots)); | ||
|
|
||
| for (unsigned globalIndex : m_globalObjectNodeIndices) { | ||
| Edge shortcut {}; | ||
| shortcut.fromNodeId = kRootNodeIndex; | ||
| shortcut.toNodeId = globalIndex; | ||
| shortcut.typeIndex = static_cast<unsigned>(V8EdgeType::Shortcut); | ||
| m_edges.append(WTF::move(shortcut)); | ||
| appendEdge(WTF::move(shortcut)); | ||
| } | ||
| } | ||
|
|
||
|
|
@@ -221,23 +223,12 @@ void BunV8HeapSnapshotBuilder::analyzeEdge(JSCell* from, JSCell* to, RootMarkRea | |
|
|
||
| edge.typeIndex = getEdgeTypeIndex(reason); | ||
|
|
||
| // Only track parent-child relationships for non-property and non-element edges | ||
| switch (edge.typeIndex) { | ||
| case static_cast<unsigned>(V8EdgeType::Element): | ||
| case static_cast<unsigned>(V8EdgeType::Property): | ||
| case static_cast<unsigned>(V8EdgeType::Context): | ||
| break; | ||
| default: { | ||
| if (edge.typeIndex == static_cast<unsigned>(V8EdgeType::Hidden)) { | ||
| Locker locker { m_buildingNodeMutex }; | ||
| m_nodes[edge.toNodeId].parentNodeId = edge.fromNodeId; | ||
| if (edge.typeIndex == static_cast<unsigned>(V8EdgeType::Hidden)) { | ||
| edge.index = WTF::atomicExchangeAdd(&m_nodes[edge.fromNodeId].edgesCount, 1); | ||
| } | ||
| break; | ||
| } | ||
| edge.index = WTF::atomicExchangeAdd(&m_nodes[edge.fromNodeId].edgesCount, 1); | ||
| } | ||
|
|
||
| m_edges.append(WTF::move(edge)); | ||
| appendEdge(WTF::move(edge)); | ||
| } | ||
|
|
||
| void BunV8HeapSnapshotBuilder::analyzePropertyNameEdge(JSCell* from, JSCell* to, UniquedStringImpl* propertyName) | ||
|
|
@@ -251,7 +242,7 @@ void BunV8HeapSnapshotBuilder::analyzePropertyNameEdge(JSCell* from, JSCell* to, | |
| edge.toNodeId = getOrCreateNodeId(to); | ||
| edge.typeIndex = static_cast<unsigned>(V8EdgeType::Property); | ||
| edge.name = WTF::String(propertyName); | ||
| m_edges.append(WTF::move(edge)); | ||
| appendEdge(WTF::move(edge)); | ||
| } | ||
|
|
||
| void BunV8HeapSnapshotBuilder::analyzeVariableNameEdge(JSCell* from, JSCell* to, UniquedStringImpl* variableName) | ||
|
|
@@ -266,7 +257,7 @@ void BunV8HeapSnapshotBuilder::analyzeVariableNameEdge(JSCell* from, JSCell* to, | |
| edge.typeIndex = static_cast<unsigned>(V8EdgeType::Context); | ||
| edge.name = String(variableName); | ||
|
|
||
| m_edges.append(WTF::move(edge)); | ||
| appendEdge(WTF::move(edge)); | ||
| } | ||
|
|
||
| void BunV8HeapSnapshotBuilder::analyzeIndexEdge(JSCell* from, JSCell* to, uint32_t index) | ||
|
|
@@ -281,7 +272,7 @@ void BunV8HeapSnapshotBuilder::analyzeIndexEdge(JSCell* from, JSCell* to, uint32 | |
| edge.typeIndex = static_cast<unsigned>(V8EdgeType::Element); | ||
| edge.index = index; | ||
|
|
||
| m_edges.append(WTF::move(edge)); | ||
| appendEdge(WTF::move(edge)); | ||
| } | ||
|
|
||
| void BunV8HeapSnapshotBuilder::setOpaqueRootReachabilityReasonForCell(JSCell*, ASCIILiteral) {} | ||
|
|
@@ -562,48 +553,19 @@ unsigned BunV8HeapSnapshotBuilder::addString(const String& str) | |
| return it->value; | ||
|
|
||
| unsigned index = m_strings.size(); | ||
| m_strings.append(WTF::move(key)); | ||
| if (!m_strings.tryAppend(WTF::move(key))) [[unlikely]] { | ||
| m_overflowed.store(true, std::memory_order_relaxed); | ||
| return 0; | ||
| } | ||
| m_stringsLookupTable.set(hashKey, index); | ||
| return index; | ||
| } | ||
|
|
||
| std::optional<BunV8HeapSnapshotBuilder::TraceLocation> BunV8HeapSnapshotBuilder::getTraceLocation(JSCell* cell) | ||
| { | ||
| if (!cell || !cell->isCallable()) | ||
| return std::nullopt; | ||
|
|
||
| JSFunction* function = dynamicDowncast<JSFunction>(cell); | ||
| if (!function || !function->executable() || function->isHostFunction()) | ||
| return std::nullopt; | ||
|
|
||
| auto* executable = function->jsExecutable(); | ||
| if (!executable) | ||
| return std::nullopt; | ||
|
|
||
| auto* provider = executable->source().provider(); | ||
| if (!provider) | ||
| return std::nullopt; | ||
|
|
||
| TraceLocation location; | ||
| location.scriptId = provider->asID(); | ||
| location.scriptName = provider->sourceURL(); | ||
| if (location.scriptName.isEmpty()) | ||
| location.scriptName = String(); | ||
|
|
||
| location.line = executable->firstLine(); | ||
| location.column = executable->startColumn(); | ||
| return { location }; | ||
| } | ||
|
|
||
| void BunV8HeapSnapshotBuilder::TraceLocation::sourcemap(VM&) | ||
| { | ||
| if (scriptName.isEmpty()) { | ||
| return; | ||
| } | ||
| } | ||
|
|
||
| String BunV8HeapSnapshotBuilder::generateV8HeapSnapshot() | ||
| { | ||
| if (m_overflowed.load(std::memory_order_relaxed)) | ||
| return String(); | ||
|
|
||
| // Extra pass #1: fill in the node names | ||
|
claude[bot] marked this conversation as resolved.
|
||
| for (auto& node : m_nodes) { | ||
| if (node.cell) | ||
|
|
@@ -667,7 +629,7 @@ String BunV8HeapSnapshotBuilder::generateV8HeapSnapshot() | |
| m_edges.shrink(writeIndex); | ||
| } | ||
|
|
||
| StringBuilder json; | ||
| StringBuilder json(OverflowPolicy::RecordOverflow); | ||
| json.append("{\"snapshot\":{\"meta\":{"_s); | ||
|
|
||
| // Node fields | ||
|
|
@@ -805,10 +767,42 @@ String BunV8HeapSnapshotBuilder::generateV8HeapSnapshot() | |
|
|
||
| json.append("}\n"_s); | ||
|
|
||
| if (json.hasOverflowed()) | ||
| m_overflowed.store(true, std::memory_order_relaxed); | ||
| if (m_overflowed.load(std::memory_order_relaxed)) | ||
|
Comment on lines
+770
to
+772
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🚀 Performance & Scalability | 🟡 Minor | ⚡ Quick win Stop both serializers when overflow is detected. Both paths return the required empty result, but only after traversing the complete snapshot. Add checks during each serialization phase and item loop.
📍 Affects 1 file
🤖 Prompt for AI Agents |
||
| return String(); | ||
|
|
||
| return json.toString(); | ||
| } | ||
|
|
||
| static void appendUTF8BytesQuotedJSON(Vector<uint8_t>& out, const WTF::String& str) | ||
| namespace { | ||
|
|
||
| // Vector<uint8_t> caps a single allocation at ~2GiB and CRASH()es past it. | ||
| // The sink records the overflow instead so jsonBytes() can fail cleanly. | ||
| struct JsonByteSink { | ||
| Vector<uint8_t> bytes; | ||
| bool overflowed { false }; | ||
|
|
||
| void append(uint8_t c) | ||
| { | ||
| if (overflowed) [[unlikely]] | ||
| return; | ||
| if (!bytes.tryAppend(c)) [[unlikely]] | ||
| overflowed = true; | ||
| } | ||
|
|
||
| void append(std::span<const uint8_t> span) | ||
| { | ||
| if (overflowed) [[unlikely]] | ||
| return; | ||
| if (!bytes.tryAppend(span)) [[unlikely]] | ||
| overflowed = true; | ||
| } | ||
| }; | ||
|
|
||
| } // namespace | ||
|
|
||
| static void appendUTF8BytesQuotedJSON(JsonByteSink& out, const WTF::String& str) | ||
| { | ||
| out.append('"'); | ||
| if (!str.isEmpty()) { | ||
|
|
@@ -847,17 +841,17 @@ static void appendUTF8BytesQuotedJSON(Vector<uint8_t>& out, const WTF::String& s | |
| out.append('"'); | ||
| } | ||
|
|
||
| static void appendASCIILiteral(Vector<uint8_t>& out, const char* str, size_t length) | ||
| static void appendASCIILiteral(JsonByteSink& out, const char* str, size_t length) | ||
| { | ||
| out.append(std::span<const uint8_t>(reinterpret_cast<const uint8_t*>(str), length)); | ||
| } | ||
|
|
||
| static void appendASCII(Vector<uint8_t>& out, ASCIILiteral literal) | ||
| static void appendASCII(JsonByteSink& out, ASCIILiteral literal) | ||
| { | ||
| appendASCIILiteral(out, literal.characters(), literal.length()); | ||
| } | ||
|
|
||
| static void appendUnsigned(Vector<uint8_t>& out, size_t value) | ||
| static void appendUnsigned(JsonByteSink& out, size_t value) | ||
| { | ||
| // Fast path for small numbers | ||
| if (value == 0) { | ||
|
|
@@ -877,6 +871,9 @@ static void appendUnsigned(Vector<uint8_t>& out, size_t value) | |
|
|
||
| Vector<uint8_t> BunV8HeapSnapshotBuilder::generateV8HeapSnapshotBytes() | ||
| { | ||
| if (m_overflowed.load(std::memory_order_relaxed)) | ||
| return {}; | ||
|
|
||
| // Extra pass #1: fill in the node names | ||
| for (auto& node : m_nodes) { | ||
| if (node.cell) | ||
|
|
@@ -930,7 +927,7 @@ Vector<uint8_t> BunV8HeapSnapshotBuilder::generateV8HeapSnapshotBytes() | |
| m_edges.shrink(writeIndex); | ||
| } | ||
|
|
||
| Vector<uint8_t> out; | ||
| JsonByteSink out; | ||
| const unsigned NODE_FIELD_COUNT = 7; | ||
|
|
||
| appendASCII(out, "{\"snapshot\":{\"meta\":{"_s); | ||
|
|
@@ -1053,7 +1050,12 @@ Vector<uint8_t> BunV8HeapSnapshotBuilder::generateV8HeapSnapshotBytes() | |
|
|
||
| appendASCII(out, "}\n"_s); | ||
|
|
||
| return out; | ||
| if (out.overflowed) | ||
| m_overflowed.store(true, std::memory_order_relaxed); | ||
| if (m_overflowed.load(std::memory_order_relaxed)) | ||
| return {}; | ||
|
|
||
| return WTF::move(out.bytes); | ||
| } | ||
|
|
||
| } // namespace JSC | ||
|
|
||
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
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift
🔎 Supported by static analysis
🏁 Script executed:
Repository: oven-sh/WebKit
Length of output: 565
🏁 Script executed:
Repository: oven-sh/WebKit
Length of output: 565
🌐 Web query:
WebKit WTF HashMap set tryAdd allocation failure HashMap API HashTable💡 Result:
In WebKit's WTF (Web Template Framework) library, HashMap and its underlying HashTable implementation do not provide a standard "tryAdd" API that returns a boolean specifically to indicate success or failure related to memory allocation [1][2][3][4]. Instead, standard HashMap insertion methods like set and add are designed to be used in contexts where allocation success is expected [1][2]. WebKit generally handles allocation failures through systemic mechanisms (e.g., crashing or calling out-of-memory handlers) rather than through per-call error codes for containers [4][5][6]. Regarding your query on "tryAdd": 1. While WTF::HashMap does not expose a general-purpose tryAdd method for allocation failure recovery, specific WebKit components (such as JSC::HasOwnPropertyCache) do implement a method named tryAdd [7][8][9]. 2. In this specific context, tryAdd is used as a caching utility to attempt to insert entries into a fixed-size or specific-policy cache [8][10][11]. It is not a general error-handling mechanism for standard HashMap allocation failures [8]. 3. The underlying HashTable, which backs HashMap, manages memory via an Allocator template parameter (commonly Malloc) [12][4][13]. It contains logic to check if allocation is allowed (e.g., Allocator::isAllocationAllowed) [4][5], but these are internal safety assertions rather than APIs intended for user-space error handling during insertion [4]. For robust handling of potential out-of-memory conditions, WebKit developers typically ensure that hash containers are either appropriately sized, reside in areas where OOM is handled by the garbage collector, or use structures designed for specific memory constraints, rather than relying on an API-level tryAdd return value [4][6].
Citations:
🌐 Web query:
site:github.com/oven-sh/WebKit "m_stringsLookupTable" "BunV8HeapSnapshotBuilder"💡 Result:
The terms m_stringsLookupTable and BunV8HeapSnapshotBuilder are not standard public-facing APIs or widely recognized components within the Bun runtime or its underlying WebKit-based JavaScriptCore engine. Research into the Bun and WebKit repositories does not yield these specific identifiers, suggesting they may be internal, localized, or transient names used in private development branches, specific research projects, or internal tooling not reflected in the public source code [1][2][3]. In the context of Bun's architecture: - Bun utilizes a custom version of WebKit's JavaScriptCore (JSC) with various additions and patches (guarded by USE(BUN_JSC_ADDITIONS)) to support Node.js compatibility, native FFI, and performance optimizations [1][2]. - String handling in Bun often involves specialized utilities, such as
src/codegen/generate-compact-string-table.tsandsrc/comptime_string_map.zig, which are used to generate efficient, compile-time string tables and maps [4][5]. - Heap profiling and snapshotting in WebKit/JSC are typically managed via components likeHeapSnapshot,HeapProfiler, andSlotVisitor[6][7]. If you encountered these terms in a specific stack trace, error message, or private code base, it is likely they belong to a non-public or highly specialized extension of the engine [2][8].Citations:
🌐 Web query:
site:github.com/WebKit/WebKit/blob/master/Source/WTF/wtf/HashMap.h "try" "set" allocation💡 Result:
The relevant WebKit
HashMap.hcode is:set()declarations at lines 1901–1903.set()callsinlineSet().inlineSet()callsinlineAdd(), which delegates to the underlying hash table’sadd().Direct file:
Source/WTF/wtf/HashMap.hon WebKit’smainbranch.Citations:
🌐 Web query:
WebKit WTF HashMap fallible allocation tryReserve tryAdd HashTable add out of memory💡 Result:
In the WebKit WTF (Web Template Framework) library, memory allocation for
HashMapandHashSetis managed by the underlyingHashTableimplementation [1][2]. Regarding your specific questions on fallible allocation,tryReserve,tryAdd, and out-of-memory (OOM) handling: 1. Allocation Strategy: TheWTF::HashTablecore, which powersHashMap, typically relies on internalallocateTableroutines [1][2]. These operations check if allocation is allowed (usingAllocator::isAllocationAllowed()) before proceeding [1][3]. If a memory allocation fails, the default behavior in many WebKit contexts is to trigger a fatal error or panic (often viaRELEASE_ASSERTor similar mechanisms) rather than returning a status code to the caller [1][3]. 2. Lack of Explicit 'try' API: Unlike standard C++ containers that might offertry_emplaceor similar patterns designed for graceful error handling, the standardWTF::HashMap::addandHashTable::addmethods are generally designed under the assumption that allocations will succeed [1][2]. There is no ubiquitous, standardized 'tryAdd' or 'tryReserve' API across all WebKit hash collections that explicitly returns a success boolean to handle OOM scenarios gracefully at the application level [4][5]. 3. OOM and Inconsistency: WebKit has historically addressed concerns regarding container consistency during OOM events [6]. For example, recent improvements toOrderedHashTablehave focused on ensuring that if an OOM occurs during a rehash or expansion, the container does not remain in an inconsistent, corrupted state [6]. This is often achieved by calculating necessary metadata before resizing and ensuring that the new buffer is fully prepared before swapping it with the old one, allowing the operation to be atomic from the perspective of the container's integrity [6]. If you are encountering OOM issues, the architectural expectation in WebKit is that the system memory pressure will be managed at a higher level (e.g., via the JavaScriptCore GC or platform-specific memory pressure handlers) rather than through fine-grained, fallible allocation checks on every hash map insertion [1][6]. If you require specific behavior for memory-constrained environments, you would typically need to implement custom allocators or use specialized container types that are designed to handle memory failure scenarios explicitly [7].Citations:
🌐 Web query:
site:github.com/oven-sh/WebKit/blob/69fa2714ab5f917c2d15501ff8cfdccfaea78882/Source/WTF/wtf/HashMap.h "inlineSet"💡 Result:
inlineSetis defined at lines 2377–2392 in the specifiedHashMap.hrevision. It callsinlineAdd, then overwrites the mapped value if the key already exists. (github.com)Citations:
Handle allocation failure during lookup-table growth.
When a new
hashKeyrequires growth,m_stringsLookupTable.set()callsinlineAdd()and the underlyingHashTable::add()allocation path. This path is infallible, so allocation failure can terminate beforem_overflowedis set. Use a fallible lookup-table insertion path and setm_overflowedwhen it fails.🤖 Prompt for AI Agents