Remove cache-friendly offset.

gaxlin · May 18, 2021 · e77a5d9 · e77a5d9
1 parent f0ebbeb
commit e77a5d9
Show file tree

Hide file tree

Showing 8 changed files with 29 additions and 132 deletions.
diff --git a/CMakeLists.txt b/CMakeLists.txt
@@ -17,7 +17,6 @@ option(SNMALLOC_RUST_SUPPORT "Build static library for rust" OFF)
 option(SNMALLOC_STATIC_LIBRARY   "Build static libraries" ON)
 option(SNMALLOC_QEMU_WORKAROUND "Disable using madvise(DONT_NEED) to zero memory on Linux" Off)
 option(SNMALLOC_OPTIMISE_FOR_CURRENT_MACHINE "Compile for current machine architecture" Off)
-set(CACHE_FRIENDLY_OFFSET OFF CACHE STRING "Base offset to place linked-list nodes.")
 set(SNMALLOC_STATIC_LIBRARY_PREFIX "sn_" CACHE STRING "Static library function prefix")
 option(SNMALLOC_USE_CXX20 "Build as C++20, not C++17; experimental as yet" OFF)
 
@@ -157,10 +156,6 @@ if(SNMALLOC_CI_BUILD)
   target_compile_definitions(snmalloc_lib INTERFACE -DSNMALLOC_CI_BUILD)
 endif()
 
-if(CACHE_FRIENDLY_OFFSET)
-  target_compile_definitions(snmalloc_lib INTERFACE -DCACHE_FRIENDLY_OFFSET=${CACHE_FRIENDLY_OFFSET})
-endif()
-
 if(USE_POSIX_COMMIT_CHECKS)
   target_compile_definitions(snmalloc_lib INTERFACE -DUSE_POSIX_COMMIT_CHECKS)
 endif()

diff --git a/azure-pipelines.yml b/azure-pipelines.yml
@@ -92,14 +92,6 @@ jobs:
         CMakeArgs: ''
         Image: snmallocciteam/build_linux_x64:latest
 
-      64-bit Cache Friendly:
-        CC: clang-7
-        CXX: clang++-7
-        BuildType: Debug
-        SelfHost: false
-        CMakeArgs: '-DCACHE_FRIENDLY_OFFSET=64'
-        Image: snmallocciteam/build_linux_x64:latest
-
       32-bit Clang-9 Debug:
         CC: clang-9
         CXX: clang++-9

diff --git a/src/mem/alloc.h b/src/mem/alloc.h
@@ -517,31 +517,6 @@ namespace snmalloc
     std::conditional_t<IsQueueInline, RemoteAllocator, RemoteAllocator*>
       remote_alloc;
 
-#ifdef CACHE_FRIENDLY_OFFSET
-    size_t remote_offset = 0;
-
-    template<capptr_bounds B>
-    CapPtr<FreeObject, B>
-    apply_cache_friendly_offset(CapPtr<void, B> p, sizeclass_t sizeclass)
-    {
-      size_t mask = sizeclass_to_cache_friendly_mask(sizeclass);
-
-      size_t offset = remote_offset & mask;
-      remote_offset += CACHE_FRIENDLY_OFFSET;
-
-      return CapPtr<FreeObject, B>(reinterpret_cast<FreeObject*>(
-        reinterpret_cast<uintptr_t>(p.unsafe_capptr) + offset));
-    }
-#else
-    template<capptr_bounds B>
-    static CapPtr<FreeObject, B>
-    apply_cache_friendly_offset(CapPtr<void, B> p, sizeclass_t sizeclass)
-    {
-      UNUSED(sizeclass);
-      return p.template as_static<FreeObject>();
-    }
-#endif
-
     auto* public_state()
     {
       if constexpr (IsQueueInline)
@@ -740,14 +715,15 @@ namespace snmalloc
         // Destined for my slabs
         auto p_auth = large_allocator.template capptr_amplify<Remote>(p);
         auto super = Superslab::get(p_auth);
-        dealloc_not_large_local(super, p, p->sizeclass());
+        auto sizeclass = p->sizeclass();
+        dealloc_not_large_local(super, Remote::clear(p), sizeclass);
       }
       else
       {
         // Merely routing; despite the cast here, p is going to be cast right
         // back to a Remote.
         remote_cache.dealloc<Allocator>(
-          target_id, p.template as_reinterpret<FreeObject>(), p->sizeclass());
+          target_id, p.template as_reinterpret<void>(), p->sizeclass());
       }
     }
 
