From 81203f66b4ad962ba358e387b09a95419c7e73c1 Mon Sep 17 00:00:00 2001 From: Joyee Cheung Date: Sat, 30 Mar 2019 21:09:22 -0400 Subject: [PATCH] fixup! src: implement MemoryRetainer in Environment --- src/base_object.h | 2 +- src/env-inl.h | 6 ++-- src/env.cc | 23 +++++++++++---- src/env.h | 66 ++++++++++++++++++++------------------------ src/memory_tracker.h | 9 ++++++ 5 files changed, 60 insertions(+), 46 deletions(-) diff --git a/src/base_object.h b/src/base_object.h index 8bbb2f279dc0cf..f1c666224f0401 100644 --- a/src/base_object.h +++ b/src/base_object.h @@ -94,7 +94,7 @@ class BaseObject : public MemoryRetainer { // position of members in memory are predictable. For more information please // refer to `doc/guides/node-postmortem-support.md` friend int GenDebugSymbols(); - friend class Environment; + friend class CleanupHookCallback; Persistent persistent_handle_; Environment* env_; diff --git a/src/env-inl.h b/src/env-inl.h index cb721717f77981..ec966643ddf4a8 100644 --- a/src/env-inl.h +++ b/src/env-inl.h @@ -976,17 +976,17 @@ void Environment::RemoveCleanupHook(void (*fn)(void*), void* arg) { cleanup_hooks_.erase(search); } -size_t Environment::CleanupHookCallback::Hash::operator()( +size_t CleanupHookCallback::Hash::operator()( const CleanupHookCallback& cb) const { return std::hash()(cb.arg_); } -bool Environment::CleanupHookCallback::Equal::operator()( +bool CleanupHookCallback::Equal::operator()( const CleanupHookCallback& a, const CleanupHookCallback& b) const { return a.fn_ == b.fn_ && a.arg_ == b.arg_; } -BaseObject* Environment::CleanupHookCallback::GetBaseObject() const { +BaseObject* CleanupHookCallback::GetBaseObject() const { if (fn_ == BaseObject::DeleteMe) return static_cast(arg_); else diff --git a/src/env.cc b/src/env.cc index c8ec76543021bb..36c420551d6e0e 100644 --- a/src/env.cc +++ b/src/env.cc @@ -123,11 +123,12 @@ void IsolateData::MemoryInfo(MemoryTracker* tracker) const { PER_ISOLATE_STRING_PROPERTIES(V) #undef V - tracker->TrackFieldWithSize( - "allocator", sizeof(*allocator_), "v8::ArrayBuffer::Allocator"); if (node_allocator_ != nullptr) { tracker->TrackFieldWithSize( "node_allocator", sizeof(*node_allocator_), "NodeArrayBufferAllocator"); + } else { + tracker->TrackFieldWithSize( + "allocator", sizeof(*allocator_), "v8::ArrayBuffer::Allocator"); } tracker->TrackFieldWithSize( "platform", sizeof(*platform_), "MultiIsolatePlatform"); @@ -852,15 +853,25 @@ void Environment::stop_sub_worker_contexts() { } } -void Environment::CleanupHookCallback::MemoryInfo( - MemoryTracker* tracker) const { +void MemoryTracker::TrackField(const char* edge_name, + const CleanupHookCallback& value, + const char* node_name) { + v8::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 = GetBaseObject(); + BaseObject* obj = value.GetBaseObject(); if (obj != nullptr) { - tracker->TrackField("arg", obj); + this->TrackField("arg", obj); } + CHECK_EQ(CurrentNode(), n); + CHECK_NE(n->size_, 0); + PopNode(); } void Environment::BuildEmbedderGraph(Isolate* isolate, diff --git a/src/env.h b/src/env.h index 131d805852093c..5f54f1ecff253b 100644 --- a/src/env.h +++ b/src/env.h @@ -739,6 +739,36 @@ class ShouldNotAbortOnUncaughtScope { Environment* env_; }; +class CleanupHookCallback { + public: + CleanupHookCallback(void (*fn)(void*), + void* arg, + uint64_t insertion_order_counter) + : fn_(fn), arg_(arg), insertion_order_counter_(insertion_order_counter) {} + + // Only hashes `arg_`, since that is usually enough to identify the hook. + struct Hash { + inline size_t operator()(const CleanupHookCallback& cb) const; + }; + + // Compares by `fn_` and `arg_` being equal. + struct Equal { + inline bool operator()(const CleanupHookCallback& a, + const CleanupHookCallback& b) const; + }; + + inline BaseObject* GetBaseObject() const; + + private: + friend class Environment; + void (*fn_)(void*); + void* arg_; + + // We keep track of the insertion order for these objects, so that we can + // call the callbacks in reverse order when we are cleaning up. + uint64_t insertion_order_counter_; +}; + class Environment : public MemoryRetainer { public: Environment(const Environment&) = delete; @@ -1224,42 +1254,6 @@ class Environment : public MemoryRetainer { void RunAndClearNativeImmediates(); static void CheckImmediate(uv_check_t* handle); - class CleanupHookCallback : public MemoryRetainer { - public: - CleanupHookCallback(void (*fn)(void*), - void* arg, - uint64_t insertion_order_counter) - : fn_(fn), - arg_(arg), - insertion_order_counter_(insertion_order_counter) {} - - // Only hashes `arg_`, since that is usually enough to identify the hook. - struct Hash { - inline size_t operator()(const CleanupHookCallback& cb) const; - }; - - // Compares by `fn_` and `arg_` being equal. - struct Equal { - inline bool operator()(const CleanupHookCallback& a, - const CleanupHookCallback& b) const; - }; - - SET_MEMORY_INFO_NAME(CleanupHookCallback); - SET_SELF_SIZE(CleanupHookCallback); - void MemoryInfo(MemoryTracker* tracker) const override; - - inline BaseObject* GetBaseObject() const; - - private: - friend class Environment; - void (*fn_)(void*); - void* arg_; - - // We keep track of the insertion order for these objects, so that we can - // call the callbacks in reverse order when we are cleaning up. - uint64_t insertion_order_counter_; - }; - // Use an unordered_set, so that we have efficient insertion and removal. std::unordered_set& 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);