Skip to content

Commit 17285d3

Browse files
addaleaxnodejs-github-bot
authored andcommitted
src: allow tracking children in MemoryTracker with weak edges
Allowing the addition of child nodes with a weak edge between them enables expressing a relationship of the "node A keeps track of node B and no other node does, but node A does not keep node B alive" kind, which is the relationship between `Realm`s and weak `BaseObject` instances. This is a prerequisite for the following commit. Note that the relevant V8 feature is not available in Node.js yet. It's still worth doing this, both because it allows us to prepare for that upstream change, and because the current representation of these edges in heap dumps is factually incorrect and hinders debugging in existing Node.js versions. Refs: v8/v8@e37cadf Signed-off-by: Anna Henningsen <anna@addaleax.net> PR-URL: #63842 Refs: #57417 Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com> Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
1 parent 1efdce3 commit 17285d3

3 files changed

Lines changed: 30 additions & 11 deletions

File tree

src/base_object.cc

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -180,4 +180,6 @@ void BaseObjectList::MemoryInfo(node::MemoryTracker* tracker) const {
180180
}
181181
}
182182

183+
const char* const MemoryTracker::kWeakEdge = "<MemoryTracker::kWeakEdge>";
184+
183185
} // namespace node

src/memory_tracker-inl.h

Lines changed: 18 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -131,7 +131,7 @@ void MemoryTracker::TrackField(const char* edge_name,
131131
if (value == nullptr) return;
132132
auto it = seen_.find(value);
133133
if (it != seen_.end()) {
134-
graph_->AddEdge(CurrentNode(), it->second, edge_name);
134+
AddEdge(CurrentNode(), it->second, edge_name);
135135
} else {
136136
Track(value, edge_name);
137137
}
@@ -253,9 +253,9 @@ void MemoryTracker::TrackField(const char* edge_name,
253253
const v8::Local<T>& value,
254254
const char* node_name) {
255255
if (!value.IsEmpty())
256-
graph_->AddEdge(CurrentNode(),
257-
graph_->V8Node(value.template As<v8::Value>()),
258-
edge_name);
256+
AddEdge(CurrentNode(),
257+
graph_->V8Node(value.template As<v8::Value>()),
258+
edge_name);
259259
}
260260

261261
template <typename T>
@@ -300,7 +300,7 @@ void MemoryTracker::Track(const CppgcMixin* retainer, const char* edge_name) {
300300
auto it = seen_.find(retainer);
301301
if (it != seen_.end()) {
302302
if (CurrentNode() != nullptr) {
303-
graph_->AddEdge(CurrentNode(), it->second, edge_name);
303+
AddEdge(CurrentNode(), it->second, edge_name);
304304
}
305305
return; // It has already been tracked, no need to call MemoryInfo again
306306
}
@@ -317,7 +317,7 @@ void MemoryTracker::Track(const MemoryRetainer* retainer,
317317
auto it = seen_.find(retainer);
318318
if (it != seen_.end()) {
319319
if (CurrentNode() != nullptr) {
320-
graph_->AddEdge(CurrentNode(), it->second, edge_name);
320+
AddEdge(CurrentNode(), it->second, edge_name);
321321
}
322322
return; // It has already been tracked, no need to call MemoryInfo again
323323
}
@@ -376,7 +376,7 @@ MemoryRetainerNode* MemoryTracker::AddNode(const CppgcMixin* retainer,
376376
MemoryRetainerNode* n = new MemoryRetainerNode(this, retainer);
377377
seen_[retainer] = n;
378378
if (CurrentNode() != nullptr) {
379-
graph_->AddEdge(CurrentNode(), n->JSWrapperNode(), edge_name);
379+
AddEdge(CurrentNode(), n->JSWrapperNode(), edge_name);
380380
}
381381

382382
return n;
@@ -392,11 +392,11 @@ MemoryRetainerNode* MemoryTracker::AddNode(const MemoryRetainer* retainer,
392392
MemoryRetainerNode* n = new MemoryRetainerNode(this, retainer);
393393
graph_->AddNode(std::unique_ptr<v8::EmbedderGraph::Node>(n));
394394
seen_[retainer] = n;
395-
if (CurrentNode() != nullptr) graph_->AddEdge(CurrentNode(), n, edge_name);
395+
if (CurrentNode() != nullptr) AddEdge(CurrentNode(), n, edge_name);
396396

397397
if (n->JSWrapperNode() != nullptr) {
398-
graph_->AddEdge(n, n->JSWrapperNode(), "native_to_javascript");
399-
graph_->AddEdge(n->JSWrapperNode(), n, "javascript_to_native");
398+
AddEdge(n, n->JSWrapperNode(), "native_to_javascript");
399+
AddEdge(n->JSWrapperNode(), n, "javascript_to_native");
400400
}
401401

402402
return n;
@@ -408,7 +408,7 @@ MemoryRetainerNode* MemoryTracker::AddNode(const char* node_name,
408408
MemoryRetainerNode* n = new MemoryRetainerNode(this, node_name, size);
409409
graph_->AddNode(std::unique_ptr<v8::EmbedderGraph::Node>(n));
410410

411-
if (CurrentNode() != nullptr) graph_->AddEdge(CurrentNode(), n, edge_name);
411+
if (CurrentNode() != nullptr) AddEdge(CurrentNode(), n, edge_name);
412412

413413
return n;
414414
}
@@ -435,6 +435,13 @@ MemoryRetainerNode* MemoryTracker::PushNode(const char* node_name,
435435
return n;
436436
}
437437

438+
void MemoryTracker::AddEdge(v8::EmbedderGraph::Node* from,
439+
v8::EmbedderGraph::Node* to,
440+
const char* edge_name) {
441+
if (edge_name == kWeakEdge) return;
442+
graph_->AddEdge(from, to, edge_name);
443+
}
444+
438445
void MemoryTracker::PopNode() {
439446
node_stack_.pop();
440447
}

src/memory_tracker.h

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -300,6 +300,13 @@ class MemoryTracker {
300300
v8::EmbedderGraph* graph)
301301
: isolate_(isolate), graph_(graph) {}
302302

303+
// Can be passed to Track() if it is not desirable
304+
// to create a strong edge between nodes, i.e. when
305+
// the node should be added to the graph and the
306+
// parent node is "tracking" the target node but
307+
// not directly keeping it alive.
308+
static const char* const kWeakEdge;
309+
303310
private:
304311
typedef std::unordered_map<const MemoryRetainer*, MemoryRetainerNode*>
305312
NodeMap;
@@ -321,6 +328,9 @@ class MemoryTracker {
321328
size_t size,
322329
const char* edge_name = nullptr);
323330
inline void PopNode();
331+
inline void AddEdge(v8::EmbedderGraph::Node* from,
332+
v8::EmbedderGraph::Node* to,
333+
const char* edge_name = nullptr);
324334

325335
v8::Isolate* isolate_;
326336
v8::EmbedderGraph* graph_;

0 commit comments

Comments
 (0)