@@ -758,19 +734,19 @@ namespace snmalloc
       {
         auto p_auth = large_allocator.capptr_amplify(p);
         auto super = Superslab::get(p_auth);
-        auto offseted = apply_cache_friendly_offset(p, sizeclass)
-                          .template as_reinterpret<Remote>();
-        dealloc_not_large_local(super, offseted, sizeclass);
+        dealloc_not_large_local(super, p, sizeclass);
       }
       else
       {
         remote_dealloc_and_post(target, p, sizeclass);
       }
     }
 
+    // TODO: Adjust when medium slab same as super slab.
+    //    Second parameter should be a FreeObject.
     SNMALLOC_FAST_PATH void dealloc_not_large_local(
       CapPtr<Superslab, CBChunkD> super,
-      CapPtr<Remote, CBAlloc> p_offseted,
+      CapPtr<void, CBAlloc> p,
       sizeclass_t sizeclass)
     {
       // Guard against remote queues that have colliding IDs
@@ -782,13 +758,11 @@ namespace snmalloc
         check_client(
           super->get_kind() == Super,
           "Heap Corruption: Sizeclass of remote dealloc corrupt.");
-        auto slab =
-          Metaslab::get_slab(Aal::capptr_rebound(super.as_void(), p_offseted));
+        auto slab = Metaslab::get_slab(Aal::capptr_rebound(super.as_void(), p));
         check_client(
           super->get_meta(slab)->sizeclass() == sizeclass,
           "Heap Corruption: Sizeclass of remote dealloc corrupt.");
-        small_dealloc_offseted(
-          super, slab, FreeObject::make(p_offseted), sizeclass);
+        small_dealloc_offseted(super, slab, p, sizeclass);
       }
       else
       {
@@ -800,8 +774,7 @@ namespace snmalloc
         check_client(
           medium->get_sizeclass() == sizeclass,
           "Heap Corruption: Sizeclass of remote dealloc corrupt.");
-        medium_dealloc_local(
-          medium, Remote::clear(p_offseted, sizeclass), sizeclass);
+        medium_dealloc_local(medium, p, sizeclass);
       }
     }
 
@@ -951,14 +924,15 @@ namespace snmalloc
       {
         stats().alloc_request(size);
         stats().sizeclass_alloc(sizeclass);
-        auto p = remove_cache_friendly_offset(fl.take(entropy), sizeclass);
+        auto p = fl.take(entropy);
         if constexpr (zero_mem == YesZero)
         {
           pal_zero<typename MemoryProvider::Pal>(
             p, sizeclass_to_size(sizeclass));
         }
 
-        return capptr_export(p);
+        // TODO: Should this be zeroing the next pointer?
+        return capptr_export(p.as_void());
       }
 
       if (likely(!has_messages()))
@@ -1075,14 +1049,15 @@ namespace snmalloc
       SNMALLOC_ASSERT(ffl.empty());
       Slab::alloc_new_list(bp, ffl, rsize, entropy);
 
-      auto p = remove_cache_friendly_offset(ffl.take(entropy), sizeclass);
+      auto p = ffl.take(entropy);
 
       if constexpr (zero_mem == YesZero)
       {
         pal_zero<typename MemoryProvider::Pal>(p, sizeclass_to_size(sizeclass));
       }
 
