From df1dfa55db1fb286565422ad19652541e913856b Mon Sep 17 00:00:00 2001
From: Matthew Parkinson <mattpark@microsoft.com>
Date: Thu, 21 Nov 2019 09:51:50 +0000
Subject: [PATCH] improved comments and rename field

Replaced used with needed, the number of objects needed empty this slab.

Some mild improvement to the comments.
---
 src/mem/metaslab.h | 46 ++++++++++++++++++++++++++++------------------
 src/mem/slab.h     | 11 ++++++-----
 2 files changed, 34 insertions(+), 23 deletions(-)

diff --git a/src/mem/metaslab.h b/src/mem/metaslab.h
index 6ab7b92..f364ce6 100644
--- a/src/mem/metaslab.h
+++ b/src/mem/metaslab.h
@@ -30,13 +30,27 @@ namespace snmalloc
   class Metaslab
   {
   public:
-    // Ppinter to first free entry in this slab
+    /**
+     *  Pointer to first free entry in this slab
+     *
+     *  The list will (allocated - needed - 1) long. The -1 is
+     *  for the `link` element which is not in the free list.
+     */
     void* head;
 
-    // How many entries are not in the free list of  slab.
-    uint16_t used = 0;
+    /**
+     *  How many entries are not in the free list of slab, i.e.
+     *  how many entries are needed to fully free this slab.
+     *
+     *  In the case of a fully allocated slab, where link==1 needed
+     *  will be 1. This enables 'return_object' to detect the slow path
+     *  case with a single operation subtract and test.
+     */
+    uint16_t needed = 0;
 
-    // How many entries have been allocated from this slab.
+    /**
+     *  How many entries have been allocated from this slab.
+     */
     uint16_t allocated;
 
     // When a slab has free space it will be on the has space list for
@@ -48,25 +62,21 @@ namespace snmalloc
     // Initially zero to encode the superslabs relative list of slabs.
     uint8_t next = 0;
 
-    void add_use()
-    {
-      used++;
-    }
-
     /**
-     * Removes a use, if the slab is either
-     *  - empty after removing the use, or
-     *  - was full before the substraction
+     * Updates statistics for adding an entry to the free list, if the
+     * slab is either
+     *  - empty adding the entry to the free list, or
+     *  - was full before the subtraction
      * this returns true, otherwise returns false.
      **/
-    bool sub_use()
+    bool return_object()
     {
-      return (--used) == 0;
+      return (--needed) == 0;
     }
 
     bool is_unused()
     {
-      return used == 0;
+      return needed == 0;
     }
 
     bool is_full()
@@ -81,9 +91,9 @@ namespace snmalloc
       assert(head == nullptr);
       assert(link != 1);
       link = 1;
-      // Set used to 1, so that "sub_use" will return true after calling
+      // Set needed to 1, so that "return_object" will return true after calling
       // set_full
-      used = 1;
+      needed = 1;
     }
 
     SlabLink* get_link(Slab* slab)
@@ -192,7 +202,7 @@ namespace snmalloc
 
       size_t size = sizeclass_to_size(sizeclass);
       size_t offset = get_initial_offset(sizeclass, is_short);
-      size_t accounted_for = used * size + offset;
+      size_t accounted_for = needed * size + offset;
 
       // Block is not full
       assert(SLAB_SIZE > accounted_for);
diff --git a/src/mem/slab.h b/src/mem/slab.h
index db30997..4606a87 100644
--- a/src/mem/slab.h
+++ b/src/mem/slab.h
@@ -56,8 +56,9 @@ namespace snmalloc
         bumpptr += meta.allocated * rsize;
         if (bumpptr == SLAB_SIZE)
         {
-          meta.add_use();
-          assert(meta.used == meta.allocated);
+          // Everything is in use, so we need all entries to be
+          // return before we can reclaim this slab.
+          meta.needed = meta.allocated;
 
           void* link = pointer_offset(this, meta.link);
           p = remove_cache_friendly_offset(link, meta.sizeclass);
@@ -119,7 +120,7 @@ namespace snmalloc
         fast_free_list.value = next;
         // Treat stealing the free list as allocating it all.
         // Link is not in use, i.e. - 1 is required.
-        meta.used = meta.allocated - 1;
+        meta.needed = meta.allocated - 1;
 
         p = remove_cache_friendly_offset(p, meta.sizeclass);
       }
@@ -159,7 +160,7 @@ namespace snmalloc
 #endif
       meta.debug_slab_invariant(is_short(), this);
 
-      if (unlikely(meta.sub_use()))
+      if (unlikely(meta.return_object()))
         return false;
 
       // Update the head and the next pointer in the free list.
@@ -201,7 +202,7 @@ namespace snmalloc
         assert(meta.head == nullptr);
         //        assert(meta.fully_allocated(is_short()));
         meta.link = index;
-        meta.used = meta.allocated - 1;
+        meta.needed = meta.allocated - 1;
 
         // Push on the list of slabs for this sizeclass.
         sl->insert_back(meta.get_link(this));