src: refactor memory tracker implementation

Use a template class instead of virtual methods to allow inlining
the subclass methods, as well as other minor cleanups.

PR-URL: #126
Reviewed-By: Daniel Bevenius <[email protected]>
Reviewed-By: James M Snell <[email protected]>

Author: addaleax

node.gyp

@@ -622,6 +622,7 @@
         'src/node_internals.h',
         'src/node_main_instance.h',
         'src/node_mem.h',
+        'src/node_mem-inl.h',
         'src/node_messaging.h',
         'src/node_metadata.h',
         'src/node_mutex.h',

src/node_mem-inl.h

@@ -0,0 +1,112 @@
+#ifndef SRC_NODE_MEM_INL_H_
+#define SRC_NODE_MEM_INL_H_
+
+#if defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS
+
+#include "node_mem.h"
+#include "node_internals.h"
+
+namespace node {
+namespace mem {
+
+template <typename Class, typename AllocatorStruct>
+AllocatorStruct NgLibMemoryManager<Class, AllocatorStruct>::MakeAllocator() {
+  return AllocatorStruct {
+    static_cast<void*>(static_cast<Class*>(this)),
+    MallocImpl,
+    FreeImpl,
+    CallocImpl,
+    ReallocImpl
+  };
+}
+
+template <typename Class, typename T>
+void* NgLibMemoryManager<Class, T>::ReallocImpl(void* ptr,
+                                             size_t size,
+                                             void* user_data) {
+  Class* manager = static_cast<Class*>(user_data);
+
+  size_t previous_size = 0;
+  char* original_ptr = nullptr;
+
+  // We prepend each allocated buffer with a size_t containing the full
+  // size of the allocation.
+  if (size > 0) size += sizeof(size_t);
+
+  if (ptr != nullptr) {
+    // We are free()ing or re-allocating.
+    original_ptr = static_cast<char*>(ptr) - sizeof(size_t);
+    previous_size = *reinterpret_cast<size_t*>(original_ptr);
+    // This means we called StopTracking() on this pointer before.
+    if (previous_size == 0) {
+      // Fall back to the standard Realloc() function.
+      char* ret = UncheckedRealloc(original_ptr, size);
+      if (ret != nullptr)
+        ret += sizeof(size_t);
+      return ret;
+    }
+  }
+
+  manager->CheckAllocatedSize(previous_size);
+
+  char* mem = UncheckedRealloc(original_ptr, size);
+
+  if (mem != nullptr) {
+    // Adjust the memory info counter.
+    // TODO(addaleax): Avoid the double bookkeeping we do with
+    // current_nghttp2_memory_ + AdjustAmountOfExternalAllocatedMemory
+    // and provide versions of our memory allocation utilities that take an
+    // Environment*/Isolate* parameter and call the V8 method transparently.
+    const int64_t new_size = size - previous_size;
+    manager->IncreaseAllocatedSize(new_size);
+    manager->env()->isolate()->AdjustAmountOfExternalAllocatedMemory(
+        new_size);
+    *reinterpret_cast<size_t*>(mem) = size;
+    mem += sizeof(size_t);
+  } else if (size == 0) {
+    manager->DecreaseAllocatedSize(previous_size);
+    manager->env()->isolate()->AdjustAmountOfExternalAllocatedMemory(
+        -static_cast<int64_t>(previous_size));
+  }
+  return mem;
+}
+
+template <typename Class, typename T>
+void* NgLibMemoryManager<Class, T>::MallocImpl(size_t size, void* user_data) {
+  return ReallocImpl(nullptr, size, user_data);
+}
+
+template <typename Class, typename T>
+void NgLibMemoryManager<Class, T>::FreeImpl(void* ptr, void* user_data) {
+  if (ptr == nullptr) return;
+  CHECK_NULL(ReallocImpl(ptr, 0, user_data));
+}
+
+template <typename Class, typename T>
+void* NgLibMemoryManager<Class, T>::CallocImpl(size_t nmemb,
+                                            size_t size,
+                                            void* user_data) {
+  size_t real_size = MultiplyWithOverflowCheck(nmemb, size);
+  void* mem = MallocImpl(real_size, user_data);
+  if (mem != nullptr)
+    memset(mem, 0, real_size);
+  return mem;
+}
+
+template <typename Class, typename T>
+void NgLibMemoryManager<Class, T>::StopTrackingMemory(void* ptr) {
+  size_t* original_ptr = reinterpret_cast<size_t*>(
+      static_cast<char*>(ptr) - sizeof(size_t));
+  Class* manager = static_cast<Class*>(this);
+  manager->DecreaseAllocatedSize(*original_ptr);
+  manager->env()->isolate()->AdjustAmountOfExternalAllocatedMemory(
+      -static_cast<int64_t>(*original_ptr));
+  *original_ptr = 0;
+}
+
+}  // namespace mem
+}  // namespace node
+
+#endif  // defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS
+
+#endif  // SRC_NODE_MEM_INL_H_

src/node_mem.h

