From d4226a1ea2d536e0ad3b6dfeb6a69ea85e1800ae Mon Sep 17 00:00:00 2001
From: David Chisnall <davidchisnall@users.noreply.github.com>
Date: Mon, 11 Apr 2022 13:28:03 +0100
Subject: [PATCH] Remove indirection of state in ranges. (#505)

This doesn't give any extra flexibility: the range itself can be either
a stateless class, a class with no per-instance state that stores all of
static fields, or a class with stateful instances.  It did add a
requirement that every range implementation added an indirection layer.
---
 src/snmalloc/backend/backend.h                | 36 +++++++++----------
 src/snmalloc/backend_helpers/commitrange.h    | 19 ++--------
 src/snmalloc/backend_helpers/empty_range.h    | 12 -------
 src/snmalloc/backend_helpers/globalrange.h    | 21 +++--------
 .../backend_helpers/largebuddyrange.h         | 27 ++++----------
 .../backend_helpers/pagemapregisterrange.h    | 17 ++-------
 src/snmalloc/backend_helpers/palrange.h       | 14 --------
 .../backend_helpers/smallbuddyrange.h         | 23 +++---------
 src/snmalloc/backend_helpers/statsrange.h     | 21 +++--------
 src/snmalloc/backend_helpers/subrange.h       | 17 ++-------
 10 files changed, 45 insertions(+), 162 deletions(-)

diff --git a/src/snmalloc/backend/backend.h b/src/snmalloc/backend/backend.h
index e791fe2..5bf16a6 100644
--- a/src/snmalloc/backend/backend.h
+++ b/src/snmalloc/backend/backend.h
@@ -202,17 +202,17 @@ namespace snmalloc
 
     struct LocalState
     {
-      typename ObjectRange::State object_range;
+      ObjectRange object_range;
 
 #ifdef SNMALLOC_META_PROTECTED
-      typename MetaRange::State meta_range;
+      MetaRange meta_range;
 
-      typename MetaRange::State& get_meta_range()
+      MetaRange& get_meta_range()
       {
         return meta_range;
       }
 #else
-      typename ObjectRange::State& get_meta_range()
+      ObjectRange& get_meta_range()
       {
         return object_range;
       }
@@ -243,8 +243,8 @@ namespace snmalloc
         capptr::Chunk<void>(heap_base),
         heap_length,
         [&](capptr::Chunk<void> p, size_t sz, bool) {
-          typename GlobalR::State g;
-          g->dealloc_range(p, sz);
+          GlobalR g;
+          g.dealloc_range(p, sz);
         });
     }
 
@@ -266,15 +266,15 @@ namespace snmalloc
       capptr::Chunk<void> p;
       if (local_state != nullptr)
       {
-        p = local_state->get_meta_range()->alloc_range_with_leftover(size);
+        p = local_state->get_meta_range().alloc_range_with_leftover(size);
       }
       else
       {
         static_assert(
           GlobalMetaRange::ConcurrencySafe,
           "Global meta data range needs to be concurrency safe.");
-        typename GlobalMetaRange::State global_state;
-        p = global_state->alloc_range(bits::next_pow2(size));
+        GlobalMetaRange global_state;
+        p = global_state.alloc_range(bits::next_pow2(size));
       }
 
       if (p == nullptr)
@@ -299,7 +299,7 @@ namespace snmalloc
       SNMALLOC_ASSERT(size >= MIN_CHUNK_SIZE);
 
       auto meta_cap =
-        local_state.get_meta_range()->alloc_range(sizeof(SlabMetadata));
+        local_state.get_meta_range().alloc_range(sizeof(SlabMetadata));
 
       auto meta = meta_cap.template as_reinterpret<SlabMetadata>().unsafe_ptr();
 
@@ -309,14 +309,14 @@ namespace snmalloc
         return {nullptr, nullptr};
       }
 
