WIP

Author: mjp41

Diff for: docs/AddressSpace.md

@@ -8,8 +8,6 @@ We give here some notes on the internal orchestration.
 Consider a first, "small" allocation (typically less than a platform page); such allocations showcase more of the machinery.
 For simplicity, we assume that
 
-TODO CoreAllocator rewrite here:
-
 - this is not an `OPEN_ENCLAVE` build,
 - the `BackendAllocator` has not been told to use a `fixed_range`,
 - this is not a `SNMALLOC_CHECK_CLIENT` build, and
@@ -18,10 +16,10 @@ TODO CoreAllocator rewrite here:
 Since this is the first allocation, all the internal caches will be empty, and so we will hit all the slow paths.
 For simplicity, we gloss over much of the "lazy initialization" that would actually be implied by a first allocation.
 
-1. The `LocalAlloc::small_alloc` finds that it cannot satisfy the request because its `LocalCache` lacks a free list for this size class.
-   The request is delegated, unchanged, to `CoreAllocator::small_alloc`.
+1. The `Allocator::small_alloc` finds that it cannot satisfy the request because its lacks a fast free list for this size class.
+   The request is delegated, unchanged, to `Allocator::small_refill`.
 
-2. The `CoreAllocator` has no active slab for this sizeclass, so `CoreAllocator::small_alloc_slow` delegates to `BackendAllocator::alloc_chunk`.
+2. The `Allocator` has no active slab for this sizeclass, so `Allocator::small_refill_slow` delegates to `BackendAllocator::alloc_chunk`.
    At this point, the allocation request is enlarged to one or a few chunks (a small counting number multiple of `MIN_CHUNK_SIZE`, which is typically 16KiB); see `sizeclass_to_slab_size`.
 
 3. `BackendAllocator::alloc_chunk` at this point splits the allocation request in two, allocating both the chunk's metadata structure (of size `PAGEMAP_METADATA_STRUCT_SIZE`) and the chunk itself (a multiple of `MIN_CHUNK_SIZE`).

Diff for: src/snmalloc/backend_helpers/commonconfig.h

