From f8def281755d8cca925fe7ee0b4c26d12b6e43ae Mon Sep 17 00:00:00 2001 From: Joyee Cheung Date: Fri, 24 Feb 2023 15:25:10 +0100 Subject: [PATCH 1/5] src: make util.h self-containted Before it depended on util-inl.h. Fix it by moving MaybeStackBuffer::AllocateSufficientStorage() into util-inl.h --- src/util-inl.h | 16 ++++++++++++++++ src/util.h | 14 +------------- 2 files changed, 17 insertions(+), 13 deletions(-) diff --git a/src/util-inl.h b/src/util-inl.h index 833082291a16aa..864f6d86cdf689 100644 --- a/src/util-inl.h +++ b/src/util-inl.h @@ -510,6 +510,22 @@ SlicedArguments::SlicedArguments( (*this)[i] = args[i + start]; } +template +void MaybeStackBuffer::AllocateSufficientStorage( + size_t storage) { + CHECK(!IsInvalidated()); + if (storage > capacity()) { + bool was_allocated = IsAllocated(); + T* allocated_ptr = was_allocated ? buf_ : nullptr; + buf_ = Realloc(allocated_ptr, storage); + capacity_ = storage; + if (!was_allocated && length_ > 0) + memcpy(buf_, buf_st_, length_ * sizeof(buf_[0])); + } + + length_ = storage; +} + template ArrayBufferViewContents::ArrayBufferViewContents( v8::Local value) { diff --git a/src/util.h b/src/util.h index e30f298f8dd836..b0dbed52fa9004 100644 --- a/src/util.h +++ b/src/util.h @@ -412,19 +412,7 @@ class MaybeStackBuffer { // This method can be called multiple times throughout the lifetime of the // buffer, but once this has been called Invalidate() cannot be used. // Content of the buffer in the range [0, length()) is preserved. - void AllocateSufficientStorage(size_t storage) { - CHECK(!IsInvalidated()); - if (storage > capacity()) { - bool was_allocated = IsAllocated(); - T* allocated_ptr = was_allocated ? buf_ : nullptr; - buf_ = Realloc(allocated_ptr, storage); - capacity_ = storage; - if (!was_allocated && length_ > 0) - memcpy(buf_, buf_st_, length_ * sizeof(buf_[0])); - } - - length_ = storage; - } + void AllocateSufficientStorage(size_t storage); void SetLength(size_t length) { // capacity() returns how much memory is actually available. From cf1cc6a6bc95868a6a2a610cb97bed7ea3e003eb Mon Sep 17 00:00:00 2001 From: Joyee Cheung Date: Fri, 24 Feb 2023 15:46:19 +0100 Subject: [PATCH 2/5] src: move AliasedBuffer implementation to -inl.h Drive-by: Replace the SFINAE with a static_assert because we don't have (or need) an implementation for non-scalar AliasedBufferBase otherwise. Add forward declarations to memory_tracker.h now that aliased-buffer.h no longer includes util-inl.h. --- node.gyp | 1 + src/aliased_buffer-inl.h | 213 +++++++++++++++++++++++++++++ src/aliased_buffer.h | 201 ++++++--------------------- src/env-inl.h | 2 +- src/memory_tracker-inl.h | 1 + src/memory_tracker.h | 7 + src/node_file.cc | 2 +- src/node_http2.cc | 2 +- src/node_perf.cc | 2 +- src/node_v8.cc | 1 + test/cctest/test_aliased_buffer.cc | 3 +- 11 files changed, 268 insertions(+), 167 deletions(-) create mode 100644 src/aliased_buffer-inl.h diff --git a/node.gyp b/node.gyp index cb46307ef62012..f4df47d2c17c36 100644 --- a/node.gyp +++ b/node.gyp @@ -571,6 +571,7 @@ 'src/uv.cc', # headers to make for a more pleasant IDE experience 'src/aliased_buffer.h', + 'src/aliased_buffer-inl.h', 'src/aliased_struct.h', 'src/aliased_struct-inl.h', 'src/async_wrap.h', diff --git a/src/aliased_buffer-inl.h b/src/aliased_buffer-inl.h new file mode 100644 index 00000000000000..a3bee2593aeca6 --- /dev/null +++ b/src/aliased_buffer-inl.h @@ -0,0 +1,213 @@ +#ifndef SRC_ALIASED_BUFFER_INL_H_ +#define SRC_ALIASED_BUFFER_INL_H_ + +#if defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS + +#include "aliased_buffer.h" +#include "util-inl.h" + +namespace node { + +typedef size_t AliasedBufferIndex; + +template +AliasedBufferBase::AliasedBufferBase( + v8::Isolate* isolate, const size_t count, const AliasedBufferIndex* index) + : isolate_(isolate), count_(count), byte_offset_(0), index_(index) { + CHECK_GT(count, 0); + if (index != nullptr) { + // Will be deserialized later. + return; + } + const v8::HandleScope handle_scope(isolate_); + const size_t size_in_bytes = + MultiplyWithOverflowCheck(sizeof(NativeT), count); + + // allocate v8 ArrayBuffer + v8::Local ab = v8::ArrayBuffer::New(isolate_, size_in_bytes); + buffer_ = static_cast(ab->Data()); + + // allocate v8 TypedArray + v8::Local js_array = V8T::New(ab, byte_offset_, count); + js_array_ = v8::Global(isolate, js_array); +} + +template +AliasedBufferBase::AliasedBufferBase( + v8::Isolate* isolate, + const size_t byte_offset, + const size_t count, + const AliasedBufferBase& backing_buffer, + const AliasedBufferIndex* index) + : isolate_(isolate), + count_(count), + byte_offset_(byte_offset), + index_(index) { + if (index != nullptr) { + // Will be deserialized later. + return; + } + const v8::HandleScope handle_scope(isolate_); + v8::Local ab = backing_buffer.GetArrayBuffer(); + + // validate that the byte_offset is aligned with sizeof(NativeT) + CHECK_EQ(byte_offset & (sizeof(NativeT) - 1), 0); + // validate this fits inside the backing buffer + CHECK_LE(MultiplyWithOverflowCheck(sizeof(NativeT), count), + ab->ByteLength() - byte_offset); + + buffer_ = reinterpret_cast( + const_cast(backing_buffer.GetNativeBuffer() + byte_offset)); + + v8::Local js_array = V8T::New(ab, byte_offset, count); + js_array_ = v8::Global(isolate, js_array); +} + +template +AliasedBufferBase::AliasedBufferBase( + const AliasedBufferBase& that) + : isolate_(that.isolate_), + count_(that.count_), + byte_offset_(that.byte_offset_), + buffer_(that.buffer_) { + DCHECK_NULL(index_); + js_array_ = v8::Global(that.isolate_, that.GetJSArray()); +} + +template +AliasedBufferIndex AliasedBufferBase::Serialize( + v8::Local context, v8::SnapshotCreator* creator) { + DCHECK_NULL(index_); + return creator->AddData(context, GetJSArray()); +} + +template +inline void AliasedBufferBase::Deserialize( + v8::Local context) { + DCHECK_NOT_NULL(index_); + v8::Local arr = + context->GetDataFromSnapshotOnce(*index_).ToLocalChecked(); + // These may not hold true for AliasedBuffers that have grown, so should + // be removed when we expand the snapshot support. + DCHECK_EQ(count_, arr->Length()); + DCHECK_EQ(byte_offset_, arr->ByteOffset()); + uint8_t* raw = static_cast(arr->Buffer()->Data()); + buffer_ = reinterpret_cast(raw + byte_offset_); + js_array_.Reset(isolate_, arr); + index_ = nullptr; +} + +template +AliasedBufferBase& AliasedBufferBase::operator=( + AliasedBufferBase&& that) noexcept { + DCHECK_NULL(index_); + this->~AliasedBufferBase(); + isolate_ = that.isolate_; + count_ = that.count_; + byte_offset_ = that.byte_offset_; + buffer_ = that.buffer_; + + js_array_.Reset(isolate_, that.js_array_.Get(isolate_)); + + that.buffer_ = nullptr; + that.js_array_.Reset(); + return *this; +} + +template +v8::Local AliasedBufferBase::GetJSArray() const { + DCHECK_NULL(index_); + return js_array_.Get(isolate_); +} + +template +void AliasedBufferBase::Release() { + DCHECK_NULL(index_); + js_array_.Reset(); +} + +template +v8::Local AliasedBufferBase::GetArrayBuffer() + const { + return GetJSArray()->Buffer(); +} + +template +inline const NativeT* AliasedBufferBase::GetNativeBuffer() const { + DCHECK_NULL(index_); + return buffer_; +} + +template +inline const NativeT* AliasedBufferBase::operator*() const { + return GetNativeBuffer(); +} + +template +inline void AliasedBufferBase::SetValue(const size_t index, + NativeT value) { + DCHECK_LT(index, count_); + DCHECK_NULL(index_); + buffer_[index] = value; +} + +template +inline const NativeT AliasedBufferBase::GetValue( + const size_t index) const { + DCHECK_NULL(index_); + DCHECK_LT(index, count_); + return buffer_[index]; +} + +template +typename AliasedBufferBase::Reference +AliasedBufferBase::operator[](size_t index) { + DCHECK_NULL(index_); + return Reference(this, index); +} + +template +NativeT AliasedBufferBase::operator[](size_t index) const { + return GetValue(index); +} + +template +size_t AliasedBufferBase::Length() const { + return count_; +} + +template +void AliasedBufferBase::reserve(size_t new_capacity) { + DCHECK_NULL(index_); + DCHECK_GE(new_capacity, count_); + DCHECK_EQ(byte_offset_, 0); + const v8::HandleScope handle_scope(isolate_); + + const size_t old_size_in_bytes = sizeof(NativeT) * count_; + const size_t new_size_in_bytes = + MultiplyWithOverflowCheck(sizeof(NativeT), new_capacity); + + // allocate v8 new ArrayBuffer + v8::Local ab = + v8::ArrayBuffer::New(isolate_, new_size_in_bytes); + + // allocate new native buffer + NativeT* new_buffer = static_cast(ab->Data()); + // copy old content + memcpy(new_buffer, buffer_, old_size_in_bytes); + + // allocate v8 TypedArray + v8::Local js_array = V8T::New(ab, byte_offset_, new_capacity); + + // move over old v8 TypedArray + js_array_ = std::move(v8::Global(isolate_, js_array)); + + buffer_ = new_buffer; + count_ = new_capacity; +} + +} // namespace node + +#endif // defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS + +#endif // SRC_ALIASED_BUFFER_INL_H_ diff --git a/src/aliased_buffer.h b/src/aliased_buffer.h index 98ea2d31febce2..1b9336e1cd3397 100644 --- a/src/aliased_buffer.h +++ b/src/aliased_buffer.h @@ -4,7 +4,6 @@ #if defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS #include -#include "util-inl.h" #include "v8.h" namespace node { @@ -28,34 +27,14 @@ typedef size_t AliasedBufferIndex; * The encapsulation herein provides a placeholder where such writes can be * observed. Any notification APIs will be left as a future exercise. */ -template ::value>> +template class AliasedBufferBase { public: + static_assert(std::is_scalar::value); + AliasedBufferBase(v8::Isolate* isolate, const size_t count, - const AliasedBufferIndex* index = nullptr) - : isolate_(isolate), count_(count), byte_offset_(0), index_(index) { - CHECK_GT(count, 0); - if (index != nullptr) { - // Will be deserialized later. - return; - } - const v8::HandleScope handle_scope(isolate_); - const size_t size_in_bytes = - MultiplyWithOverflowCheck(sizeof(NativeT), count); - - // allocate v8 ArrayBuffer - v8::Local ab = v8::ArrayBuffer::New( - isolate_, size_in_bytes); - buffer_ = static_cast(ab->Data()); - - // allocate v8 TypedArray - v8::Local js_array = V8T::New(ab, byte_offset_, count); - js_array_ = v8::Global(isolate, js_array); - } + const AliasedBufferIndex* index = nullptr); /** * Create an AliasedBufferBase over a sub-region of another aliased buffer. @@ -71,74 +50,16 @@ class AliasedBufferBase { const size_t byte_offset, const size_t count, const AliasedBufferBase& backing_buffer, - const AliasedBufferIndex* index = nullptr) - : isolate_(isolate), - count_(count), - byte_offset_(byte_offset), - index_(index) { - if (index != nullptr) { - // Will be deserialized later. - return; - } - const v8::HandleScope handle_scope(isolate_); - v8::Local ab = backing_buffer.GetArrayBuffer(); - - // validate that the byte_offset is aligned with sizeof(NativeT) - CHECK_EQ(byte_offset & (sizeof(NativeT) - 1), 0); - // validate this fits inside the backing buffer - CHECK_LE(MultiplyWithOverflowCheck(sizeof(NativeT), count), - ab->ByteLength() - byte_offset); - - buffer_ = reinterpret_cast( - const_cast(backing_buffer.GetNativeBuffer() + byte_offset)); - - v8::Local js_array = V8T::New(ab, byte_offset, count); - js_array_ = v8::Global(isolate, js_array); - } - - AliasedBufferBase(const AliasedBufferBase& that) - : isolate_(that.isolate_), - count_(that.count_), - byte_offset_(that.byte_offset_), - buffer_(that.buffer_) { - DCHECK_NULL(index_); - js_array_ = v8::Global(that.isolate_, that.GetJSArray()); - } + const AliasedBufferIndex* index = nullptr); + + AliasedBufferBase(const AliasedBufferBase& that); AliasedBufferIndex Serialize(v8::Local context, - v8::SnapshotCreator* creator) { - DCHECK_NULL(index_); - return creator->AddData(context, GetJSArray()); - } - - inline void Deserialize(v8::Local context) { - DCHECK_NOT_NULL(index_); - v8::Local arr = - context->GetDataFromSnapshotOnce(*index_).ToLocalChecked(); - // These may not hold true for AliasedBuffers that have grown, so should - // be removed when we expand the snapshot support. - DCHECK_EQ(count_, arr->Length()); - DCHECK_EQ(byte_offset_, arr->ByteOffset()); - uint8_t* raw = static_cast(arr->Buffer()->Data()); - buffer_ = reinterpret_cast(raw + byte_offset_); - js_array_.Reset(isolate_, arr); - index_ = nullptr; - } - - AliasedBufferBase& operator=(AliasedBufferBase&& that) noexcept { - DCHECK_NULL(index_); - this->~AliasedBufferBase(); - isolate_ = that.isolate_; - count_ = that.count_; - byte_offset_ = that.byte_offset_; - buffer_ = that.buffer_; - - js_array_.Reset(isolate_, that.js_array_.Get(isolate_)); - - that.buffer_ = nullptr; - that.js_array_.Reset(); - return *this; - } + v8::SnapshotCreator* creator); + + inline void Deserialize(v8::Local context); + + AliasedBufferBase& operator=(AliasedBufferBase&& that) noexcept; /** * Helper class that is returned from operator[] to support assignment into @@ -191,105 +112,50 @@ class AliasedBufferBase { /** * Get the underlying v8 TypedArray overlayed on top of the native buffer */ - v8::Local GetJSArray() const { - DCHECK_NULL(index_); - return js_array_.Get(isolate_); - } + v8::Local GetJSArray() const; - void Release() { - DCHECK_NULL(index_); - js_array_.Reset(); - } + void Release(); /** * Get the underlying v8::ArrayBuffer underlying the TypedArray and * overlaying the native buffer */ - v8::Local GetArrayBuffer() const { - return GetJSArray()->Buffer(); - } + v8::Local GetArrayBuffer() const; /** * Get the underlying native buffer. Note that all reads/writes should occur * through the GetValue/SetValue/operator[] methods */ - inline const NativeT* GetNativeBuffer() const { - DCHECK_NULL(index_); - return buffer_; - } + inline const NativeT* GetNativeBuffer() const; /** * Synonym for GetBuffer() */ - inline const NativeT* operator * () const { - return GetNativeBuffer(); - } + inline const NativeT* operator*() const; /** * Set position index to given value. */ - inline void SetValue(const size_t index, NativeT value) { - DCHECK_LT(index, count_); - DCHECK_NULL(index_); - buffer_[index] = value; - } + inline void SetValue(const size_t index, NativeT value); /** * Get value at position index */ - inline const NativeT GetValue(const size_t index) const { - DCHECK_NULL(index_); - DCHECK_LT(index, count_); - return buffer_[index]; - } + inline const NativeT GetValue(const size_t index) const; /** * Effectively, a synonym for GetValue/SetValue */ - Reference operator[](size_t index) { - DCHECK_NULL(index_); - return Reference(this, index); - } + Reference operator[](size_t index); - NativeT operator[](size_t index) const { - return GetValue(index); - } + NativeT operator[](size_t index) const; - size_t Length() const { - return count_; - } + size_t Length() const; // Should only be used to extend the array. // Should only be used on an owning array, not one created as a sub array of // an owning `AliasedBufferBase`. - void reserve(size_t new_capacity) { - DCHECK_NULL(index_); - DCHECK_GE(new_capacity, count_); - DCHECK_EQ(byte_offset_, 0); - const v8::HandleScope handle_scope(isolate_); - - const size_t old_size_in_bytes = sizeof(NativeT) * count_; - const size_t new_size_in_bytes = MultiplyWithOverflowCheck(sizeof(NativeT), - new_capacity); - - // allocate v8 new ArrayBuffer - v8::Local ab = v8::ArrayBuffer::New( - isolate_, new_size_in_bytes); - - // allocate new native buffer - NativeT* new_buffer = static_cast(ab->Data()); - // copy old content - memcpy(new_buffer, buffer_, old_size_in_bytes); - - // allocate v8 TypedArray - v8::Local js_array = V8T::New(ab, byte_offset_, new_capacity); - - // move over old v8 TypedArray - js_array_ = std::move(v8::Global(isolate_, js_array)); - - buffer_ = new_buffer; - count_ = new_capacity; - } + void reserve(size_t new_capacity); private: v8::Isolate* isolate_ = nullptr; @@ -302,11 +168,22 @@ class AliasedBufferBase { const AliasedBufferIndex* index_ = nullptr; }; -typedef AliasedBufferBase AliasedInt32Array; -typedef AliasedBufferBase AliasedUint8Array; -typedef AliasedBufferBase AliasedUint32Array; -typedef AliasedBufferBase AliasedFloat64Array; -typedef AliasedBufferBase AliasedBigInt64Array; +#define ALIASED_BUFFER_LIST(V) \ + V(int8_t, Int8Array) \ + V(uint8_t, Uint8Array) \ + V(int16_t, Int16Array) \ + V(uint16_t, Uint16Array) \ + V(int32_t, Int32Array) \ + V(uint32_t, Uint32Array) \ + V(float, Float32Array) \ + V(double, Float64Array) \ + V(int64_t, BigInt64Array) + +#define V(NativeT, V8T) \ + typedef AliasedBufferBase Aliased##V8T; +ALIASED_BUFFER_LIST(V) +#undef V + } // namespace node #endif // defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS diff --git a/src/env-inl.h b/src/env-inl.h index 6b4d63616abe39..1d7a502b35aafd 100644 --- a/src/env-inl.h +++ b/src/env-inl.h @@ -24,7 +24,7 @@ #if defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS -#include "aliased_buffer.h" +#include "aliased_buffer-inl.h" #include "callback_queue-inl.h" #include "env.h" #include "node.h" diff --git a/src/memory_tracker-inl.h b/src/memory_tracker-inl.h index a4970d9392faa7..b8d2667d981c9d 100644 --- a/src/memory_tracker-inl.h +++ b/src/memory_tracker-inl.h @@ -3,6 +3,7 @@ #if defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS +#include "aliased_buffer-inl.h" #include "memory_tracker.h" namespace node { diff --git a/src/memory_tracker.h b/src/memory_tracker.h index 4682f25916e52d..ae3b00317906f5 100644 --- a/src/memory_tracker.h +++ b/src/memory_tracker.h @@ -13,8 +13,15 @@ #include #include +namespace v8 { +class BackingStore; +} + namespace node { +template +struct MallocedBuffer; + // Set the node name of a MemoryRetainer to klass #define SET_MEMORY_INFO_NAME(Klass) \ inline const char* MemoryInfoName() const override { return #Klass; } diff --git a/src/node_file.cc b/src/node_file.cc index c6d22b51f255b5..de623b598db348 100644 --- a/src/node_file.cc +++ b/src/node_file.cc @@ -19,7 +19,7 @@ // OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE // USE OR OTHER DEALINGS IN THE SOFTWARE. #include "node_file.h" // NOLINT(build/include_inline) -#include "aliased_buffer.h" +#include "aliased_buffer-inl.h" #include "memory_tracker-inl.h" #include "node_buffer.h" #include "node_external_reference.h" diff --git a/src/node_http2.cc b/src/node_http2.cc index 7e8e04f440ae85..049b71cb54e58a 100644 --- a/src/node_http2.cc +++ b/src/node_http2.cc @@ -1,5 +1,5 @@ #include "node_http2.h" -#include "aliased_buffer.h" +#include "aliased_buffer-inl.h" #include "aliased_struct-inl.h" #include "debug_utils-inl.h" #include "histogram-inl.h" diff --git a/src/node_perf.cc b/src/node_perf.cc index edc578d1b89ea7..8e8173e16ca7fa 100644 --- a/src/node_perf.cc +++ b/src/node_perf.cc @@ -1,5 +1,5 @@ #include "node_perf.h" -#include "aliased_buffer.h" +#include "aliased_buffer-inl.h" #include "env-inl.h" #include "histogram-inl.h" #include "memory_tracker-inl.h" diff --git a/src/node_v8.cc b/src/node_v8.cc index f4900328cfe553..82ae6caa952be7 100644 --- a/src/node_v8.cc +++ b/src/node_v8.cc @@ -20,6 +20,7 @@ // USE OR OTHER DEALINGS IN THE SOFTWARE. #include "node_v8.h" +#include "aliased_buffer-inl.h" #include "base_object-inl.h" #include "env-inl.h" #include "memory_tracker-inl.h" diff --git a/test/cctest/test_aliased_buffer.cc b/test/cctest/test_aliased_buffer.cc index ba947700c1bf27..e09e1d23c099fd 100644 --- a/test/cctest/test_aliased_buffer.cc +++ b/test/cctest/test_aliased_buffer.cc @@ -1,6 +1,7 @@ -#include "v8.h" +#include "aliased_buffer-inl.h" #include "aliased_buffer.h" #include "node_test_fixture.h" +#include "v8.h" using node::AliasedBufferBase; From 1e7b4446d9dc8e4cd1f1a447cf3d62a64d105474 Mon Sep 17 00:00:00 2001 From: Joyee Cheung Date: Fri, 24 Feb 2023 15:51:07 +0100 Subject: [PATCH 3/5] 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. --- src/aliased_buffer-inl.h | 19 +++++++++++++++++++ src/aliased_buffer.h | 8 +++++++- src/memory_tracker-inl.h | 9 +-------- src/memory_tracker.h | 5 ----- 4 files changed, 27 insertions(+), 14 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. From 832a61dc644cffd205f6d512f15012c9ae8103d9 Mon Sep 17 00:00:00 2001 From: Joyee Cheung Date: Tue, 28 Feb 2023 00:28:19 +0100 Subject: [PATCH 4/5] fixup! src: fix AliasedBuffer memory attribution in heap snapshots --- test/pummel/test-heapdump-fs-promise.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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' }, ], }, ]); From 5cdd9e1db154fcbf47b6ea9f9045166d5d4d0e9e Mon Sep 17 00:00:00 2001 From: Joyee Cheung Date: Tue, 28 Feb 2023 14:57:46 +0100 Subject: [PATCH 5/5] fixup! src: move AliasedBuffer implementation to -inl.h --- test/cctest/test_aliased_buffer.cc | 1 - 1 file changed, 1 deletion(-) diff --git a/test/cctest/test_aliased_buffer.cc b/test/cctest/test_aliased_buffer.cc index e09e1d23c099fd..6d62444d42c7cc 100644 --- a/test/cctest/test_aliased_buffer.cc +++ b/test/cctest/test_aliased_buffer.cc @@ -1,5 +1,4 @@ #include "aliased_buffer-inl.h" -#include "aliased_buffer.h" #include "node_test_fixture.h" #include "v8.h"