Template construction of Pool elements (#641)

* Template construction of Pool elements

The Pool class is used by verona-rt.  The recent changes made this
less nice to consume as an API.

This change makes the construction logic a template parameter to the
Pool. This enables standard allocation to be used from Verona.

* Drop parameter from acquire

Pool::acquire took a list of parameters to initialise the object that it
constructed.  But if this was serviced from the pool, the parameter
would be ignored.  This is not an ideal API.

This PR removes the ability to pass a parameter.

src/snmalloc/mem/corealloc.h

@@ -621,8 +621,7 @@ namespace snmalloc
     template<
       typename Config_ = Config,
       typename = std::enable_if_t<Config_::Options.CoreAllocOwnsLocalState>>
-    CoreAllocator(Range<capptr::bounds::Alloc>& spare, LocalCache* cache)
-    : attached_cache(cache)
+    CoreAllocator(Range<capptr::bounds::Alloc>& spare)
     {
       init(spare);
     }
@@ -1015,9 +1014,42 @@ namespace snmalloc
     }
   };
 
+  template<typename Config>
+  class ConstructCoreAlloc
+  {
+    using CA = CoreAllocator<Config>;
+
+  public:
+    static capptr::Alloc<CA> make()
+    {
+      size_t size = sizeof(CA);
+      size_t round_sizeof = Aal::capptr_size_round(size);
+      size_t request_size = bits::next_pow2(round_sizeof);
+      size_t spare = request_size - round_sizeof;
+
+      auto raw =
+        Config::Backend::template alloc_meta_data<CA>(nullptr, request_size);
+
+      if (raw == nullptr)
+      {
+        Config::Pal::error("Failed to initialise thread local allocator.");
+      }
+
+      capptr::Alloc<void> spare_start = pointer_offset(raw, round_sizeof);
+      Range<capptr::bounds::Alloc> r{spare_start, spare};
+
+      auto p = capptr::Alloc<CA>::unsafe_from(new (raw.unsafe_ptr()) CA(r));
+
+      // Remove excess from the bounds.
+      p = Aal::capptr_bound<CA, capptr::bounds::Alloc>(p, round_sizeof);
+      return p;
+    }
+  };
+
   /**
    * Use this alias to access the pool of allocators throughout snmalloc.
    */
   template<typename Config>
-  using AllocPool = Pool<CoreAllocator<Config>, Config, Config::pool>;
+  using AllocPool =
+    Pool<CoreAllocator<Config>, ConstructCoreAlloc<Config>, Config::pool>;
 } // namespace snmalloc

src/snmalloc/mem/localalloc.h

@@ -393,7 +393,7 @@ namespace snmalloc
       // Initialise the global allocator structures
       ensure_init();
       // Grab an allocator for this thread.
-      init(AllocPool<Config>::acquire(&(this->local_cache)));
+      init(AllocPool<Config>::acquire());
     }
 
     // Return all state in the fast allocator and release the underlying

src/snmalloc/mem/pool.h