-      return capptr_export(p);
+      // TODO: Should this be zeroing the next pointer?
+      return capptr_export(p.as_void());
     }
 
     /**
@@ -1163,8 +1138,7 @@ namespace snmalloc
 
       if (likely(target == public_state()))
       {
-        auto offseted = apply_cache_friendly_offset(p, sizeclass);
-        small_dealloc_offseted(super, slab, offseted, sizeclass);
+        small_dealloc_offseted(super, slab, p, sizeclass);
       }
       else
         remote_dealloc(target, p, sizeclass);
@@ -1173,12 +1147,12 @@ namespace snmalloc
     SNMALLOC_FAST_PATH void small_dealloc_offseted(
       CapPtr<Superslab, CBChunkD> super,
       CapPtr<Slab, CBChunkD> slab,
-      CapPtr<FreeObject, CBAlloc> p,
+      CapPtr<void, CBAlloc> p,
       sizeclass_t sizeclass)
     {
       stats().sizeclass_dealloc(sizeclass);
 
-      small_dealloc_offseted_inner(super, slab, p, sizeclass);
+      small_dealloc_offseted_inner(super, slab, FreeObject::make(p), sizeclass);
     }
 
     SNMALLOC_FAST_PATH void small_dealloc_offseted_inner(
@@ -1537,9 +1511,7 @@ namespace snmalloc
       if (remote_cache.capacity > 0)
       {
         stats().remote_free(sizeclass);
-        auto offseted = apply_cache_friendly_offset(p, sizeclass);
-        remote_cache.dealloc<Allocator>(
-          target->trunc_id(), offseted, sizeclass);
+        remote_cache.dealloc<Allocator>(target->trunc_id(), p, sizeclass);
         return;
       }
 
@@ -1577,8 +1549,7 @@ namespace snmalloc
       handle_message_queue();
 
       stats().remote_free(sizeclass);
-      auto offseted = apply_cache_friendly_offset(p_auth, sizeclass);
-      remote_cache.dealloc<Allocator>(target->trunc_id(), offseted, sizeclass);
+      remote_cache.dealloc<Allocator>(target->trunc_id(), p_auth, sizeclass);
 
       stats().remote_post();
       remote_cache.post<Allocator>(this, get_trunc_id());

diff --git a/src/mem/freelist.h b/src/mem/freelist.h
@@ -128,9 +128,6 @@ namespace snmalloc
    * Free objects within each slab point directly to the next.
    * The next_object pointer can be encoded to detect
    * corruption caused by writes in a UAF or a double free.
-   *
-   * If cache-friendly offsets are used, then the FreeObject is
-   * potentially offset from the start of the object.
    */
   class FreeObject
   {

diff --git a/src/mem/metaslab.h b/src/mem/metaslab.h
@@ -174,17 +174,16 @@ namespace snmalloc
       SNMALLOC_ASSERT(!self->is_full());
 
       self->free_queue.close(fast_free_list, entropy);
-      auto n = fast_free_list.take(entropy);
-      auto n_slab = Aal::capptr_rebound(self.as_void(), n);
-      auto meta = Metaslab::get_slab(n_slab);
+      auto p = fast_free_list.take(entropy);
+      auto slab = Aal::capptr_rebound(self.as_void(), p);
+      auto meta = Metaslab::get_slab(slab);
 
       entropy.refresh_bits();
 
       // Treat stealing the free list as allocating it all.
       self->remove();
       self->set_full(meta);
 
-      auto p = remove_cache_friendly_offset(n, self->sizeclass());
       SNMALLOC_ASSERT(self->is_start_of_object(address_cast(p)));
 
       self->debug_slab_invariant(meta, entropy);
@@ -201,7 +200,8 @@ namespace snmalloc
         UNUSED(rsize);
       }
 
-      return capptr_export(p);
+      // TODO: Should this be zeroing the FreeObject state?
+      return capptr_export(p.as_void());
     }
 
     template<capptr_bounds B>

diff --git a/src/mem/remoteallocator.h b/src/mem/remoteallocator.h
@@ -100,11 +100,10 @@ namespace snmalloc
 
     /** Zero out a Remote tracking structure, return pointer to object base */
     template<capptr_bounds B>
-    SNMALLOC_FAST_PATH static CapPtr<void, B>
-    clear(CapPtr<Remote, B> self, sizeclass_t sizeclass)
+    SNMALLOC_FAST_PATH static CapPtr<void, B> clear(CapPtr<Remote, B> self)
     {
       pal_zero<Pal>(self, sizeof(Remote));
-      return remove_cache_friendly_offset(self, sizeclass);
+      return self.as_void();
     }
   };
 
