From 554db3997dba26aeafbb859946e24d2da6a49c6b Mon Sep 17 00:00:00 2001
From: Nathaniel Wesley Filardo <nfilardo@microsoft.com>
Date: Sun, 17 Oct 2021 18:03:39 +0100
Subject: [PATCH] localalloc::dealloc tweak size math for large objects

Avoid computing bits::next_pow2_bits(1 << n).  Even if the compiler can see
through enough of the algebra, it's surely more direct to just use n.

While here, slightly expand documentation about what's going on with the
"sizeclass" encoded into MetaEntry-s.
---
 src/backend/address_space_core.h |  5 ++++-
 src/mem/localalloc.h             | 18 ++++++++++++++----
 src/mem/metaslab.h               | 12 ++++++++++++
 src/mem/sizeclasstable.h         | 10 ++++++++++
 4 files changed, 40 insertions(+), 5 deletions(-)

diff --git a/src/backend/address_space_core.h b/src/backend/address_space_core.h
index d3088c0..03308c0 100644
--- a/src/backend/address_space_core.h
+++ b/src/backend/address_space_core.h
@@ -77,13 +77,16 @@ namespace snmalloc
     {
       if (align_bits >= MIN_CHUNK_BITS)
       {
-        // The pagemap stores MetaEntrys, abuse the metaslab field to be the
+        // The pagemap stores `MetaEntry`s; abuse the metaslab field to be the
         // next block in the stack of blocks.
         //
         // The pagemap entries here have nullptr (i.e., fake_large_remote) as
         // their remote, and so other accesses to the pagemap (by
         // external_pointer, for example) will not attempt to follow this
         // "Metaslab" pointer.
+        //
+        // dealloc() can reject attempts to free such MetaEntry-s due to the
+        // zero sizeclass.
         MetaEntry t(reinterpret_cast<Metaslab*>(next.unsafe_ptr()), nullptr, 0);
         Pagemap::set_metaentry(local_state, address_cast(base), 1, t);
         return;
diff --git a/src/mem/localalloc.h b/src/mem/localalloc.h
index 7c48b2a..38ee42b 100644
--- a/src/mem/localalloc.h
+++ b/src/mem/localalloc.h
@@ -508,21 +508,31 @@ namespace snmalloc
       // Large deallocation or null.
       if (likely(p_tame != nullptr))
       {
-        // Check this is managed by this pagemap.
-        check_client(entry.get_sizeclass() != 0, "Not allocated by snmalloc.");
+        size_t entry_sizeclass = entry.get_sizeclass();
 
-        size_t size = bits::one_at_bit(entry.get_sizeclass());
+        // Check this is managed by this pagemap.
+        //
+        // TODO: Should this be tested even in the !CHECK_CLIENT case?  Things
+        // go fairly pear-shaped, with the ASM's ranges[] getting cross-linked
+        // with a ChunkAllocator's chunk_stack[0], which seems bad.
+        check_client(entry_sizeclass != 0, "Not allocated by snmalloc.");
+
+        size_t size = bits::one_at_bit(entry_sizeclass);
+        size_t slab_sizeclass =
+          metaentry_chunk_sizeclass_to_slab_sizeclass(entry_sizeclass);
 
         // Check for start of allocation.
         check_client(
           pointer_align_down(p_tame, size) == p_tame,
           "Not start of an allocation.");
 
-        size_t slab_sizeclass = large_size_to_chunk_sizeclass(size);
 #  ifdef SNMALLOC_TRACING
         std::cout << "Large deallocation: " << size
                   << " chunk sizeclass: " << slab_sizeclass << std::endl;
+#  else
+        UNUSED(size);
 #  endif
+
         ChunkRecord* slab_record =
           reinterpret_cast<ChunkRecord*>(entry.get_metaslab());
         /*
diff --git a/src/mem/metaslab.h b/src/mem/metaslab.h
index 9ff584c..ce9d610 100644
--- a/src/mem/metaslab.h
+++ b/src/mem/metaslab.h
@@ -188,6 +188,18 @@ namespace snmalloc
   class MetaEntry
   {
     Metaslab* meta{nullptr};
+
+    /**
+     * A bit-packed pointer to the owning allocator (if any), and the sizeclass
+     * of this chunk.  The sizeclass here is itself a union between two cases:
+     *
+     *  * log_2(size), at least MIN_CHUNK_BITS, for large allocations.
+     *
+     *  * a value in [0, NUM_SIZECLASSES] for small allocations.  These may be
+     *    directly passed to the sizeclass (not slab_sizeclass) functions of
+     *    sizeclasstable.h
+     *
+     */
     uintptr_t remote_and_sizeclass{0};
 
   public:
diff --git a/src/mem/sizeclasstable.h b/src/mem/sizeclasstable.h
index c406ac1..ebd091f 100644
--- a/src/mem/sizeclasstable.h
+++ b/src/mem/sizeclasstable.h
@@ -165,6 +165,16 @@ namespace snmalloc
     return bits::one_at_bit(MIN_CHUNK_BITS + sizeclass);
   }
 
+  /**
+   * For large allocations, the metaentry stores the raw log_2 of the size,
+   * which must be shifted into the index space of slab_sizeclass-es.
+   */
+  inline static size_t
+  metaentry_chunk_sizeclass_to_slab_sizeclass(sizeclass_t sizeclass)
+  {
+    return sizeclass - MIN_CHUNK_BITS;
+  }
+
   inline constexpr static uint16_t
   sizeclass_to_slab_object_count(sizeclass_t sizeclass)
   {