(opportunistically found while grokking code new to me; but perf-relevant, could be significant) Removed an INFO-severity logged message that is logged from ipc::shm::arena_lend::jemalloc::Ipc_arena::get_collection_id(), a public API but more likely invoked internally from Shm_session::lend_object() which can be oft-used directly and for each capnp-encoded message sent through a SHM-jemalloc-backed ipc::transport::struc::Channel. / (opportunistically found while grokking code new to me; but somewhat perf-relevant) Use move-semantics in a few places internally to avoid unnecessary shared_ptr ref-count arithmetic. / (opportunistically found while grokking code new to me; but slightly perf-relevant if asserts-on) Effectively remove a few redundant internal assert()s. / Comment and/or doc changes. (Opportunistic.)

Author: ygoldfeld

src/ipc/shm/arena_lend/jemalloc/ipc_arena.cpp

@@ -240,14 +240,14 @@ void Ipc_arena::Event_listener_impl::notify_removed_shm_pool(const shared_ptr<Sh
 }
 
 Ipc_arena::Ipc_object_deleter_no_cache::Ipc_object_deleter_no_cache(
-  const shared_ptr<Shm_pool_collection>& pool_collection, Arena_id arena_id) :
-  Object_deleter_no_cache(pool_collection, arena_id)
+  shared_ptr<Shm_pool_collection>&& pool_collection, Arena_id arena_id) :
+  Object_deleter_no_cache(std::move(pool_collection), arena_id)
 {
 }
 
 Ipc_arena::Ipc_object_deleter_cache::Ipc_object_deleter_cache(
-  const shared_ptr<Thread_cache>& thread_cache) :
-  Object_deleter_cache(thread_cache)
+  shared_ptr<Thread_cache>&& thread_cache) :
+  Object_deleter_cache(std::move(thread_cache))
 {
 }
 

src/ipc/shm/arena_lend/jemalloc/ipc_arena.hpp

@@ -230,7 +230,7 @@ class Ipc_arena :
      * @param pool_collection The object that allocated the underlying memory from which it should be returned
      * @param arena_id The arena in which the allocation was performed
      */
