diff --git a/src/base_object.cc b/src/base_object.cc index 102832c58c62ee..d2799597ac3308 100644 --- a/src/base_object.cc +++ b/src/base_object.cc @@ -174,10 +174,18 @@ void BaseObjectList::Cleanup() { } } -void BaseObjectList::MemoryInfo(node::MemoryTracker* tracker) const { +void BaseObjectList::MemoryInfo(MemoryTracker* tracker) const { for (auto bo : *this) { - if (bo->IsDoneInitializing()) tracker->Track(bo); + if (bo->IsDoneInitializing()) { + // TODO(addaleax): Add weak edges instead of no edges once + // https://github.com/v8/v8/commit/e37cadf1143a8c5bbe44c0408186b5a26cc23863 + // is available for us + tracker->Track( + bo, bo->persistent().IsWeak() ? MemoryTracker::kNoEdge : nullptr); + } } } +const char* const MemoryTracker::kNoEdge = ""; + } // namespace node diff --git a/src/cppgc_helpers.cc b/src/cppgc_helpers.cc index 4e3ffb678c2d09..5d6b68620a2a8f 100644 --- a/src/cppgc_helpers.cc +++ b/src/cppgc_helpers.cc @@ -16,7 +16,10 @@ void CppgcWrapperList::MemoryInfo(MemoryTracker* tracker) const { for (auto node : *this) { CppgcMixin* ptr = node->persistent.Get(); if (ptr != nullptr) { - tracker->Track(ptr); + // TODO(addaleax): Add weak edges instead of no edges once + // https://github.com/v8/v8/commit/e37cadf1143a8c5bbe44c0408186b5a26cc23863 + // is available for us + tracker->Track(ptr, MemoryTracker::kNoEdge); } } } diff --git a/src/memory_tracker-inl.h b/src/memory_tracker-inl.h index 56f4a180633d39..58bb9e848805c8 100644 --- a/src/memory_tracker-inl.h +++ b/src/memory_tracker-inl.h @@ -131,7 +131,7 @@ void MemoryTracker::TrackField(const char* edge_name, if (value == nullptr) return; auto it = seen_.find(value); if (it != seen_.end()) { - graph_->AddEdge(CurrentNode(), it->second, edge_name); + AddEdge(CurrentNode(), it->second, edge_name); } else { Track(value, edge_name); } @@ -253,9 +253,9 @@ void MemoryTracker::TrackField(const char* edge_name, const v8::Local& value, const char* node_name) { if (!value.IsEmpty()) - graph_->AddEdge(CurrentNode(), - graph_->V8Node(value.template As()), - edge_name); + AddEdge(CurrentNode(), + graph_->V8Node(value.template As()), + edge_name); } template @@ -300,7 +300,7 @@ void MemoryTracker::Track(const CppgcMixin* retainer, const char* edge_name) { auto it = seen_.find(retainer); if (it != seen_.end()) { if (CurrentNode() != nullptr) { - graph_->AddEdge(CurrentNode(), it->second, edge_name); + AddEdge(CurrentNode(), it->second, edge_name); } return; // It has already been tracked, no need to call MemoryInfo again } @@ -317,7 +317,7 @@ void MemoryTracker::Track(const MemoryRetainer* retainer, auto it = seen_.find(retainer); if (it != seen_.end()) { if (CurrentNode() != nullptr) { - graph_->AddEdge(CurrentNode(), it->second, edge_name); + AddEdge(CurrentNode(), it->second, edge_name); } return; // It has already been tracked, no need to call MemoryInfo again } @@ -376,7 +376,7 @@ MemoryRetainerNode* MemoryTracker::AddNode(const CppgcMixin* retainer, MemoryRetainerNode* n = new MemoryRetainerNode(this, retainer); seen_[retainer] = n; if (CurrentNode() != nullptr) { - graph_->AddEdge(CurrentNode(), n->JSWrapperNode(), edge_name); + AddEdge(CurrentNode(), n->JSWrapperNode(), edge_name); } return n; @@ -392,11 +392,11 @@ MemoryRetainerNode* MemoryTracker::AddNode(const MemoryRetainer* retainer, MemoryRetainerNode* n = new MemoryRetainerNode(this, retainer); graph_->AddNode(std::unique_ptr(n)); seen_[retainer] = n; - if (CurrentNode() != nullptr) graph_->AddEdge(CurrentNode(), n, edge_name); + if (CurrentNode() != nullptr) AddEdge(CurrentNode(), n, edge_name); if (n->JSWrapperNode() != nullptr) { - graph_->AddEdge(n, n->JSWrapperNode(), "native_to_javascript"); - graph_->AddEdge(n->JSWrapperNode(), n, "javascript_to_native"); + AddEdge(n, n->JSWrapperNode(), "native_to_javascript"); + AddEdge(n->JSWrapperNode(), n, "javascript_to_native"); } return n; @@ -408,7 +408,7 @@ MemoryRetainerNode* MemoryTracker::AddNode(const char* node_name, MemoryRetainerNode* n = new MemoryRetainerNode(this, node_name, size); graph_->AddNode(std::unique_ptr(n)); - if (CurrentNode() != nullptr) graph_->AddEdge(CurrentNode(), n, edge_name); + if (CurrentNode() != nullptr) AddEdge(CurrentNode(), n, edge_name); return n; } @@ -435,6 +435,13 @@ MemoryRetainerNode* MemoryTracker::PushNode(const char* node_name, return n; } +void MemoryTracker::AddEdge(v8::EmbedderGraph::Node* from, + v8::EmbedderGraph::Node* to, + const char* edge_name) { + if (edge_name == kNoEdge) return; + graph_->AddEdge(from, to, edge_name); +} + void MemoryTracker::PopNode() { node_stack_.pop(); } diff --git a/src/memory_tracker.h b/src/memory_tracker.h index a6c3fee6a847a1..ec5ad35a57ebca 100644 --- a/src/memory_tracker.h +++ b/src/memory_tracker.h @@ -300,6 +300,11 @@ class MemoryTracker { v8::EmbedderGraph* graph) : isolate_(isolate), graph_(graph) {} + // Can be passed to Track() if it is _not_ desirable + // to create an edge between nodes, and the target node is + // only supposed to be added to the graph, not (yet) connected. + static const char* const kNoEdge; + private: typedef std::unordered_map NodeMap; @@ -321,6 +326,9 @@ class MemoryTracker { size_t size, const char* edge_name = nullptr); inline void PopNode(); + inline void AddEdge(v8::EmbedderGraph::Node* from, + v8::EmbedderGraph::Node* to, + const char* edge_name = nullptr); v8::Isolate* isolate_; v8::EmbedderGraph* graph_; diff --git a/test/common/heap.js b/test/common/heap.js index ac7e090326cd18..6c83ca9978270f 100644 --- a/test/common/heap.js +++ b/test/common/heap.js @@ -336,6 +336,19 @@ function validateByRetainingPath(...args) { return validateByRetainingPathFromNodes(nodes, ...args); } +function getRetainingNodes(startingNode, filter) { + const seen = new Set(); + function listNodes(node) { + if (!filter(node) || seen.has(node)) return; + seen.add(node); + for (const edge of node.incomingEdges) { + listNodes(edge.from); + } + } + listNodes(startingNode); + return [...seen]; +} + module.exports = { recordState, validateSnapshotNodes, @@ -343,4 +356,5 @@ module.exports = { validateByRetainingPathFromNodes, getHeapSnapshotOptionTests, createJSHeapSnapshot, + getRetainingNodes, }; diff --git a/test/pummel/test-heapdump-zlib.js b/test/pummel/test-heapdump-zlib.js index 9f11fbcadad306..f6ca5ceaab899b 100644 --- a/test/pummel/test-heapdump-zlib.js +++ b/test/pummel/test-heapdump-zlib.js @@ -3,7 +3,7 @@ const common = require('../common'); const assert = require('assert'); -const { validateByRetainingPath, validateByRetainingPathFromNodes } = require('../common/heap'); +const { validateByRetainingPath, validateByRetainingPathFromNodes, getRetainingNodes } = require('../common/heap'); const zlib = require('zlib'); // Before zlib stream is created, no ZlibStream should be created. @@ -27,6 +27,18 @@ const gzip = zlib.createGzip(); assert.strictEqual(withMemory.length, 0); } +{ + // Assert that the `ZlibStream` has no unexpected connections + // to other `Node / ...` nodes. + const contexts = validateByRetainingPath('Node / ZlibContext', []); + assert.deepStrictEqual( + getRetainingNodes(contexts[0], (node) => node.name?.startsWith('Node /')) + .map((node) => node.name).sort(), [ + 'Node / ZlibContext', + 'Node / ZlibStream', + ]); +} + // After zlib stream is written, zlib_memory should be created. gzip.write('hello world', common.mustCall(() => { validateByRetainingPath('Node / ZlibStream', [