dtoolbase: Make MemoryHook constant-initialized

init_memory_hook() is no longer required, eliminating static initialization order issues

This required moving the DeletedBufferChain map elsewhere, which now also has a const-initialized array for relatively small allocations.

Also, deletedChain.T has been renamed to deletedChain.I

Fixes #539

dtool/src/dtoolbase/CMakeLists.txt

@@ -21,7 +21,7 @@ set(P3DTOOLBASE_HEADERS
   atomicAdjustWin32Impl.h atomicAdjustWin32Impl.I
   cmath.I cmath.h
   deletedBufferChain.h deletedBufferChain.I
-  deletedChain.h deletedChain.T
+  deletedChain.h deletedChain.I
   dtoolbase.h dtoolbase_cc.h dtoolsymbols.h
   dtool_platform.h
   fakestringstream.h

dtool/src/dtoolbase/deletedBufferChain.I

@@ -11,6 +11,34 @@
  * @date 2007-07-20
  */
 
+/**
+ * Use get_deleted_chain() to get a new DeletedBufferChain of the appropriate
+ * size.
+ */
+constexpr DeletedBufferChain::
+DeletedBufferChain(size_t buffer_size) : _buffer_size(buffer_size) {
+}
+
+/**
+ * Move constructor.
+ */
+INLINE DeletedBufferChain::
+DeletedBufferChain(DeletedBufferChain &&from) noexcept :
+  _deleted_chain(from._deleted_chain),
+  _buffer_size(from._buffer_size) {
+  from._deleted_chain = nullptr;
+}
+
+/**
+ * Copy constructor.
+ */
+INLINE DeletedBufferChain::
+DeletedBufferChain(const DeletedBufferChain &copy) :
+  _deleted_chain(nullptr),
+  _buffer_size(copy._buffer_size) {
+  assert(copy._deleted_chain == nullptr);
+}
+
 /**
  * Returns true if the pointer is valid, false if it has been deleted or if it
  * was never a valid pointer.
@@ -41,6 +69,30 @@ get_buffer_size() const {
   return _buffer_size;
 }
 
+/**
+ *
+ */
+INLINE bool DeletedBufferChain::
+operator < (const DeletedBufferChain &other) const {
+  return _buffer_size < other._buffer_size;
+}
+
+/**
+ * Returns a deleted chain of the given size.
+ */
+INLINE DeletedBufferChain *DeletedBufferChain::
+get_deleted_chain(size_t buffer_size) {
+  // We must allocate at least this much space for bookkeeping reasons.
+  buffer_size = (std::max)(buffer_size, sizeof(ObjectNode));
+
+  size_t index = ((buffer_size + sizeof(void *) - 1) / sizeof(void *)) - 1;
+  if (index < num_small_deleted_chains) {
+    return &_small_deleted_chains[index];
+  } else {
+    return get_large_deleted_chain((index + 1) * sizeof(void *));
+  }
+}
+
 /**
  * Casts an ObjectNode* to a void* buffer.
  */

dtool/src/dtoolbase/deletedBufferChain.cxx

@@ -14,18 +14,34 @@
 #include "deletedBufferChain.h"
 #include "memoryHook.h"
 
-/**
- * Use the global MemoryHook to get a new DeletedBufferChain of the
- * appropriate size.
- */
-DeletedBufferChain::
-DeletedBufferChain(size_t buffer_size) {
-  _deleted_chain = nullptr;
-  _buffer_size = buffer_size;
+#include <set>
 
-  // We must allocate at least this much space for bookkeeping reasons.
-  _buffer_size = std::max(_buffer_size, sizeof(ObjectNode));
-}
+DeletedBufferChain DeletedBufferChain::_small_deleted_chains[DeletedBufferChain::num_small_deleted_chains] = {
+  DeletedBufferChain(sizeof(void *)),
+  DeletedBufferChain(sizeof(void *) * 2),
+  DeletedBufferChain(sizeof(void *) * 3),
+  DeletedBufferChain(sizeof(void *) * 4),
+  DeletedBufferChain(sizeof(void *) * 5),
+  DeletedBufferChain(sizeof(void *) * 6),
+  DeletedBufferChain(sizeof(void *) * 7),
+  DeletedBufferChain(sizeof(void *) * 8),
+  DeletedBufferChain(sizeof(void *) * 9),
+  DeletedBufferChain(sizeof(void *) * 10),
+  DeletedBufferChain(sizeof(void *) * 11),
+  DeletedBufferChain(sizeof(void *) * 12),
+  DeletedBufferChain(sizeof(void *) * 13),
+  DeletedBufferChain(sizeof(void *) * 14),
+  DeletedBufferChain(sizeof(void *) * 15),
+  DeletedBufferChain(sizeof(void *) * 16),
+  DeletedBufferChain(sizeof(void *) * 17),
+  DeletedBufferChain(sizeof(void *) * 18),
+  DeletedBufferChain(sizeof(void *) * 19),
+  DeletedBufferChain(sizeof(void *) * 20),
+  DeletedBufferChain(sizeof(void *) * 21),
+  DeletedBufferChain(sizeof(void *) * 22),
+  DeletedBufferChain(sizeof(void *) * 23),
+  DeletedBufferChain(sizeof(void *) * 24),
+};
 
 /**
  * Allocates the memory for a new buffer of the indicated size (which must be
@@ -139,3 +155,16 @@ deallocate(void *ptr, TypeHandle type_handle) {
   PANDA_FREE_SINGLE(ptr);
 #endif  // USE_DELETED_CHAIN
 }
+
+/**
+ * Returns a new DeletedBufferChain.
+ */
+DeletedBufferChain *DeletedBufferChain::
+get_large_deleted_chain(size_t buffer_size) {
+  static MutexImpl lock;
+  lock.lock();
+  static std::set<DeletedBufferChain> deleted_chains;
+  DeletedBufferChain *result = (DeletedBufferChain *)&*deleted_chains.insert(DeletedBufferChain(buffer_size)).first;
+  lock.unlock();
+  return result;
+}

dtool/src/dtoolbase/deletedBufferChain.h

@@ -58,16 +58,25 @@ enum DeletedChainFlag : unsigned int {
  */
 class EXPCL_DTOOL_DTOOLBASE DeletedBufferChain {
 protected:
-  DeletedBufferChain(size_t buffer_size);
+  constexpr explicit DeletedBufferChain(size_t buffer_size);
 
 public:
+  INLINE DeletedBufferChain(DeletedBufferChain &&from) noexcept;
+  INLINE DeletedBufferChain(const DeletedBufferChain &copy);
+
   void *allocate(size_t size, TypeHandle type_handle);
   void deallocate(void *ptr, TypeHandle type_handle);
 
   INLINE bool validate(void *ptr);
   INLINE size_t get_buffer_size() const;
 
+  INLINE bool operator < (const DeletedBufferChain &other) const;
+
+  static INLINE DeletedBufferChain *get_deleted_chain(size_t buffer_size);
+
 private:
+  static DeletedBufferChain *get_large_deleted_chain(size_t buffer_size);
+
   class ObjectNode {
   public:
 #ifdef USE_DELETEDCHAINFLAG
@@ -87,10 +96,10 @@ private:
   static INLINE void *node_to_buffer(ObjectNode *node);
   static INLINE ObjectNode *buffer_to_node(void *buffer);
 
-  ObjectNode *_deleted_chain;
+  ObjectNode *_deleted_chain = nullptr;
 
   MutexImpl _lock;
-  size_t _buffer_size;
+  const size_t _buffer_size;
 
 #ifndef USE_DELETEDCHAINFLAG
   // Without DELETEDCHAINFLAG, we don't even store the _flag member at all.
@@ -101,6 +110,11 @@ private:
   static const size_t flag_reserved_bytes = sizeof(AtomicAdjust::Integer);
 #endif  // USE_DELETEDCHAINFLAG
 
+  // This array stores the deleted chains for smaller sizes, starting with
+  // sizeof(void *) and increasing in multiples thereof.
+  static const size_t num_small_deleted_chains = 24;
+  static DeletedBufferChain _small_deleted_chains[num_small_deleted_chains];
+
   friend class MemoryHook;
 };
 

dtool/src/dtoolbase/deletedChain.I

@@ -6,7 +6,7 @@
  * license.  You should have received a copy of this license along
  * with this source code in a file named "LICENSE."
  *
- * @file deletedChain.T
+ * @file deletedChain.I
  * @author drose
  * @date 2006-04-01
  */
@@ -82,7 +82,7 @@ validate(const Type *ptr) {
 template<class Type>
 INLINE ReferenceCount *DeletedChain<Type>::
 make_ref_ptr(void *) {
-  return NULL;
+  return nullptr;
 }
 
 /**
@@ -106,9 +106,8 @@ make_ref_ptr(ReferenceCount *ptr) {
 template<class Type>
 void DeletedChain<Type>::
 init_deleted_chain() {
-  if (_chain == (DeletedBufferChain *)NULL) {
-    init_memory_hook();
-    _chain = memory_hook->get_deleted_chain(sizeof(Type));
+  if (_chain == nullptr) {
+    _chain = DeletedBufferChain::get_deleted_chain(sizeof(Type));
   }
 }
 

dtool/src/dtoolbase/deletedChain.h

@@ -136,6 +136,6 @@ public:
 
 #endif  // USE_DELETED_CHAIN
 
-#include "deletedChain.T"
+#include "deletedChain.I"
 
 #endif

dtool/src/dtoolbase/dtoolbase.cxx

@@ -23,34 +23,18 @@
 bool __tau_shutdown = false;
 #endif
 
-MemoryHook *memory_hook;
+MemoryHook default_memory_hook;
+MemoryHook *memory_hook = &default_memory_hook;
 
 /**
- * Any code that might need to use PANDA_MALLOC or PANDA_FREE, or any methods
- * of the global memory_hook object, at static init time, should ensure that
- * it calls init_memory_hook() first to ensure that the pointer has been
- * properly initialized.  There is no harm in calling this function more than
- * once.
- *
- * There is no need to call this function other than at static init time.
+ * It used to be necessary to call this before using operator new at static
+ * init time.  There is no need to call this function anymore.
  */
 void
 init_memory_hook() {
-  if (memory_hook == nullptr) {
-    memory_hook = new MemoryHook;
-  }
+  assert(memory_hook != nullptr);
 }
 
-// Here's a quick way to ensure the above function is called at least once at
-// static init time.
-class InitMemoryHook {
-public:
-  InitMemoryHook() {
-    init_memory_hook();
-  }
-};
-static InitMemoryHook _imh_object;
-
 #if defined(HAVE_THREADS) && defined(SIMPLE_THREADS)
 
 static void

dtool/src/dtoolbase/memoryHook.I

@@ -40,6 +40,9 @@ dec_heap(size_t size) {
  */
 INLINE size_t MemoryHook::
 get_page_size() const {
+  if (_page_size == 0) {
+    determine_page_size();
+  }
   return _page_size;
 }
 
@@ -49,6 +52,9 @@ get_page_size() const {
  */
 INLINE size_t MemoryHook::
 round_up_to_page_size(size_t size) const {
+  if (_page_size == 0) {
+    determine_page_size();
+  }
   return  ((size + _page_size - 1) / _page_size) * _page_size;
 }
 

dtool/src/dtoolbase/memoryHook.cxx

@@ -198,33 +198,6 @@ ptr_to_alloc(void *ptr, size_t &size) {
 #endif  // DO_MEMORY_USAGE
 }
 
-/**
- *
- */
-MemoryHook::
-MemoryHook() {
-#ifdef _WIN32
-
-  // Windows case.
-  SYSTEM_INFO sysinfo;
-  GetSystemInfo(&sysinfo);
-
-  _page_size = (size_t)sysinfo.dwPageSize;
-
-#else
-
-  // Posix case.
-  _page_size = sysconf(_SC_PAGESIZE);
-
-#endif  // WIN32
-
-  _total_heap_single_size = 0;
-  _total_heap_array_size = 0;
-  _requested_heap_size = 0;
-  _total_mmap_size = 0;
-  _max_heap_size = ~(size_t)0;
-}
-
 /**
  *
  */
@@ -236,20 +209,8 @@ MemoryHook(const MemoryHook &copy) :
   _total_mmap_size(copy._total_mmap_size),
   _max_heap_size(copy._max_heap_size),
   _page_size(copy._page_size) {
-
-  copy._lock.lock();
-  _deleted_chains = copy._deleted_chains;
-  copy._lock.unlock();
 }
 
-/**
- *
- */
-MemoryHook::
-~MemoryHook() {
-  // Really, we only have this destructor to shut up gcc about the virtual
-  // functions warning.
-}
 
 /**
  * Allocates a block of memory from the heap, similar to malloc().  This will
@@ -522,6 +483,9 @@ heap_trim(size_t pad) {
  */
 void *MemoryHook::
 mmap_alloc(size_t size, bool allow_exec) {
+  if (_page_size == 0) {
+    determine_page_size();
+  }
   assert((size % _page_size) == 0);
 
 #ifdef DO_MEMORY_USAGE
@@ -576,6 +540,7 @@ mmap_alloc(size_t size, bool allow_exec) {
  */
 void MemoryHook::
 mmap_free(void *ptr, size_t size) {
+  assert(_page_size != 0);
   assert((size % _page_size) == 0);
 
 #ifdef DO_MEMORY_USAGE
@@ -601,29 +566,6 @@ void MemoryHook::
 mark_pointer(void *, size_t, ReferenceCount *) {
 }
 
-/**
- * Returns a pointer to a global DeletedBufferChain object suitable for
- * allocating arrays of the indicated size.  There is one unique
- * DeletedBufferChain object for every different size.
- */
-DeletedBufferChain *MemoryHook::
-get_deleted_chain(size_t buffer_size) {
-  DeletedBufferChain *chain;
-
-  _lock.lock();
-  DeletedChains::iterator dci = _deleted_chains.find(buffer_size);
-  if (dci != _deleted_chains.end()) {
-    chain = (*dci).second;
-  } else {
-    // Once allocated, this DeletedBufferChain object is never deleted.
-    chain = new DeletedBufferChain(buffer_size);
-    _deleted_chains.insert(DeletedChains::value_type(buffer_size, chain));
-  }
-
-  _lock.unlock();
-  return chain;
-}
-
 /**
  * This callback method is called whenever a low-level call to call_malloc()
  * has returned NULL, indicating failure.
@@ -656,3 +598,24 @@ overflow_heap_size() {
   _max_heap_size = ~(size_t)0;
 #endif  // DO_MEMORY_USAGE
 }
+
+/**
+ * Asks the operating system for the page size.
+ */
+void MemoryHook::
+determine_page_size() const {
+#ifdef _WIN32
+  // Windows case.
+  SYSTEM_INFO sysinfo;
+  GetSystemInfo(&sysinfo);
+
+  _page_size = (size_t)sysinfo.dwPageSize;
+
+#else
+  // Posix case.
+  _page_size = sysconf(_SC_PAGESIZE);
+
+#endif  // WIN32
+
+  assert(_page_size != 0);
+}

dtool/src/dtoolbase/memoryHook.h

@@ -20,8 +20,6 @@
 #include "mutexImpl.h"
 #include <map>
 
-class DeletedBufferChain;
-
 /**
  * This class provides a wrapper around the various possible malloc schemes
  * Panda might employ.  It also exists to allow the MemoryUsage class in Panda
@@ -36,9 +34,9 @@ class DeletedBufferChain;
  */
 class EXPCL_DTOOL_DTOOLBASE MemoryHook {
 public:
-  MemoryHook();
+  constexpr MemoryHook() = default;
   MemoryHook(const MemoryHook &copy);
-  virtual ~MemoryHook();
+  virtual ~MemoryHook() = default;
 
   virtual void *heap_alloc_single(size_t size);
   virtual void heap_free_single(void *ptr);
@@ -63,29 +61,26 @@ public:
 
   virtual void mark_pointer(void *ptr, size_t orig_size, ReferenceCount *ref_ptr);
 
-  DeletedBufferChain *get_deleted_chain(size_t buffer_size);
-
   virtual void alloc_fail(size_t attempted_size);
 
   INLINE static size_t get_ptr_size(void *ptr);
 
 protected:
-  TVOLATILE AtomicAdjust::Integer _total_heap_single_size;
-  TVOLATILE AtomicAdjust::Integer _total_heap_array_size;
-  TVOLATILE AtomicAdjust::Integer _requested_heap_size;
-  TVOLATILE AtomicAdjust::Integer _total_mmap_size;
+  TVOLATILE AtomicAdjust::Integer _total_heap_single_size = 0;
+  TVOLATILE AtomicAdjust::Integer _total_heap_array_size = 0;
+  TVOLATILE AtomicAdjust::Integer _requested_heap_size = 0;
+  TVOLATILE AtomicAdjust::Integer _total_mmap_size = 0;
 
   // If the allocated heap size crosses this threshold, we call
   // overflow_heap_size().
-  size_t _max_heap_size;
+  size_t _max_heap_size = ~(size_t)0;
 
   virtual void overflow_heap_size();
 
-private:
-  size_t _page_size;
+  void determine_page_size() const;
 
-  typedef std::map<size_t, DeletedBufferChain *> DeletedChains;
-  DeletedChains _deleted_chains;
+private:
+  mutable size_t _page_size = 0;
 
   mutable MutexImpl _lock;
 };

dtool/src/dtoolbase/typeRegistry.cxx

@@ -539,7 +539,6 @@ TypeRegistry() {
  */
 void TypeRegistry::
 init_global_pointer() {
-  init_memory_hook();
   _global_pointer = new TypeRegistry;
 }
 

dtool/src/prc/configVariableManager.cxx

@@ -28,7 +28,6 @@ ConfigVariableManager *ConfigVariableManager::_global_ptr = nullptr;
  */
 ConfigVariableManager::
 ConfigVariableManager() {
-  init_memory_hook();
 }
 
 /**

dtool/src/prc/notify.cxx

@@ -291,7 +291,6 @@ write_string(const string &str) {
 Notify *Notify::
 ptr() {
   if (_global_ptr == nullptr) {
-    init_memory_hook();
     _global_ptr = new Notify;
   }
   return _global_ptr;

panda/src/event/asyncTaskManager.cxx

@@ -649,7 +649,6 @@ void AsyncTaskManager::
 make_global_ptr() {
   nassertv(_global_ptr == nullptr);
 
-  init_memory_hook();
   _global_ptr = new AsyncTaskManager("TaskManager");
   _global_ptr->ref();
 }

panda/src/event/eventHandler.cxx

@@ -395,7 +395,6 @@ remove_all_hooks() {
  */
 void EventHandler::
 make_global_event_handler() {
-  init_memory_hook();
   _global_event_handler = new EventHandler(EventQueue::get_global_event_queue());
 }
 

panda/src/event/eventQueue.cxx

@@ -108,6 +108,5 @@ dequeue_event() {
  */
 void EventQueue::
 make_global_event_queue() {
-  init_memory_hook();
   _global_event_queue = new EventQueue;
 }

panda/src/express/memoryUsage.cxx

@@ -535,7 +535,6 @@ MemoryUsage(const MemoryHook &copy) :
 void MemoryUsage::
 init_memory_usage() {
 #ifdef DO_MEMORY_USAGE
-  init_memory_hook();
   _global_ptr = new MemoryUsage(*memory_hook);
   memory_hook = _global_ptr;
 #else

panda/src/movies/movieVideoCursor.cxx

@@ -363,7 +363,7 @@ MovieVideoCursor::Buffer::
 Buffer(size_t block_size) :
   _block_size(block_size)
 {
-  _deleted_chain = memory_hook->get_deleted_chain(_block_size);
+  _deleted_chain = DeletedBufferChain::get_deleted_chain(_block_size);
   _block = (unsigned char *)_deleted_chain->allocate(_block_size, get_class_type());
 }
 

panda/src/pipeline/thread.cxx

@@ -209,7 +209,6 @@ init_main_thread() {
   static int count = 0;
   ++count;
   if (count == 1 && _main_thread == nullptr) {
-    init_memory_hook();
     _main_thread = new MainThread;
     _main_thread->ref();
   }
@@ -221,7 +220,6 @@ init_main_thread() {
 void Thread::
 init_external_thread() {
   if (_external_thread == nullptr) {
-    init_memory_hook();
     _external_thread = new ExternalThread;
     _external_thread->ref();
   }

panda/src/putil/simpleHashMap.I

@@ -46,7 +46,7 @@ SimpleHashMap(const SimpleHashMap &copy) :
     size_t alloc_size = _table_size * (sizeof(TableEntry) + sizeof(int) * sparsity);
 
     init_type();
-    _deleted_chain = memory_hook->get_deleted_chain(alloc_size);
+    _deleted_chain = DeletedBufferChain::get_deleted_chain(alloc_size);
     _table = (TableEntry *)_deleted_chain->allocate(alloc_size, _type_handle);
 
     for (size_t i = 0; i < _num_entries; ++i) {
@@ -106,7 +106,7 @@ operator = (const SimpleHashMap<Key, Value, Compare> &copy) {
       size_t alloc_size = _table_size * (sizeof(TableEntry) + sizeof(int) * sparsity);
 
       init_type();
-      _deleted_chain = memory_hook->get_deleted_chain(alloc_size);
+      _deleted_chain = DeletedBufferChain::get_deleted_chain(alloc_size);
       _table = (TableEntry *)_deleted_chain->allocate(alloc_size, _type_handle);
       for (size_t i = 0; i < _num_entries; ++i) {
         new(&_table[i]) TableEntry(copy._table[i]);
@@ -691,7 +691,7 @@ new_table() {
   size_t alloc_size = _table_size * (sizeof(TableEntry) + sizeof(int) * sparsity);
 
   init_type();
-  _deleted_chain = memory_hook->get_deleted_chain(alloc_size);
+  _deleted_chain = DeletedBufferChain::get_deleted_chain(alloc_size);
   _table = (TableEntry *)_deleted_chain->allocate(alloc_size, _type_handle);
   memset(get_index_array(), -1, _table_size * sizeof(int) * sparsity);
 }
@@ -749,7 +749,7 @@ resize_table(size_t new_size) {
   // We allocate enough bytes for _table_size elements of TableEntry, plus
   // _table_size * sparsity more ints at the end (for the sparse index array).
   size_t alloc_size = _table_size * sizeof(TableEntry) + _table_size * sparsity * sizeof(int);
-  _deleted_chain = memory_hook->get_deleted_chain(alloc_size);
+  _deleted_chain = DeletedBufferChain::get_deleted_chain(alloc_size);
   _table = (TableEntry *)_deleted_chain->allocate(alloc_size, _type_handle);
   int *index_array = get_index_array();
   memset(index_array, -1, _table_size * sizeof(int) * sparsity);