-      auto p = local_state.object_range->alloc_range(size);
+      auto p = local_state.object_range.alloc_range(size);
 
 #ifdef SNMALLOC_TRACING
       message<1024>("Alloc chunk: {} ({})", p.unsafe_ptr(), size);
 #endif
       if (p == nullptr)
       {
-        local_state.get_meta_range()->dealloc_range(
+        local_state.get_meta_range().dealloc_range(
           meta_cap, sizeof(SlabMetadata));
         errno = ENOMEM;
 #ifdef SNMALLOC_TRACING
@@ -356,26 +356,26 @@ namespace snmalloc
         Pagemap::get_metaentry(address_cast(alloc)).get_slab_metadata());
       Pagemap::set_metaentry(address_cast(alloc), size, t);
 
-      local_state.get_meta_range()->dealloc_range(
+      local_state.get_meta_range().dealloc_range(
         capptr::Chunk<void>(&slab_metadata), sizeof(SlabMetadata));
 
       // On non-CHERI platforms, we don't need to re-derive to get a pointer to
       // the chunk.  On CHERI platforms this will need to be stored in the
       // SlabMetadata or similar.
       capptr::Chunk<void> chunk{alloc.unsafe_ptr()};
-      local_state.object_range->dealloc_range(chunk, size);
+      local_state.object_range.dealloc_range(chunk, size);
     }
 
     static size_t get_current_usage()
     {
-      typename StatsR::State stats_state;
-      return stats_state->get_current_usage();
+      StatsR stats_state;
+      return stats_state.get_current_usage();
     }
 
     static size_t get_peak_usage()
     {
-      typename StatsR::State stats_state;
-      return stats_state->get_peak_usage();
+      StatsR stats_state;
+      return stats_state.get_peak_usage();
     }
   };
 } // namespace snmalloc
diff --git a/src/snmalloc/backend_helpers/commitrange.h b/src/snmalloc/backend_helpers/commitrange.h
index c9cdb39..d2e6d54 100644
--- a/src/snmalloc/backend_helpers/commitrange.h
+++ b/src/snmalloc/backend_helpers/commitrange.h
@@ -7,22 +7,9 @@ namespace snmalloc
   template<typename ParentRange, typename PAL>
   class CommitRange
   {
-    typename ParentRange::State parent{};
+    ParentRange parent{};
 
   public:
-    class State
-    {
-      CommitRange commit_range{};
-
-    public:
-      constexpr State() = default;
-
-      CommitRange* operator->()
-      {
-        return &commit_range;
-      }
-    };
-
     static constexpr bool Aligned = ParentRange::Aligned;
 
     static constexpr bool ConcurrencySafe = ParentRange::ConcurrencySafe;
@@ -31,7 +18,7 @@ namespace snmalloc
 
     capptr::Chunk<void> alloc_range(size_t size)
     {
-      auto range = parent->alloc_range(size);
+      auto range = parent.alloc_range(size);
       if (range != nullptr)
         PAL::template notify_using<NoZero>(range.unsafe_ptr(), size);
       return range;
@@ -40,7 +27,7 @@ namespace snmalloc
     void dealloc_range(capptr::Chunk<void> base, size_t size)
     {
       PAL::notify_not_using(base.unsafe_ptr(), size);
-      parent->dealloc_range(base, size);
+      parent.dealloc_range(base, size);
     }
   };
 } // namespace snmalloc
