trafficserver-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From wka...@apache.org
Subject [trafficserver] branch master updated: Make Allocator.h less silly (no creepy "proto" object). (#6241)
Date Wed, 03 Mar 2021 02:04:27 GMT
This is an automated email from the ASF dual-hosted git repository.

wkaras pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/trafficserver.git


The following commit(s) were added to refs/heads/master by this push:
     new f51f8ed  Make Allocator.h less silly (no creepy "proto" object). (#6241)
f51f8ed is described below

commit f51f8ed85443b595e379b2e0201095422082ba57
Author: Walt Karas <wkaras@verizonmedia.com>
AuthorDate: Tue Mar 2 20:04:09 2021 -0600

    Make Allocator.h less silly (no creepy "proto" object). (#6241)
    
    Also make class and proxy allocators compatible with destructors.
---
 include/tscore/Allocator.h            | 110 +++++++++++++---------------------
 iocore/cache/P_CacheInternal.h        |   2 +-
 iocore/eventsystem/I_ProxyAllocator.h |  96 ++++++++++-------------------
 3 files changed, 77 insertions(+), 131 deletions(-)

diff --git a/include/tscore/Allocator.h b/include/tscore/Allocator.h
index c03f996..fd5200b 100644
--- a/include/tscore/Allocator.h
+++ b/include/tscore/Allocator.h
@@ -41,6 +41,7 @@
 
 #include <new>
 #include <cstdlib>
+#include <utility>
 #include "tscore/ink_queue.h"
 #include "tscore/ink_defs.h"
 #include "tscore/ink_resource.h"
@@ -108,29 +109,39 @@ public:
     ink_freelist_madvise_init(&this->fl, name, element_size, chunk_size, alignment,
advice);
   }
 
+  // Dummies
+  void
+  destroy_if_enabled(void *)
+  {
+  }
+  Allocator &
+  raw()
+  {
+    return *this;
+  }
+
 protected:
   InkFreeList *fl;
 };
 
 /**
-  Allocator for Class objects. It uses a prototype object to do
-  fast initialization. Prototype of the template class is created
-  when the fast allocator is created. This is instantiated with
-  default (no argument) constructor. Constructor is not called for
-  the allocated objects. Instead, the prototype is just memory
-  copied onto the new objects. This is done for performance reasons.
+  Allocator for Class objects.
 
 */
-template <class C> class ClassAllocator : public Allocator
+template <class C, bool Destruct_on_free_ = false> class ClassAllocator : private Allocator
 {
 public:
-  /** Allocates objects of the templated type. */
+  using Value_type                   = C;
+  static bool const Destruct_on_free = Destruct_on_free_;
+
+  /** Allocates objects of the templated type.  Arguments are forwarded to the constructor
for the object. */
+  template <typename... Args>
   C *
-  alloc()
+  alloc(Args &&... args)
   {
     void *ptr = ink_freelist_new(this->fl);
 
-    memcpy(ptr, (void *)&this->proto.typeObject, sizeof(C));
+    ::new (ptr) C(std::forward<Args>(args)...);
     return (C *)ptr;
   }
 
@@ -142,82 +153,47 @@ public:
   void
   free(C *ptr)
   {
+    destroy_if_enabled(ptr);
+
     ink_freelist_free(this->fl, ptr);
   }
 
   /**
-    Deallocates objects of the templated type.
-
-    @param head pointer to be freed.
-    @param tail pointer to be freed.
-    @param num_item of blocks to be freed.
-   */
-  void
-  free_bulk(C *head, C *tail, size_t num_item)
-  {
-    ink_freelist_free_bulk(this->fl, head, tail, num_item);
-  }
+    Create a new class specific ClassAllocator.
 
-  /**
-    Allocate objects of the templated type via the inherited interface
-    using void pointers.
+    @param name some identifying name, used for mem tracking purposes.
+    @param chunk_size number of units to be allocated if free pool is empty.
+    @param alignment of objects must be a power of 2.
   */
-  void *
-  alloc_void()
+  ClassAllocator(const char *name, unsigned int chunk_size = 128, unsigned int alignment
= 16)
   {
-    return (void *)alloc();
+    ink_freelist_init(&this->fl, name, RND16(sizeof(C)), chunk_size, RND16(alignment));
   }
 
-  /**
-    Deallocate objects of the templated type via the inherited
-    interface using void pointers.
-
-    @param ptr pointer to be freed.
-  */
-  void
-  free_void(void *ptr)
+  Allocator &
+  raw()
   {
-    free((C *)ptr);
+    return *this;
   }
 
-  /**
-    Deallocate objects of the templated type via the inherited
-    interface using void pointers.
-
-    @param head pointer to be freed.
-    @param tail pointer to be freed.
-    @param num_item of blocks.
-  */
   void
-  free_void_bulk(void *head, void *tail, size_t num_item)
+  destroy_if_enabled(C *ptr)
   {
-    free_bulk((C *)head, (C *)tail, num_item);
-  }
-
-  /**
-    Create a new class specific ClassAllocator.
-
-    @param name some identifying name, used for mem tracking purposes.
-    @param chunk_size number of units to be allocated if free pool is empty.
-    @param alignment of objects must be a power of 2.
-  */
-  ClassAllocator(const char *name, unsigned int chunk_size = 128, unsigned int alignment
= 16)
-  {
-    ::new ((void *)&proto.typeObject) C();
-    ink_freelist_init(&this->fl, name, RND16(sizeof(C)), chunk_size, RND16(alignment));
+    if (Destruct_on_free) {
+      ptr->~C();
+    }
   }
 
-  struct {
-    uint8_t typeObject[sizeof(C)];
-    int64_t space_holder = 0;
-  } proto;
+  // Ensure that C is big enough to hold a void pointer (when it's stored in the free list
as raw memory).
+  //
+  static_assert(sizeof(C) >= sizeof(void *), "Can not allocate instances of this class
using ClassAllocator");
 };
 
-template <class C> class TrackerClassAllocator : public ClassAllocator<C>
+template <class C, bool Destruct_on_free = false> class TrackerClassAllocator : public
ClassAllocator<C, Destruct_on_free>
 {
 public:
   TrackerClassAllocator(const char *name, unsigned int chunk_size = 128, unsigned int alignment
= 16)
-    : ClassAllocator<C>(name, chunk_size, alignment), allocations(0), trackerLock(PTHREAD_MUTEX_INITIALIZER)
+    : ClassAllocator<C, Destruct_on_free>(name, chunk_size, alignment), allocations(0),
trackerLock(PTHREAD_MUTEX_INITIALIZER)
   {
   }
 
@@ -226,7 +202,7 @@ public:
   {
     void *callstack[3];
     int frames = backtrace(callstack, 3);
-    C *ptr     = ClassAllocator<C>::alloc();
+    C *ptr     = ClassAllocator<C, Destruct_on_free>::alloc();
 
     const void *symbol = nullptr;
     if (frames == 3 && callstack[2] != nullptr) {
@@ -252,7 +228,7 @@ public:
       reverse_lookup.erase(it);
     }
     ink_mutex_release(&trackerLock);
-    ClassAllocator<C>::free(ptr);
+    ClassAllocator<C, Destruct_on_free>::free(ptr);
   }
 
 private:
diff --git a/iocore/cache/P_CacheInternal.h b/iocore/cache/P_CacheInternal.h
index 3e05f2c..70f6705 100644
--- a/iocore/cache/P_CacheInternal.h
+++ b/iocore/cache/P_CacheInternal.h
@@ -422,7 +422,7 @@ struct CacheVC : public CacheVConnection {
   Ptr<IOBufferBlock> blocks; // data available to write
   Ptr<IOBufferBlock> writer_buf;
 
-  OpenDirEntry *od;
+  OpenDirEntry *od = nullptr;
   AIOCallbackInternal io;
   int alternate_index = CACHE_ALT_INDEX_DEFAULT; // preferred position in vector
   LINK(CacheVC, opendir_link);
diff --git a/iocore/eventsystem/I_ProxyAllocator.h b/iocore/eventsystem/I_ProxyAllocator.h
index a5edd12..346591d 100644
--- a/iocore/eventsystem/I_ProxyAllocator.h
+++ b/iocore/eventsystem/I_ProxyAllocator.h
@@ -30,6 +30,9 @@
 *****************************************************************************/
 #pragma once
 
+#include <new>
+#include <utility>
+
 #include "tscore/ink_platform.h"
 
 class EThread;
@@ -45,84 +48,51 @@ struct ProxyAllocator {
   ProxyAllocator() {}
 };
 
-template <class C>
-inline C *
-thread_alloc(ClassAllocator<C> &a, ProxyAllocator &l)
+template <class CAlloc, typename... Args>
+typename CAlloc::Value_type *
+thread_alloc(CAlloc &a, ProxyAllocator &l, Args &&... args)
 {
   if (!cmd_disable_pfreelist && l.freelist) {
-    C *v       = (C *)l.freelist;
-    l.freelist = *(C **)l.freelist;
+    void *v    = l.freelist;
+    l.freelist = *reinterpret_cast<void **>(l.freelist);
     --(l.allocated);
-    *(void **)v = *(void **)&a.proto.typeObject;
-    return v;
+    ::new (v) typename CAlloc::Value_type(std::forward<Args>(args)...);
+    return static_cast<typename CAlloc::Value_type *>(v);
   }
-  return a.alloc();
+  return a.alloc(std::forward<Args>(args)...);
 }
 
-template <class C>
-inline C *
-thread_alloc_init(ClassAllocator<C> &a, ProxyAllocator &l)
-{
-  if (!cmd_disable_pfreelist && l.freelist) {
-    C *v       = (C *)l.freelist;
-    l.freelist = *(C **)l.freelist;
-    --(l.allocated);
-    memcpy((void *)v, (void *)&a.proto.typeObject, sizeof(C));
-    return v;
-  }
-  return a.alloc();
-}
+class Allocator;
 
-template <class C>
-inline void
-thread_free(ClassAllocator<C> &a, C *p)
-{
-  a.free(p);
-}
+void *thread_alloc(Allocator &a, ProxyAllocator &l);
+void thread_freeup(Allocator &a, ProxyAllocator &l);
 
-static inline void
-thread_free(Allocator &a, void *p)
-{
-  a.free_void(p);
-}
+#if 1
 
-template <class C>
-inline void
-thread_freeup(ClassAllocator<C> &a, ProxyAllocator &l)
-{
-  C *head      = (C *)l.freelist;
-  C *tail      = (C *)l.freelist;
-  size_t count = 0;
-  while (l.freelist && l.allocated > thread_freelist_low_watermark) {
-    tail       = (C *)l.freelist;
-    l.freelist = *(C **)l.freelist;
-    --(l.allocated);
-    ++count;
-  }
+// Potentially empty varaiable arguments -- non-standard GCC way
+//
+#define THREAD_ALLOC(_a, _t, ...) thread_alloc(::_a, _t->_a, ##__VA_ARGS__)
+#define THREAD_ALLOC_INIT(_a, _t, ...) thread_alloc(::_a, _t->_a, ##__VA_ARGS__)
 
-  if (unlikely(count == 1)) {
-    a.free(tail);
-  } else if (count > 0) {
-    a.free_bulk(head, tail, count);
-  }
+#else
 
-  ink_assert(l.allocated >= thread_freelist_low_watermark);
-}
+// Potentially empty varaiable arguments -- Standard C++20 way
+//
+#define THREAD_ALLOC(_a, _t, ...) thread_alloc(::_a, _t->_a __VA_OPT__(, ) __VA_ARGS__)
+#define THREAD_ALLOC_INIT(_a, _t, ...) thread_alloc(::_a, _t->_a __VA_OPT__(, ) __VA_ARGS__)
 
-void *thread_alloc(Allocator &a, ProxyAllocator &l);
-void thread_freeup(Allocator &a, ProxyAllocator &l);
+#endif
 
-#define THREAD_ALLOC(_a, _t) thread_alloc(::_a, _t->_a)
-#define THREAD_ALLOC_INIT(_a, _t) thread_alloc_init(::_a, _t->_a)
 #define THREAD_FREE(_p, _a, _t)                              \
-  if (!cmd_disable_pfreelist) {                              \
-    do {                                                     \
+  do {                                                       \
+    ::_a.destroy_if_enabled(_p);                             \
+    if (!cmd_disable_pfreelist) {                            \
       *(char **)_p    = (char *)_t->_a.freelist;             \
       _t->_a.freelist = _p;                                  \
       _t->_a.allocated++;                                    \
       if (_t->_a.allocated > thread_freelist_high_watermark) \
-        thread_freeup(::_a, _t->_a);                         \
-    } while (0);                                             \
-  } else {                                                   \
-    thread_free(::_a, _p);                                   \
-  }
+        thread_freeup(::_a.raw(), _t->_a);                   \
+    } else {                                                 \
+      ::_a.raw().free_void(_p);                              \
+    }                                                        \
+  } while (0)


Mime
View raw message