-    Ipc_object_deleter_no_cache(const std::shared_ptr<Shm_pool_collection>& pool_collection,
+    Ipc_object_deleter_no_cache(std::shared_ptr<Shm_pool_collection>&& pool_collection,
                                 Arena_id arena_id);
 
     /**
@@ -259,7 +259,7 @@ class Ipc_arena :
      *
      * @param thread_cache The thread cache to associate the allocation with.
      */
-    Ipc_object_deleter_cache(const std::shared_ptr<Thread_cache>& thread_cache);
+    Ipc_object_deleter_cache(std::shared_ptr<Thread_cache>&& thread_cache);
 
     /**
      * Release callback executed by shared pointer destruction. The destructor for the object will be called
@@ -402,10 +402,11 @@ Ipc_arena::Handle<T> Ipc_arena::construct_impl(bool use_cache, Args&&... args)
   Ipc_arena_activator ctx(this);
   if (use_cache)
   {
-    std::shared_ptr<Thread_cache> thread_cache = get_or_create_thread_cache(arena_id);
+    auto thread_cache = get_or_create_thread_cache(arena_id);
+    const auto thread_cache_id = thread_cache->get_thread_cache_id();
     return construct_helper<T>(
-      Shm_pool_collection::allocate(sizeof(T), arena_id, thread_cache->get_thread_cache_id()),
-      Ipc_object_deleter_cache(thread_cache),
+      Shm_pool_collection::allocate(sizeof(T), arena_id, thread_cache_id),
+      Ipc_object_deleter_cache(std::move(thread_cache)),
       std::forward<Args>(args)...);
   }
   else
@@ -422,23 +423,15 @@ template <typename T>
 Collection_id Ipc_arena::get_collection_id(const std::shared_ptr<T>& object)
 {
   {
-    const auto* deleter = std::get_deleter<Ipc_object_deleter_no_cache>(object);
+    auto const * const deleter = std::get_deleter<Ipc_object_deleter_no_cache>(object);
     if (deleter != nullptr)
     {
-      auto pool_collection = deleter->get_pool_collection();
-      auto* logger = pool_collection->get_logger();
-      if ((logger != nullptr) && logger->should_log(flow::log::Sev::S_INFO, Log_component::S_SESSION))
-      {
-        FLOW_LOG_SET_CONTEXT(logger, Log_component::S_SESSION);
-        FLOW_LOG_INFO_WITHOUT_CHECKING("Pool collection " << pool_collection << ", count " <<
-                                       pool_collection.use_count());
-      }
-      return pool_collection->get_id();
+      return deleter->get_pool_collection()->get_id();
     }
   }
 
   {
-    const auto* deleter = std::get_deleter<Ipc_object_deleter_cache>(object);
+    auto const * const deleter = std::get_deleter<Ipc_object_deleter_cache>(object);
     if (deleter != nullptr)
     {
       return deleter->get_thread_cache()->get_owner()->get_id();

src/ipc/shm/arena_lend/jemalloc/shm_pool_collection.cpp

@@ -230,7 +230,7 @@ void* Shm_pool_collection::allocate(size_t size)
   // Use the first arena
   auto iter = m_arenas.begin();
   assert(iter != m_arenas.end());
-  return allocate(size, *iter);
+  return get_jemalloc_memory_manager()->allocate(size, *iter);
 }
 
 void* Shm_pool_collection::allocate(size_t size, Arena_id arena_id)
@@ -256,7 +256,7 @@ void Shm_pool_collection::deallocate(void* address)
   // Use the first arena
   auto iter = m_arenas.begin();
   assert(iter != m_arenas.end());
-  deallocate(address, *iter);
+  get_jemalloc_memory_manager()->deallocate(address, *iter);
 }
 
 void Shm_pool_collection::deallocate(void* address, Arena_id arena_id)
@@ -614,17 +614,17 @@ Shm_pool_collection::get_or_create_thread_cache(Arena_id arena_id)
 }
 
 Shm_pool_collection::Object_deleter_no_cache::Object_deleter_no_cache(
-  const shared_ptr<Shm_pool_collection>& pool_collection, Arena_id arena_id) :
-  m_pool_collection(pool_collection),
+  shared_ptr<Shm_pool_collection>&& pool_collection, Arena_id arena_id) :
+  m_pool_collection(std::move(pool_collection)),
   m_arena_id(arena_id)
 {
 }
 
 Shm_pool_collection::Object_deleter_cache::Object_deleter_cache(
-  const shared_ptr<Thread_cache>& thread_cache) :
-  m_thread_cache(thread_cache)
+  shared_ptr<Thread_cache>&& thread_cache) :
+  m_thread_cache(std::move(thread_cache))
 {
-  assert(thread_cache != nullptr);
+  assert(m_thread_cache != nullptr);
 }
 
 bool Shm_pool_collection::Thread_local_data::insert_cache(const shared_ptr<Thread_cache>& thread_cache)

src/ipc/shm/arena_lend/jemalloc/shm_pool_collection.hpp

@@ -113,17 +113,16 @@ class Shm_pool_collection :
   {
     if (use_cache)
     {
-      std::shared_ptr<Thread_cache> thread_cache = get_or_create_thread_cache(arena_id);
-      return construct_helper<T>(allocate(sizeof(T), arena_id, thread_cache->get_thread_cache_id()),
-                                 Object_deleter_cache(thread_cache),
-                                 std::forward<Args>(args)...);
-    }
-    else
-    {
-      return construct_helper<T>(allocate(sizeof(T), arena_id),
-                                 Object_deleter_no_cache(shared_from_this(), arena_id),
+      auto thread_cache = get_or_create_thread_cache(arena_id);
+      const auto thread_cache_id = thread_cache->get_thread_cache_id();
+      return construct_helper<T>(allocate(sizeof(T), arena_id, thread_cache_id),
+                                 Object_deleter_cache(std::move(thread_cache)),
                                  std::forward<Args>(args)...);
     }
+    // else
+    return construct_helper<T>(allocate(sizeof(T), arena_id),
+                               Object_deleter_no_cache(shared_from_this(), arena_id),
+                               std::forward<Args>(args)...);
   }
 
   /**
@@ -542,7 +541,7 @@ class Shm_pool_collection :
      * @param pool_collection The object that allocated the underlying memory from which it should be returned.
      * @param arena_id The arena in which the allocation was performed.
      */
-    Object_deleter_no_cache(const std::shared_ptr<Shm_pool_collection>& pool_collection, Arena_id arena_id);
+    Object_deleter_no_cache(std::shared_ptr<Shm_pool_collection>&& pool_collection, Arena_id arena_id);
 
     /**
      * Release callback executed by shared pointer destruction. The destructor for the object will be called
@@ -554,7 +553,13 @@ class Shm_pool_collection :
     template <typename T>
     void operator()(T* p)
     {
-      // Call object's destructor (works for primitives)
+      /* Call object's destructor (works for primitives).
+       * Key context: an allocator-equipped T (usually STL-compliant container) here will (in T::~T())
+       * call-through to m_pool_collection->deallocate() for each buffer that T had allocated for itself
+       * (e.g., for vector<> internal buffer; for list<> the various nodes). In the process more destructors
+       * may be invoked which would quite possibly do more dtor calling and thus deallocate()ing. And so on.
+       * Hence the present operator()() is called at just the outer layer, then the inner deallocations (if any)
+       * happen, and then we call deallocate() on the outer object's memory, last-thing, just below. */
       p->~T();
       m_pool_collection->deallocate(p, m_arena_id);
     }
@@ -591,7 +596,7 @@ class Shm_pool_collection :
      *
      * @param thread_cache The thread cache to associate the allocation with.
      */
-    Object_deleter_cache(const std::shared_ptr<Thread_cache>& thread_cache);
+    Object_deleter_cache(std::shared_ptr<Thread_cache>&& thread_cache);
 
     /**
      * Release callback executed by shared pointer destruction. The destructor for the object will be called
@@ -603,7 +608,7 @@ class Shm_pool_collection :
     template <typename T>
     void operator()(T* p)
     {
-      // Call object's destructor (works for primitives)
+      // Call object's destructor (works for primitives). See more key notes in Object_deleter_no_cache comment.
       p->~T();
       m_thread_cache->get_owner()->deallocate(p,
                                               m_thread_cache->get_arena_id(),

src/ipc/shm/arena_lend/owner_shm_pool_collection.hpp

@@ -266,9 +266,9 @@ class Owner_shm_pool_collection :
   inline std::string generate_shm_object_name(pool_id_t shm_pool_id) const;
 
   /**
-   * Performs an allocation backed by shared memory, uses the allocation to construct an object and returns a shared
+   * Construct an object at a pre-allocated location in shared memory and returns a shared
    * pointer to this object. When the shared pointer has no more references, it will be destructed by this pool
-   * collection instance. The start() method must be executed prior to any calls to this.
+   * collection instance via the given deleter. The start() method must be executed prior to any calls to this.
    *
    * @tparam T The object type to be created.
    * @tparam Deleter A copy-constructible class containing operator() that performs destruction of the memory.