diff --git a/src/snmalloc/backend_helpers/empty_range.h b/src/snmalloc/backend_helpers/empty_range.h
index b5102a1..6507a01 100644
--- a/src/snmalloc/backend_helpers/empty_range.h
+++ b/src/snmalloc/backend_helpers/empty_range.h
@@ -6,18 +6,6 @@ namespace snmalloc
   class EmptyRange
   {
   public:
-    class State
-    {
-    public:
-      EmptyRange* operator->()
-      {
-        static EmptyRange range{};
-        return &range;
-      }
-
-      constexpr State() = default;
-    };
-
     static constexpr bool Aligned = true;
 
     static constexpr bool ConcurrencySafe = true;
diff --git a/src/snmalloc/backend_helpers/globalrange.h b/src/snmalloc/backend_helpers/globalrange.h
index 016701e..b21d4d0 100644
--- a/src/snmalloc/backend_helpers/globalrange.h
+++ b/src/snmalloc/backend_helpers/globalrange.h
@@ -11,28 +11,15 @@ namespace snmalloc
   template<typename ParentRange>
   class GlobalRange
   {
-    typename ParentRange::State parent{};
+    SNMALLOC_REQUIRE_CONSTINIT static inline ParentRange parent{};
 
     /**
      * This is infrequently used code, a spin lock simplifies the code
      * considerably, and should never be on the fast path.
      */
-    FlagWord spin_lock{};
+    SNMALLOC_REQUIRE_CONSTINIT static inline FlagWord spin_lock{};
 
   public:
-    class State
-    {
-      SNMALLOC_REQUIRE_CONSTINIT static inline GlobalRange global_range{};
-
-    public:
-      constexpr GlobalRange* operator->()
-      {
-        return &global_range;
-      }
-
-      constexpr State() = default;
-    };
-
     static constexpr bool Aligned = ParentRange::Aligned;
 
     static constexpr bool ConcurrencySafe = true;
@@ -42,13 +29,13 @@ namespace snmalloc
     capptr::Chunk<void> alloc_range(size_t size)
     {
       FlagLock lock(spin_lock);
-      return parent->alloc_range(size);
+      return parent.alloc_range(size);
     }
 
     void dealloc_range(capptr::Chunk<void> base, size_t size)
     {
       FlagLock lock(spin_lock);
-      parent->dealloc_range(base, size);
+      parent.dealloc_range(base, size);
     }
   };
 } // namespace snmalloc