@@ -22,7 +22,7 @@ namespace snmalloc
   {
     template<
       typename TT,
-      SNMALLOC_CONCEPT(IsConfig) Config,
+      SNMALLOC_CONCEPT(Constructable<TT>) Construct,
       PoolState<TT>& get_state()>
     friend class Pool;
 
@@ -45,50 +45,10 @@ namespace snmalloc
    * SingletonPoolState::pool is the default provider for the PoolState within
    * the Pool class.
    */
-  template<typename T, SNMALLOC_CONCEPT(IsConfig) Config>
+  template<typename T>
   class SingletonPoolState
   {
-    /**
-     * SFINAE helper.  Matched only if `T` implements `ensure_init`.  Calls it
-     * if it exists.
-     */
-    template<typename SharedStateHandle_>
-    SNMALLOC_FAST_PATH static auto call_ensure_init(SharedStateHandle_*, int)
-      -> decltype(SharedStateHandle_::ensure_init())
-    {
-      static_assert(
-        std::is_same<Config, SharedStateHandle_>::value,
-        "SFINAE parameter, should only be used with Config");
-      SharedStateHandle_::ensure_init();
-    }
-
-    /**
-     * SFINAE helper.  Matched only if `T` does not implement `ensure_init`.
-     * Does nothing if called.
-     */
-    template<typename SharedStateHandle_>
-    SNMALLOC_FAST_PATH static auto call_ensure_init(SharedStateHandle_*, long)
-    {
-      static_assert(
-        std::is_same<Config, SharedStateHandle_>::value,
-        "SFINAE parameter, should only be used with Config");
-    }
-
-    /**
-     * Call `Config::ensure_init()` if it is implemented, do nothing
-     * otherwise.
-     */
-    SNMALLOC_FAST_PATH static void ensure_init()
-    {
-      call_ensure_init<Config>(nullptr, 0);
-    }
-
-    static void make_pool(PoolState<T>*) noexcept
-    {
-      ensure_init();
-      // Default initializer already called on PoolState, no need to use
-      // placement new.
-    }
+    static void make_pool(PoolState<T>*) noexcept {}
 
   public:
     /**
@@ -101,6 +61,22 @@ namespace snmalloc
     }
   };
 
+  /**
+   * @brief Default construct helper for the pool. Just uses `new`.  This can't
+   * be used by the allocator pool as it has not created memory yet.
+   *
+   * @tparam T
+   */
+  template<typename T>
+  class DefaultConstruct
+  {
+  public:
+    static capptr::Alloc<T> make()
+    {
+      return capptr::Alloc<T>::unsafe_from(new T());
+    }
+  };
+
   /**
    * Wrapper class to access a pool of a particular type of object.
    *
@@ -116,13 +92,12 @@ namespace snmalloc
    */
   template<
     typename T,
-    SNMALLOC_CONCEPT(IsConfig) Config,
-    PoolState<T>& get_state() = SingletonPoolState<T, Config>::pool>
+    SNMALLOC_CONCEPT(Constructable<T>) ConstructT = DefaultConstruct<T>,
+    PoolState<T>& get_state() = SingletonPoolState<T>::pool>
   class Pool
   {
   public:
-    template<typename... Args>
-    static T* acquire(Args&&... args)
+    static T* acquire()
     {
       PoolState<T>& pool = get_state();
       {
@@ -141,26 +116,7 @@ namespace snmalloc
         }
       }
 
-      size_t request_size = bits::next_pow2(sizeof(T));
-      size_t round_sizeof = Aal::capptr_size_round(sizeof(T));
-      size_t spare = request_size - round_sizeof;
-
-      auto raw =
-        Config::Backend::template alloc_meta_data<T>(nullptr, request_size);
-
-      if (raw == nullptr)
-      {
-        Config::Pal::error("Failed to initialise thread local allocator.");
-      }
-
-      capptr::Alloc<void> spare_start = pointer_offset(raw, round_sizeof);
-      Range<capptr::bounds::Alloc> r{spare_start, spare};
-
-      auto p = capptr::Alloc<T>::unsafe_from(
-        new (raw.unsafe_ptr()) T(r, std::forward<Args>(args)...));
-
-      // Remove excess from the permissions.
-      p = Aal::capptr_bound<T, capptr::bounds::Alloc>(p, round_sizeof);
+      auto p = ConstructT::make();
 
       FlagLock f(pool.lock);
       p->list_next = pool.list;

src/snmalloc/mem/pooled.h

@@ -15,6 +15,15 @@ namespace snmalloc
   template<class T>
   class PoolState;
 
+#ifdef __cpp_concepts
+  template<typename C, typename T>
+  concept Constructable = requires() {
+                            {
+                              C::make()
+                              } -> ConceptSame<capptr::Alloc<T>>;
+                          };
+#endif // __cpp_concepts
+
   /**
    * Required to be implemented by all types that are pooled.
    *
@@ -29,7 +38,7 @@ namespace snmalloc
   public:
     template<
       typename TT,
-      SNMALLOC_CONCEPT(IsConfig) Config,
+      SNMALLOC_CONCEPT(Constructable<TT>) Construct,
       PoolState<TT>& get_state()>
     friend class Pool;
 

src/test/func/cheri/cheri.cc

@@ -133,8 +133,7 @@ int main()
     static_assert(
       std::is_same_v<decltype(alloc.alloc), LocalAllocator<StandardConfig>>);
 
-    LocalCache lc{&StandardConfig::unused_remote};
-    auto* ca = AllocPool<StandardConfig>::acquire(&lc);
+    auto* ca = AllocPool<StandardConfig>::acquire();
 
     SNMALLOC_CHECK(cap_len_is(ca, sizeof(*ca)));
     SNMALLOC_CHECK(cap_vmem_perm_is(ca, false));

src/test/func/pool/pool.cc

@@ -11,26 +11,25 @@ struct PoolAEntry : Pooled<PoolAEntry>
 {
   int field;
 
-  PoolAEntry(Range<capptr::bounds::Alloc>&) : field(1){};
+  PoolAEntry() : field(1){};
 };
 
-using PoolA = Pool<PoolAEntry, Alloc::Config>;
+using PoolA = Pool<PoolAEntry>;
 
 struct PoolBEntry : Pooled<PoolBEntry>
 {
   int field;
 
-  PoolBEntry(Range<capptr::bounds::Alloc>&) : field(0){};
-  PoolBEntry(Range<capptr::bounds::Alloc>&, int f) : field(f){};
+  PoolBEntry() : field(0){};
 };
 
-using PoolB = Pool<PoolBEntry, Alloc::Config>;
+using PoolB = Pool<PoolBEntry>;
 
 struct PoolLargeEntry : Pooled<PoolLargeEntry>
 {
   std::array<int, 2'000'000> payload;
 
-  PoolLargeEntry(Range<capptr::bounds::Alloc>&)
+  PoolLargeEntry()
   {
     printf(".");
     fflush(stdout);
@@ -41,18 +40,18 @@ struct PoolLargeEntry : Pooled<PoolLargeEntry>
   };
 };
 
-using PoolLarge = Pool<PoolLargeEntry, Alloc::Config>;
+using PoolLarge = Pool<PoolLargeEntry>;
 
 template<bool order>
 struct PoolSortEntry : Pooled<PoolSortEntry<order>>
 {
   int field;
 
-  PoolSortEntry(Range<capptr::bounds::Alloc>&, int f) : field(f){};
+  PoolSortEntry() : field(1){};
 };
 
 template<bool order>
-using PoolSort = Pool<PoolSortEntry<order>, Alloc::Config>;
+using PoolSort = Pool<PoolSortEntry<order>>;
 
 void test_alloc()
 {
@@ -73,13 +72,8 @@ void test_constructor()
   SNMALLOC_CHECK(ptr2 != nullptr);
   SNMALLOC_CHECK(ptr2->field == 0);
 
-  auto ptr3 = PoolB::acquire(1);
-  SNMALLOC_CHECK(ptr3 != nullptr);
-  SNMALLOC_CHECK(ptr3->field == 1);
-
   PoolA::release(ptr1);
   PoolB::release(ptr2);
-  PoolB::release(ptr3);
 }
 
 void test_alloc_many()
@@ -181,8 +175,8 @@ void test_sort()
 
   // This test checks that `sort` puts the elements in the right order,
   // so it is the same as if they had been allocated in that order.
-  auto a1 = PoolSort<order>::acquire(1);
-  auto a2 = PoolSort<order>::acquire(1);
+  auto a1 = PoolSort<order>::acquire();
+  auto a2 = PoolSort<order>::acquire();
 
   auto position1 = position(a1);
   auto position2 = position(a2);
@@ -201,8 +195,8 @@ void test_sort()
 
   PoolSort<order>::sort();
 
-  auto b1 = PoolSort<order>::acquire(1);
-  auto b2 = PoolSort<order>::acquire(1);
+  auto b1 = PoolSort<order>::acquire();
+  auto b2 = PoolSort<order>::acquire();
 
   SNMALLOC_CHECK(position1 == position(b1));
   SNMALLOC_CHECK(position2 == position(b2));