@@ -30,50 +30,27 @@ namespace snmalloc
   {
     /**
      * Should allocators have inline message queues?  If this is true then
-     * the `CoreAllocator` is responsible for allocating the
+     * the `Allocator` is responsible for allocating the
      * `RemoteAllocator` that contains its message queue.  If this is false
      * then the `RemoteAllocator` must be separately allocated and provided
-     * to the `CoreAllocator` before it is used.
-     *
-     * Setting this to `false` currently requires also setting
-     * `LocalAllocSupportsLazyInit` to false so that the `CoreAllocator` can
-     * be provided to the `LocalAllocator` fully initialised but in the
-     * future it may be possible to allocate the `RemoteAllocator` via
-     * `alloc_meta_data` or a similar API in the back end.
+     * to the `Allocator` before it is used.
      */
     bool IsQueueInline = true;
 
     /**
-     * Does the `CoreAllocator` own a `Backend::LocalState` object?  If this is
-     * true then the `CoreAllocator` is responsible for allocating and
+     * Does the `Allocator` own a `Backend::LocalState` object?  If this is
+     * true then the `Allocator` is responsible for allocating and
      * deallocating a local state object, otherwise the surrounding code is
      * responsible for creating it.
-     *
-     * Use cases that set this to false will probably also need to set
-     * `LocalAllocSupportsLazyInit` to false so that they can provide the local
-     * state explicitly during allocator creation.
      */
-    bool CoreAllocOwnsLocalState = true;
+    bool AllocOwnsLocalState = true;
 
     /**
-     * Are `CoreAllocator` allocated by the pool allocator?  If not then the
+     * Are `Allocator` allocated by the pool allocator?  If not then the
      * code embedding this snmalloc configuration is responsible for allocating
-     * `CoreAllocator` instances.
-     *
-     * Users setting this flag must also set `LocalAllocSupportsLazyInit` to
-     * false currently because there is no alternative mechanism for allocating
-     * core allocators.  This may change in future versions.
+     * `Allocator` instances.
      */
-    bool CoreAllocIsPoolAllocated = true;
-
-    /**
-     * Do `LocalAllocator` instances in this configuration support lazy
-     * initialisation?  If so, then the first exit from a fast path will
-     * trigger allocation of a `CoreAllocator` and associated state.  If not
-     * then the code embedding this configuration of snmalloc is responsible
-     * for allocating core allocators.
-     */
-    bool LocalAllocSupportsLazyInit = true;
+    bool AllocIsPoolAllocated = true;
 
     /**
      * Are the front and back pointers to the message queue in a RemoteAllocator

Diff for: src/snmalloc/global/globalalloc.h

@@ -9,7 +9,7 @@ namespace snmalloc
   inline static void cleanup_unused()
   {
     static_assert(
-      Config_::Options.CoreAllocIsPoolAllocated,
+      Config_::Options.AllocIsPoolAllocated,
       "Global cleanup is available only for pool-allocated configurations");
     // Call this periodically to free and coalesce memory allocated by
     // allocators that are not currently in use by any thread.
@@ -41,7 +41,7 @@ namespace snmalloc
   inline static void debug_check_empty(bool* result = nullptr)
   {
     static_assert(
-      Config_::Options.CoreAllocIsPoolAllocated,
+      Config_::Options.AllocIsPoolAllocated,
       "Global status is available only for pool-allocated configurations");
     // This is a debugging function. It checks that all memory from all
     // allocators has been freed.
@@ -106,7 +106,7 @@ namespace snmalloc
   inline static void debug_in_use(size_t count)
   {
     static_assert(
-      Config_::Options.CoreAllocIsPoolAllocated,
+      Config_::Options.AllocIsPoolAllocated,
       "Global status is available only for pool-allocated configurations");
     auto alloc = AllocPool<Config_>::iterate();
     while (alloc != nullptr)

Diff for: src/snmalloc/global/threadalloc.h

@@ -114,46 +114,33 @@ namespace snmalloc
       {
         bool post_teardown = teardown_called;
 
-        if constexpr (!Config::Options.LocalAllocSupportsLazyInit)
-        {
-          SNMALLOC_CHECK(
-            false &&
-            "lazy_init called on an allocator that doesn't support lazy "
-            "initialisation");
-          // Unreachable, but needed to keep the type checker happy in deducing
-          // the return type of this function.
-          return static_cast<decltype(action(args...))>(nullptr);
-        }
-        else
-        {
-          alloc = AllocPool<Config>::acquire();
+        alloc = AllocPool<Config>::acquire();
 
-          // register_clean_up must be called after init.  register clean up
-          // may be implemented with allocation, so need to ensure we have a
-          // valid allocator at this point.
-          if (!post_teardown)
-          {
-            // Must be called at least once per thread.
-            // A pthread implementation only calls the thread destruction handle
-            // if the key has been set.
-            Subclass::register_clean_up();
+        // register_clean_up must be called after init.  register clean up
+        // may be implemented with allocation, so need to ensure we have a
+        // valid allocator at this point.
+        if (!post_teardown)
+        {
+          // Must be called at least once per thread.
+          // A pthread implementation only calls the thread destruction handle
+          // if the key has been set.
+          Subclass::register_clean_up();
 
-            // Perform underlying operation
-            return r(alloc, args...);
-          }
+          // Perform underlying operation
+          return r(alloc, args...);
+        }
 
-          OnDestruct od([]() {
+        OnDestruct od([]() {
 #  ifdef SNMALLOC_TRACING
-            message<1024>("post_teardown flush()");
+          message<1024>("post_teardown flush()");
 #  endif
-            // We didn't have an allocator because the thread is being torndown.
-            // We need to return any local state, so we don't leak it.
-            ThreadAlloc::teardown();
-          });
+          // We didn't have an allocator because the thread is being torndown.
+          // We need to return any local state, so we don't leak it.
+          ThreadAlloc::teardown();
+        });
 
-          // Perform underlying operation
-          return r(alloc, args...);
-        }
+        // Perform underlying operation
+        return r(alloc, args...);
       }
 
     public:

Diff for: src/snmalloc/mem/backend_concept.h

@@ -164,14 +164,14 @@ namespace snmalloc
     } &&
     (
                        requires() {
-                         Config::Options.CoreAllocIsPoolAllocated == true;
+                         Config::Options.AllocIsPoolAllocated == true;
                          typename Config::GlobalPoolState;
                          {
                            Config::pool()
                            } -> ConceptSame<typename Config::GlobalPoolState&>;
                        } ||
                        requires() {
-                         Config::Options.CoreAllocIsPoolAllocated == false;
+                         Config::Options.AllocIsPoolAllocated == false;
                        });
 
   /**