diff --git a/src/snmalloc/backend_helpers/largebuddyrange.h b/src/snmalloc/backend_helpers/largebuddyrange.h
index e3b5ecc..3248df1 100644
--- a/src/snmalloc/backend_helpers/largebuddyrange.h
+++ b/src/snmalloc/backend_helpers/largebuddyrange.h
@@ -173,7 +173,7 @@ namespace snmalloc
     bool Consolidate = true>
   class LargeBuddyRange
   {
-    typename ParentRange::State parent{};
+    ParentRange parent{};
 
     static constexpr size_t REFILL_SIZE = bits::one_at_bit(REFILL_SIZE_BITS);
 
@@ -192,7 +192,7 @@ namespace snmalloc
     {
       static_assert(
         MAX_SIZE_BITS != (bits::BITS - 1), "Don't set SFINAE parameter");
-      parent->dealloc_range(base, size);
+      parent.dealloc_range(base, size);
     }
 
     void dealloc_overflow(capptr::Chunk<void> overflow)
@@ -201,7 +201,7 @@ namespace snmalloc
       {
         if (overflow != nullptr)
         {
-          parent->dealloc_range(overflow, bits::one_at_bit(MAX_SIZE_BITS));
+          parent.dealloc_range(overflow, bits::one_at_bit(MAX_SIZE_BITS));
         }
       }
       else
@@ -250,10 +250,10 @@ namespace snmalloc
         // TODO have to add consolidation blocker for these cases.
         if (size >= REFILL_SIZE)
         {
-          return parent->alloc_range(size);
+          return parent.alloc_range(size);
         }
 
-        auto refill_range = parent->alloc_range(REFILL_SIZE);
+        auto refill_range = parent.alloc_range(REFILL_SIZE);
         if (refill_range != nullptr)
           add_range(pointer_offset(refill_range, size), REFILL_SIZE - size);
         return refill_range;
@@ -270,7 +270,7 @@ namespace snmalloc
       auto refill_size = bits::max(needed_size, REFILL_SIZE);
       while (needed_size <= refill_size)
       {
-        auto refill = parent->alloc_range(refill_size);
+        auto refill = parent.alloc_range(refill_size);
 
         if (refill != nullptr)
         {
@@ -292,19 +292,6 @@ namespace snmalloc
     }
 
   public:
-    class State
-    {
-      LargeBuddyRange buddy_range;
-
-    public:
-      LargeBuddyRange* operator->()
-      {
-        return &buddy_range;
-      }
-
-      constexpr State() = default;
-    };
-
     static constexpr bool Aligned = true;
 
     static constexpr bool ConcurrencySafe = false;
@@ -319,7 +306,7 @@ namespace snmalloc
       if (size >= (bits::one_at_bit(MAX_SIZE_BITS) - 1))
       {
         if (ParentRange::Aligned)
-          return parent->alloc_range(size);
+          return parent.alloc_range(size);
 
         return nullptr;
       }
diff --git a/src/snmalloc/backend_helpers/pagemapregisterrange.h b/src/snmalloc/backend_helpers/pagemapregisterrange.h
index 059cc15..17ba6e1 100644
--- a/src/snmalloc/backend_helpers/pagemapregisterrange.h
+++ b/src/snmalloc/backend_helpers/pagemapregisterrange.h
@@ -9,22 +9,9 @@ namespace snmalloc
     typename ParentRange>
   class PagemapRegisterRange
   {
-    typename ParentRange::State state{};
+    ParentRange state{};
 
   public:
-    class State
-    {
-      PagemapRegisterRange range;
-
-    public:
-      PagemapRegisterRange* operator->()
-      {
-        return &range;
-      }
-
-      constexpr State() = default;
-    };
-
     constexpr PagemapRegisterRange() = default;
 
     static constexpr bool Aligned = ParentRange::Aligned;
@@ -33,7 +20,7 @@ namespace snmalloc
 
     capptr::Chunk<void> alloc_range(size_t size)
     {
-      auto base = state->alloc_range(size);
+      auto base = state.alloc_range(size);
 
       if (base != nullptr)
         Pagemap::register_range(address_cast(base), size);
diff --git a/src/snmalloc/backend_helpers/palrange.h b/src/snmalloc/backend_helpers/palrange.h
index f447325..0962e00 100644
--- a/src/snmalloc/backend_helpers/palrange.h
+++ b/src/snmalloc/backend_helpers/palrange.h
@@ -7,20 +7,6 @@ namespace snmalloc
   class PalRange
   {
   public:
-    class State
-    {
-    public:
-      PalRange* operator->()
-      {
-        // There is no state required for the PalRange
-        // using a global just to satisfy the typing.
-        static PalRange range{};
-        return &range;
-      }
-
-      constexpr State() = default;
-    };
-
     static constexpr bool Aligned = pal_supports<AlignedAllocation, PAL>;
 
     // Note we have always assumed the Pals to provide a concurrency safe
diff --git a/src/snmalloc/backend_helpers/smallbuddyrange.h b/src/snmalloc/backend_helpers/smallbuddyrange.h
index b3ccc45..b212323 100644
--- a/src/snmalloc/backend_helpers/smallbuddyrange.h
+++ b/src/snmalloc/backend_helpers/smallbuddyrange.h
@@ -145,7 +145,7 @@ namespace snmalloc
   template<typename ParentRange>
   class SmallBuddyRange
   {
-    typename ParentRange::State parent{};
+    ParentRange parent{};
 
     static constexpr size_t MIN_BITS =
       bits::next_pow2_bits_const(sizeof(FreeChunk));
@@ -164,13 +164,13 @@ namespace snmalloc
             buddy_small.add_block(base.as_reinterpret<FreeChunk>(), align)
               .template as_reinterpret<void>();
           if (overflow != nullptr)
-            parent->dealloc_range(overflow, bits::one_at_bit(MIN_CHUNK_BITS));
+            parent.dealloc_range(overflow, bits::one_at_bit(MIN_CHUNK_BITS));
         });
     }
 
     capptr::Chunk<void> refill(size_t size)
     {
-      auto refill = parent->alloc_range(MIN_CHUNK_SIZE);
+      auto refill = parent.alloc_range(MIN_CHUNK_SIZE);
 
       if (refill != nullptr)
         add_range(pointer_offset(refill, size), MIN_CHUNK_SIZE - size);
@@ -179,19 +179,6 @@ namespace snmalloc
     }
 
   public:
-    class State
-    {
-      SmallBuddyRange buddy_range;
-
-    public:
-      SmallBuddyRange* operator->()
-      {
-        return &buddy_range;
-      }
-
-      constexpr State() = default;
-    };
-
     static constexpr bool Aligned = true;
     static_assert(ParentRange::Aligned, "ParentRange must be aligned");
 
@@ -203,7 +190,7 @@ namespace snmalloc
     {
       if (size >= MIN_CHUNK_SIZE)
       {
-        return parent->alloc_range(size);
+        return parent.alloc_range(size);
       }
 
       auto result = buddy_small.remove_block(size);
@@ -238,7 +225,7 @@ namespace snmalloc
     {
       if (size >= MIN_CHUNK_SIZE)
       {
-        parent->dealloc_range(base, size);
+        parent.dealloc_range(base, size);
         return;
       }
 
diff --git a/src/snmalloc/backend_helpers/statsrange.h b/src/snmalloc/backend_helpers/statsrange.h
index 0693983..da874b2 100644
--- a/src/snmalloc/backend_helpers/statsrange.h
+++ b/src/snmalloc/backend_helpers/statsrange.h
@@ -10,25 +10,12 @@ namespace snmalloc
   template<typename ParentRange>
   class StatsRange
   {
-    typename ParentRange::State parent{};
+    ParentRange parent{};
 
     static inline std::atomic<size_t> current_usage{};
     static inline std::atomic<size_t> peak_usage{};
 
   public:
-    class State
-    {
-      StatsRange stats_range{};
-
-    public:
-      constexpr StatsRange* operator->()
-      {
-        return &stats_range;
-      }
-
-      constexpr State() = default;
-    };
-
     static constexpr bool Aligned = ParentRange::Aligned;
 
     static constexpr bool ConcurrencySafe = ParentRange::ConcurrencySafe;
@@ -37,7 +24,7 @@ namespace snmalloc
 
     capptr::Chunk<void> alloc_range(size_t size)
     {
-      auto result = parent->alloc_range(size);
+      auto result = parent.alloc_range(size);
       if (result != nullptr)
       {
         auto prev = current_usage.fetch_add(size);
@@ -54,7 +41,7 @@ namespace snmalloc
     void dealloc_range(capptr::Chunk<void> base, size_t size)
     {
       current_usage -= size;
-      parent->dealloc_range(base, size);
+      parent.dealloc_range(base, size);
     }
 
     size_t get_current_usage()
@@ -67,4 +54,4 @@ namespace snmalloc
       return peak_usage.load();
     }
   };
-} // namespace snmalloc
\ No newline at end of file
+} // namespace snmalloc
diff --git a/src/snmalloc/backend_helpers/subrange.h b/src/snmalloc/backend_helpers/subrange.h
index 44c1db9..3c6617d 100644
--- a/src/snmalloc/backend_helpers/subrange.h
+++ b/src/snmalloc/backend_helpers/subrange.h
@@ -11,22 +11,9 @@ namespace snmalloc
   template<typename ParentRange, typename PAL, size_t RATIO_BITS>
   class SubRange
   {
-    typename ParentRange::State parent{};
+    ParentRange parent{};
 
   public:
-    class State
-    {
-      SubRange sub_range{};
-
-    public:
-      constexpr State() = default;
-
-      SubRange* operator->()
-      {
-        return &sub_range;
-      }
-    };
-
     constexpr SubRange() = default;
 
     static constexpr bool Aligned = ParentRange::Aligned;
@@ -38,7 +25,7 @@ namespace snmalloc
       SNMALLOC_ASSERT(bits::is_pow2(sub_size));
 
       auto full_size = sub_size << RATIO_BITS;
-      auto overblock = parent->alloc_range(full_size);
+      auto overblock = parent.alloc_range(full_size);
       if (overblock == nullptr)
         return nullptr;