@@ -187,7 +186,7 @@ namespace snmalloc
     template<typename Alloc>
     SNMALLOC_FAST_PATH void dealloc(
       Remote::alloc_id_t target_id,
-      CapPtr<FreeObject, CBAlloc> p,
+      CapPtr<void, CBAlloc> p,
       sizeclass_t sizeclass)
     {
       this->capacity -= sizeclass_to_size(sizeclass);

diff --git a/src/mem/sizeclass.h b/src/mem/sizeclass.h
@@ -17,10 +17,6 @@ namespace snmalloc
   constexpr static uint16_t get_slab_capacity(sizeclass_t sc, bool is_short);
 
   constexpr static size_t sizeclass_to_size(sizeclass_t sizeclass);
-  constexpr static size_t
-  sizeclass_to_cache_friendly_mask(sizeclass_t sizeclass);
-  constexpr static size_t
-  sizeclass_to_inverse_cache_friendly_mask(sizeclass_t sc);
   constexpr static uint16_t medium_slab_free(sizeclass_t sizeclass);
   static sizeclass_t size_to_sizeclass(size_t size);
 
@@ -57,38 +53,6 @@ namespace snmalloc
   static constexpr size_t NUM_LARGE_CLASSES =
     bits::ADDRESS_BITS - SUPERSLAB_BITS;
 
-#ifdef CACHE_FRIENDLY_OFFSET
-  template<typename T, capptr_bounds B>
-  SNMALLOC_FAST_PATH static CapPtr<void, B>
-  remove_cache_friendly_offset(CapPtr<T, B> p, sizeclass_t sizeclass)
-  {
-    size_t mask = sizeclass_to_inverse_cache_friendly_mask(sizeclass);
-    return CapPtr<void, B>((void*)((uintptr_t)p.unsafe_capptr & mask));
-  }
-
-  SNMALLOC_FAST_PATH static address_t
-  remove_cache_friendly_offset(address_t relative, sizeclass_t sizeclass)
-  {
-    size_t mask = sizeclass_to_inverse_cache_friendly_mask(sizeclass);
-    return relative & mask;
-  }
-#else
-  template<typename T, capptr_bounds B>
-  SNMALLOC_FAST_PATH static CapPtr<void, B>
-  remove_cache_friendly_offset(CapPtr<T, B> p, sizeclass_t sizeclass)
-  {
-    UNUSED(sizeclass);
-    return p.as_void();
-  }
-
-  SNMALLOC_FAST_PATH static address_t
-  remove_cache_friendly_offset(address_t relative, sizeclass_t sizeclass)
-  {
-    UNUSED(sizeclass);
-    return relative;
-  }
-#endif
-
   SNMALLOC_FAST_PATH static size_t aligned_size(size_t alignment, size_t size)
   {
     // Client responsible for checking alignment is not zero

diff --git a/src/mem/sizeclasstable.h b/src/mem/sizeclasstable.h
@@ -25,8 +25,6 @@ namespace snmalloc
   {
     sizeclass_t sizeclass_lookup[sizeclass_lookup_size] = {{}};
     ModArray<NUM_SIZECLASSES, size_t> size;
-    ModArray<NUM_SIZECLASSES, size_t> cache_friendly_mask;
-    ModArray<NUM_SIZECLASSES, size_t> inverse_cache_friendly_mask;
     ModArray<NUM_SMALL_CLASSES, uint16_t> initial_offset_ptr;
     ModArray<NUM_SMALL_CLASSES, uint16_t> short_initial_offset_ptr;
     ModArray<NUM_SMALL_CLASSES, uint16_t> capacity;
@@ -39,8 +37,6 @@ namespace snmalloc
 
     constexpr SizeClassTable()
     : size(),
-      cache_friendly_mask(),
-      inverse_cache_friendly_mask(),
       initial_offset_ptr(),
       short_initial_offset_ptr(),
       capacity(),
@@ -77,11 +73,6 @@ namespace snmalloc
             sizeclass_lookup[sizeclass_lookup_index(curr)] = sizeclass;
           }
         }
-
-        size_t alignment = bits::min(
-          bits::one_at_bit(bits::ctz_const(size[sizeclass])), OS_PAGE_SIZE);
-        cache_friendly_mask[sizeclass] = (alignment - 1);
-        inverse_cache_friendly_mask[sizeclass] = ~(alignment - 1);
       }
 
       size_t header_size = sizeof(Superslab);
@@ -138,18 +129,6 @@ namespace snmalloc
     return sizeclass_metadata.size[sizeclass];
   }
 
-  constexpr static inline size_t
-  sizeclass_to_cache_friendly_mask(sizeclass_t sizeclass)
-  {
-    return sizeclass_metadata.cache_friendly_mask[sizeclass];
-  }
-
-  constexpr static SNMALLOC_FAST_PATH size_t
-  sizeclass_to_inverse_cache_friendly_mask(sizeclass_t sizeclass)
-  {
-    return sizeclass_metadata.inverse_cache_friendly_mask[sizeclass];
-  }
-
   static inline sizeclass_t size_to_sizeclass(size_t size)
   {
     if ((size - 1) <= (SLAB_SIZE - 1))