@@ -3,105 +3,34 @@
 
 #if defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS
 
-#include "node_internals.h"
+#include <stddef.h>
 
 namespace node {
 namespace mem {
 
 // Both ngtcp2 and nghttp2 allow custom allocators that
 // follow exactly the same structure and behavior, but
 // use different struct names. To allow for code re-use,
-// the Allocator template class can be used for both.
+// the NgLibMemoryManager template class can be used for both.
 
-struct Tracker {
-  virtual void CheckAllocatedSize(size_t previous_size) = 0;
-  virtual void IncrementAllocatedSize(size_t size) = 0;
-  virtual void DecrementAllocatedSize(size_t size) = 0;
-};
-
-inline static void* MemRealloc(
-    void* ptr,
-    size_t size,
-    void* user_data) {
-
-  Tracker* tracker = static_cast<Tracker*>(user_data);
-
-  size_t previous_size = 0;
-  char* original_ptr = nullptr;
-
-  if (size > 0) size += sizeof(size_t);
-
-  if (ptr != nullptr) {
-    original_ptr = static_cast<char*>(ptr) - sizeof(size_t);
-    previous_size = *reinterpret_cast<size_t*>(original_ptr);
-    if (previous_size == 0) {
-      char* ret = UncheckedRealloc(original_ptr, size);
-      if (ret != nullptr)
-        ret += sizeof(size_t);
-      return ret;
-    }
-  }
-
-  tracker->CheckAllocatedSize(previous_size);
-
-  char* mem = UncheckedRealloc(original_ptr, size);
-
-  if (mem != nullptr) {
-    tracker->IncrementAllocatedSize(size - previous_size);
-    *reinterpret_cast<size_t*>(mem) = size;
-    mem += sizeof(size_t);
-  } else if (size == 0) {
-    tracker->DecrementAllocatedSize(previous_size);
-  }
-  return mem;
-}
-
-inline static void* MemMalloc(
-    size_t size,
-    void* user_data) {
-  return MemRealloc(nullptr, size, user_data);
-}
-
-inline static void MemFree(
-    void* ptr,
-    void* user_data) {
-  if (ptr == nullptr) return;
-  CHECK_NULL(MemRealloc(ptr, 0, user_data));
-}
-
-inline static void* MemCalloc(
-    size_t nmemb,
-    size_t size,
-    void* user_data) {
-  size_t real_size = MultiplyWithOverflowCheck(nmemb, size);
-  void* mem = MemMalloc(real_size, user_data);
-  if (mem != nullptr)
-    memset(mem, 0, real_size);
-  return mem;
-}
+template <typename Class, typename AllocatorStructName>
+class NgLibMemoryManager {
+ public:
+  // Class needs to provide these methods:
+  // void CheckAllocatedSize(size_t previous_size) const;
+  // void IncreaseAllocatedSize(size_t size);
+  // void DecreaseAllocatedSize(size_t size);
+  // Environment* env() const;
 
-inline static void MemStopTracking(Tracker* tracker, void* ptr) {
-  size_t* original_ptr = reinterpret_cast<size_t*>(
-      static_cast<char*>(ptr) - sizeof(size_t));
-  tracker->DecrementAllocatedSize(*original_ptr);
-  *original_ptr = 0;
-}
+  AllocatorStructName MakeAllocator();
 
-template <typename T>
-class Allocator {
- public:
-  explicit inline Allocator(Tracker* user_data) :
-    info_({
-      user_data,
-      MemMalloc,
-      MemFree,
-      MemCalloc,
-      MemRealloc
-    }) {}
+  void StopTrackingMemory(void* ptr);
 
-  inline T* operator*() { return &info_; }
  private:
-  T info_;
+  static void* ReallocImpl(void* ptr, size_t size, void* user_data);
+  static void* MallocImpl(size_t size, void* user_data);
+  static void FreeImpl(void* ptr, void* user_data);
+  static void* CallocImpl(size_t nmemb, size_t size, void* user_data);
 };
 
 }  // namespace mem

src/node_quic_session-inl.h

@@ -11,15 +11,15 @@ namespace node {
 
 namespace quic {
 
-void QuicSession::CheckAllocatedSize(size_t previous_size) {
+void QuicSession::CheckAllocatedSize(size_t previous_size) const {
   CHECK_GE(current_ngtcp2_memory_, previous_size);
 }
 
-void QuicSession::IncrementAllocatedSize(size_t size) {
+void QuicSession::IncreaseAllocatedSize(size_t size) {
   current_ngtcp2_memory_ += size;
 }
 
-void QuicSession::DecrementAllocatedSize(size_t size) {
+void QuicSession::DecreaseAllocatedSize(size_t size) {
   current_ngtcp2_memory_ -= size;
 }
 

src/node_quic_session.cc

@@ -6,6 +6,7 @@
 #include "node_buffer.h"
 #include "node_crypto.h"
 #include "node_internals.h"
+#include "node_mem-inl.h"
 #include "node_quic_crypto.h"
 #include "node_quic_session.h"  // NOLINT(build/include_inline)
 #include "node_quic_session-inl.h"
@@ -182,7 +183,7 @@ QuicSession::QuicSession(
     idle_(new Timer(socket->env(), [this]() { OnIdleTimeout(); })),
     retransmit_(new Timer(socket->env(), [this]() { MaybeTimeout(); })),
     state_(env()->isolate(), IDX_QUIC_SESSION_STATE_COUNT),
-    allocator_(this),
+    alloc_info_(MakeAllocator()),
     crypto_rx_ack_(
         HistogramBase::New(
             socket->env(),
@@ -2013,7 +2014,7 @@ void QuicServerSession::Init(
           version,
           &callbacks,
           *cfg,
-          *allocator_,
+          &alloc_info_,
           static_cast<QuicSession*>(this)), 0);
 
   if (ocid)
@@ -2543,7 +2544,7 @@ bool QuicClientSession::Init(
           version,
           &callbacks,
           *config,
-          *allocator_,
+          &alloc_info_,
           static_cast<QuicSession*>(this)), 0);
 
   connection_.reset(conn);

src/node_quic_session.h

@@ -183,7 +183,7 @@ enum QuicSessionState : int {
 // a QuicSession object.
 class QuicSession : public AsyncWrap,
                     public std::enable_shared_from_this<QuicSession>,
-                    public mem::Tracker {
+                    public mem::NgLibMemoryManager<QuicSession, ngtcp2_mem> {
  public:
   static const int kInitialClientBufferLength = 4096;
 
@@ -338,10 +338,10 @@ class QuicSession : public AsyncWrap,
   virtual bool SendConnectionClose() = 0;
   virtual int TLSHandshake_Initial() = 0;
 
-  // Implementation for mem::Tracker
-  inline void CheckAllocatedSize(size_t previous_size) override;
-  inline void IncrementAllocatedSize(size_t size) override;
-  inline void DecrementAllocatedSize(size_t size) override;
+  // Implementation for mem::NgLibMemoryManager
+  inline void CheckAllocatedSize(size_t previous_size) const;
+  inline void IncreaseAllocatedSize(size_t size);
+  inline void DecreaseAllocatedSize(size_t size);
 
   // Tracks whether or not we are currently within an ngtcp2 callback
   // function. Certain ngtcp2 APIs are not supposed to be called when
@@ -876,7 +876,7 @@ class QuicSession : public AsyncWrap,
 
   AliasedFloat64Array state_;
 
-  mem::Allocator<ngtcp2_mem> allocator_;
+  ngtcp2_mem alloc_info_;
 
   struct session_stats {
     // The timestamp at which the session was created

src/node_quic_socket.cc

@@ -5,6 +5,7 @@
 #include "node.h"
 #include "node_crypto.h"
 #include "node_internals.h"
+#include "node_mem-inl.h"
 #include "node_quic_crypto.h"
 #include "node_quic_session-inl.h"
 #include "node_quic_socket.h"
@@ -83,7 +84,8 @@ QuicSocket::QuicSocket(
     stats_buffer_(
       env->isolate(),
       sizeof(socket_stats_) / sizeof(uint64_t),
-      reinterpret_cast<uint64_t*>(&socket_stats_)) {
+      reinterpret_cast<uint64_t*>(&socket_stats_)),
+    alloc_info_(MakeAllocator()) {
   CHECK_EQ(uv_udp_init(env->event_loop(), &handle_), 0);
   Debug(this, "New QuicSocket created.");
 
@@ -486,8 +488,6 @@ void QuicSocket::SendInitialConnectionClose(
   ngtcp2_settings settings;
   ngtcp2_settings_default(&settings);
 
-  mem::Allocator<ngtcp2_mem> allocator(this);
-
   ngtcp2_conn* conn;
   ngtcp2_conn_server_new(
     &conn,
@@ -497,7 +497,7 @@ void QuicSocket::SendInitialConnectionClose(
     version,
     &callbacks,
     &settings,
-    *allocator,
+    &alloc_info_,
     nullptr);
 
   SendWrapStack* req =
@@ -1011,19 +1011,18 @@ void QuicSocket::SetDiagnosticPacketLoss(double rx, double tx) {
   tx_loss_ = tx;
 }
 
-inline void QuicSocket::CheckAllocatedSize(size_t previous_size) {
+void QuicSocket::CheckAllocatedSize(size_t previous_size) const {
   CHECK_GE(current_ngtcp2_memory_, previous_size);
 }
 
-inline void QuicSocket::IncrementAllocatedSize(size_t size) {
+void QuicSocket::IncreaseAllocatedSize(size_t size) {
   current_ngtcp2_memory_ += size;
 }
 
-inline void QuicSocket::DecrementAllocatedSize(size_t size) {
+void QuicSocket::DecreaseAllocatedSize(size_t size) {
   current_ngtcp2_memory_ -= size;
 }
 
-
 // JavaScript API
 namespace {
 void NewQuicSocket(const FunctionCallbackInfo<Value>& args) {