From 3646a66044a5c265802499639a312d422b30b685 Mon Sep 17 00:00:00 2001 From: Joyee Cheung Date: Fri, 24 Feb 2023 15:51:07 +0100 Subject: [PATCH] src: fix AliasedBuffer memory attribution in heap snapshots Before the AliasedBuffers were represented solely by the TypedArrays event though they also retain additional memory themselves. Make the accounting more accurate by implementing MemoryRetainer in AliasedBuffer. PR-URL: https://github.com/nodejs/node/pull/46817 Reviewed-By: Anna Henningsen Reviewed-By: Matteo Collina --- src/aliased_buffer-inl.h | 19 +++++++++++++++++++ src/aliased_buffer.h | 8 +++++++- src/memory_tracker-inl.h | 9 +-------- src/memory_tracker.h | 5 ----- test/pummel/test-heapdump-fs-promise.js | 2 +- 5 files changed, 28 insertions(+), 15 deletions(-) diff --git a/src/aliased_buffer-inl.h b/src/aliased_buffer-inl.h index a3bee2593aeca6..58e9b6f8cef149 100644 --- a/src/aliased_buffer-inl.h +++ b/src/aliased_buffer-inl.h @@ -206,6 +206,25 @@ void AliasedBufferBase::reserve(size_t new_capacity) { count_ = new_capacity; } +template +inline size_t AliasedBufferBase::SelfSize() const { + return sizeof(*this); +} + +#define VM(NativeT, V8T) \ + template <> \ + inline const char* AliasedBufferBase::MemoryInfoName() \ + const { \ + return "Aliased" #V8T; \ + } \ + template <> \ + inline void AliasedBufferBase::MemoryInfo( \ + node::MemoryTracker* tracker) const { \ + tracker->TrackField("js_array", js_array_); \ + } +ALIASED_BUFFER_LIST(VM) +#undef VM + } // namespace node #endif // defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS diff --git a/src/aliased_buffer.h b/src/aliased_buffer.h index 1b9336e1cd3397..bc858de3b3d268 100644 --- a/src/aliased_buffer.h +++ b/src/aliased_buffer.h @@ -4,6 +4,7 @@ #if defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS #include +#include "memory_tracker.h" #include "v8.h" namespace node { @@ -28,7 +29,7 @@ typedef size_t AliasedBufferIndex; * observed. Any notification APIs will be left as a future exercise. */ template -class AliasedBufferBase { +class AliasedBufferBase : public MemoryRetainer { public: static_assert(std::is_scalar::value); @@ -157,6 +158,11 @@ class AliasedBufferBase { // an owning `AliasedBufferBase`. void reserve(size_t new_capacity); + inline size_t SelfSize() const override; + + inline const char* MemoryInfoName() const override; + inline void MemoryInfo(node::MemoryTracker* tracker) const override; + private: v8::Isolate* isolate_ = nullptr; size_t count_ = 0; diff --git a/src/memory_tracker-inl.h b/src/memory_tracker-inl.h index b8d2667d981c9d..0e4f2870a5aae5 100644 --- a/src/memory_tracker-inl.h +++ b/src/memory_tracker-inl.h @@ -3,8 +3,8 @@ #if defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS -#include "aliased_buffer-inl.h" #include "memory_tracker.h" +#include "util-inl.h" namespace node { @@ -270,13 +270,6 @@ void MemoryTracker::TrackInlineField(const char* name, TrackInlineFieldWithSize(name, sizeof(value), "uv_async_t"); } -template -void MemoryTracker::TrackField(const char* name, - const AliasedBufferBase& value, - const char* node_name) { - TrackField(name, value.GetJSArray(), "AliasedBuffer"); -} - void MemoryTracker::Track(const MemoryRetainer* retainer, const char* edge_name) { v8::HandleScope handle_scope(isolate_); diff --git a/src/memory_tracker.h b/src/memory_tracker.h index ae3b00317906f5..cae4e5c7a663c1 100644 --- a/src/memory_tracker.h +++ b/src/memory_tracker.h @@ -2,7 +2,6 @@ #if defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS -#include "aliased_buffer.h" #include "v8-profiler.h" #include @@ -238,10 +237,6 @@ class MemoryTracker { inline void TrackInlineField(const char* edge_name, const uv_async_t& value, const char* node_name = nullptr); - template - inline void TrackField(const char* edge_name, - const AliasedBufferBase& value, - const char* node_name = nullptr); // Put a memory container into the graph, create an edge from // the current node if there is one on the stack. diff --git a/test/pummel/test-heapdump-fs-promise.js b/test/pummel/test-heapdump-fs-promise.js index 62defa2df7820b..10ee087ae7fc23 100644 --- a/test/pummel/test-heapdump-fs-promise.js +++ b/test/pummel/test-heapdump-fs-promise.js @@ -10,7 +10,7 @@ validateSnapshotNodes('Node / FSReqPromise', [ { children: [ { node_name: 'FSReqPromise', edge_name: 'native_to_javascript' }, - { node_name: 'Float64Array', edge_name: 'stats_field_array' }, + { node_name: 'Node / AliasedFloat64Array', edge_name: 'stats_field_array' }, ], }, ]);