diff --git a/src/base_object.h b/src/base_object.h index 2c67445c31fbb6..1c52d507ab2226 100644 --- a/src/base_object.h +++ b/src/base_object.h @@ -103,6 +103,7 @@ class BaseObject : public MemoryRetainer { private: v8::Local WrappedObject() const override; + bool IsRootNode() const override; static void DeleteMe(void* data); // persistent_handle_ needs to be at a fixed offset from the start of the diff --git a/src/env.cc b/src/env.cc index 34f5adccc2e0b5..18788e4ceaf208 100644 --- a/src/env.cc +++ b/src/env.cc @@ -986,33 +986,16 @@ Environment* Environment::worker_parent_env() const { return worker_context_->env(); } -void MemoryTracker::TrackField(const char* edge_name, - const CleanupHookCallback& value, - const char* node_name) { - HandleScope handle_scope(isolate_); - // Here, we utilize the fact that CleanupHookCallback instances - // are all unique and won't be tracked twice in one BuildEmbedderGraph - // callback. - MemoryRetainerNode* n = - PushNode("CleanupHookCallback", sizeof(value), edge_name); - // TODO(joyeecheung): at the moment only arguments of type BaseObject will be - // identified and tracked here (based on their deleters), - // but we may convert and track other known types here. - BaseObject* obj = value.GetBaseObject(); - if (obj != nullptr && obj->IsDoneInitializing()) { - TrackField("arg", obj); - } - CHECK_EQ(CurrentNode(), n); - CHECK_NE(n->size_, 0); - PopNode(); -} - void Environment::BuildEmbedderGraph(Isolate* isolate, EmbedderGraph* graph, void* data) { MemoryTracker tracker(isolate, graph); Environment* env = static_cast(data); tracker.Track(env); + env->ForEachBaseObject([&](BaseObject* obj) { + if (obj->IsDoneInitializing()) + tracker.Track(obj); + }); } inline size_t Environment::SelfSize() const { @@ -1042,7 +1025,8 @@ void Environment::MemoryInfo(MemoryTracker* tracker) const { tracker->TrackField("fs_stats_field_array", fs_stats_field_array_); tracker->TrackField("fs_stats_field_bigint_array", fs_stats_field_bigint_array_); - tracker->TrackField("cleanup_hooks", cleanup_hooks_); + tracker->TrackFieldWithSize( + "cleanup_hooks", cleanup_hooks_.size() * sizeof(CleanupHookCallback)); tracker->TrackField("async_hooks", async_hooks_); tracker->TrackField("immediate_info", immediate_info_); tracker->TrackField("tick_info", tick_info_); @@ -1136,4 +1120,8 @@ Local BaseObject::WrappedObject() const { return object(); } +bool BaseObject::IsRootNode() const { + return !persistent_handle_.IsWeak(); +} + } // namespace node diff --git a/src/memory_tracker.h b/src/memory_tracker.h index 6cd372f493c41d..83443cbd6bf679 100644 --- a/src/memory_tracker.h +++ b/src/memory_tracker.h @@ -208,13 +208,6 @@ class MemoryTracker { inline void TrackField(const char* edge_name, const MallocedBuffer& value, const char* node_name = nullptr); - // We do not implement CleanupHookCallback as MemoryRetainer - // but instead specialize the method here to avoid the cost of - // virtual pointers. - // TODO(joyeecheung): do this for BaseObject and remove WrappedObject() - void TrackField(const char* edge_name, - const CleanupHookCallback& value, - const char* node_name = nullptr); inline void TrackField(const char* edge_name, const uv_buf_t& value, const char* node_name = nullptr); diff --git a/test/pummel/test-heapdump-env.js b/test/pummel/test-heapdump-env.js index 8175797cce4f1e..7030271469c512 100644 --- a/test/pummel/test-heapdump-env.js +++ b/test/pummel/test-heapdump-env.js @@ -5,7 +5,6 @@ require('../common'); const { validateSnapshotNodes } = require('../common/heap'); -const assert = require('assert'); // This is just using ContextifyScript as an example here, it can be replaced // with any BaseObject that we can easily instantiate here and register in @@ -16,51 +15,10 @@ const context = require('vm').createScript('const foo = 123'); validateSnapshotNodes('Node / Environment', [{ children: [ - cleanupHooksFilter, { node_name: 'Node / cleanup_hooks', edge_name: 'cleanup_hooks' }, { node_name: 'process', edge_name: 'process_object' }, { node_name: 'Node / IsolateData', edge_name: 'isolate_data' }, ] }]); -function cleanupHooksFilter(edge) { - if (edge.name !== 'cleanup_hooks') { - return false; - } - if (edge.to.type === 'native') { - verifyCleanupHooksInSnapshot(edge.to); - } else { - verifyCleanupHooksInGraph(edge.to); - } - return true; -} - -function verifyCleanupHooksInSnapshot(node) { - assert.strictEqual(node.name, 'Node / cleanup_hooks'); - const baseObjects = []; - for (const hook of node.outgoingEdges) { - for (const hookEdge of hook.to.outgoingEdges) { - if (hookEdge.name === 'arg') { - baseObjects.push(hookEdge.to); - } - } - } - // Make sure our ContextifyScript show up. - assert(baseObjects.some((node) => node.name === 'Node / ContextifyScript')); -} - -function verifyCleanupHooksInGraph(node) { - assert.strictEqual(node.name, 'Node / cleanup_hooks'); - const baseObjects = []; - for (const hook of node.edges) { - for (const hookEdge of hook.to.edges) { - if (hookEdge.name === 'arg') { - baseObjects.push(hookEdge.to); - } - } - } - // Make sure our ContextifyScript show up. - assert(baseObjects.some((node) => node.name === 'Node / ContextifyScript')); -} - console.log(context); // Make sure it's not GC'ed