Merge pull request #7 from Microsoft/mjp41/refactor

Fixes remote and pagemap

CMakeLists.txt

@@ -37,7 +37,7 @@ if(MSVC)
   set(CMAKE_EXE_LINKER_FLAGS_RELEASE "${CMAKE_EXE_LINKER_FLAGS_RELEASE} /DEBUG")
 else()
   find_package(Threads REQUIRED)
-  add_compile_options(-mcx16 -march=native -Wall -Wextra -Werror -g)
+  add_compile_options(-mcx16 -march=native -Wall -Wextra -Werror -Wsign-conversion -g)
 endif()
 
 set(CMAKE_CXX_STANDARD 17)

src/mem/largealloc.h

@@ -51,6 +51,13 @@ namespace snmalloc
       return std::make_pair(r, size);
     }
 
+    void new_block()
+    {
+      auto r_size = reserve_block();
+      bump = (size_t)r_size.first;
+      remaining = r_size.second;
+    }
+
   public:
     /**
      * Stack of large allocations that have been returned for reuse.
@@ -61,6 +68,7 @@ namespace snmalloc
      * Primitive allocator for structure that are required before
      * the allocator can be running.
      ***/
+    template<size_t alignment = 64>
     void* alloc_chunk(size_t size)
     {
       // Cache line align
@@ -70,12 +78,20 @@ namespace snmalloc
       {
         FlagLock f(lock);
 
+        auto aligned_bump = bits::align_up(bump, alignment);
+        if ((aligned_bump - bump) < size)
+        {
+          new_block();
+        }
+        else
+        {
+          remaining -= aligned_bump - bump;
+          bump = aligned_bump;
+        }
+
         if (remaining < size)
         {
-          auto r_size = reserve_block();
-
-          bump = (size_t)r_size.first;
-          remaining = r_size.second;
+          new_block();
         }
 
         p = (void*)bump;

src/mem/pagemap.h

@@ -86,7 +86,8 @@ namespace snmalloc
                 value, (PagemapEntry*)LOCKED_ENTRY, std::memory_order_relaxed))
           {
             auto& v = default_memory_provider;
-            value = (PagemapEntry*)v.alloc_chunk(PAGEMAP_NODE_SIZE);
+            value =
+              (PagemapEntry*)v.alloc_chunk<OS_PAGE_SIZE>(PAGEMAP_NODE_SIZE);
             e->store(value, std::memory_order_release);
           }
           else
@@ -160,6 +161,13 @@ namespace snmalloc
       return &(leaf_ix.first->values[leaf_ix.second]);
     }
 
+    std::atomic<T>* get_ptr(void* p)
+    {
+      bool success;
+      return get_addr<true>(p, success);
+    }
+
+  public:
     /**
      * Returns the index of a pagemap entry within a given page.  This is used
      * in code that propagates changes to the pagemap elsewhere.
@@ -182,13 +190,6 @@ namespace snmalloc
         reinterpret_cast<uintptr_t>(get_addr<true>(p, success)));
     }
 
-    std::atomic<T>* get_ptr(void* p)
-    {
-      bool success;
-      return get_addr<true>(p, success);
-    }
-
-  public:
     T get(void* p)
     {
       bool success;

src/mem/remoteallocator.h

@@ -2,6 +2,7 @@
 
 #include "../ds/mpscq.h"
 #include "../mem/allocconfig.h"
+#include "../mem/sizeclass.h"
 
 #include <atomic>
 
@@ -10,13 +11,11 @@ namespace snmalloc
   struct Remote
   {
     static const size_t PTR_BITS = sizeof(void*) * 8;
-    static const size_t SIZECLASS_BITS = sizeof(uint8_t) * 8;
+    static const size_t SIZECLASS_BITS =
+      bits::next_pow2_bits_const(NUM_SIZECLASSES);
     static const bool USE_TOP_BITS =
-      SIZECLASS_BITS + bits::ADDRESS_BITS <= PTR_BITS;
-    static const uintptr_t SIZECLASS_SHIFT = PTR_BITS - SIZECLASS_BITS;
-    static const uintptr_t SIZECLASS_MASK = ((1ULL << SIZECLASS_BITS) - 1)
-      << SIZECLASS_SHIFT;
-    static const uintptr_t TARGET_MASK = ~SIZECLASS_MASK;
+      SIZECLASS_BITS + bits::ADDRESS_BITS < PTR_BITS;
+    static const uintptr_t SIZECLASS_MASK = ((1ULL << SIZECLASS_BITS) - 1);
 
     using alloc_id_t = size_t;
     union
@@ -25,7 +24,11 @@ namespace snmalloc
       Remote* non_atomic_next;
     };
 
-    uintptr_t value;
+    // Uses an intptr_t so that when we use the TOP_BITS to encode the
+    // sizeclass, then we can use signed shift to correctly handle kernel versus
+    // user mode.
+    intptr_t value;
+
     // This will not exist for the minimum object size. This is only used if
     // USE_TOP_BITS is false, and the bottom bit of value is set.
     uint8_t possible_sizeclass;
@@ -34,21 +37,19 @@ namespace snmalloc
     {
       if constexpr (USE_TOP_BITS)
       {
-        assert(id == (id & TARGET_MASK));
-        value = (id & TARGET_MASK) |
-          ((static_cast<uint64_t>(sizeclass) << SIZECLASS_SHIFT) &
-           SIZECLASS_MASK);
+        value = (intptr_t)(
+          (id << SIZECLASS_BITS) | ((static_cast<uintptr_t>(sizeclass))));
       }
       else
       {
         assert((id & 1) == 0);
         if (sizeclass == 0)
         {
-          value = id | 1;
+          value = (intptr_t)(id | 1);
         }
         else
         {
-          value = id;
+          value = (intptr_t)id;
           possible_sizeclass = sizeclass;
         }
       }
@@ -58,11 +59,11 @@ namespace snmalloc
     {
       if constexpr (USE_TOP_BITS)
       {
-        return value & TARGET_MASK;
+        return (alloc_id_t)(value >> SIZECLASS_BITS);
       }
       else
       {
-        return value & ~1;
+        return (alloc_id_t)(value & ~1);
       }
     }
 
@@ -70,7 +71,7 @@ namespace snmalloc
     {
       if constexpr (USE_TOP_BITS)
       {
-        return (value & SIZECLASS_MASK) >> SIZECLASS_SHIFT;
+        return (static_cast<uint8_t>((uintptr_t)value & SIZECLASS_MASK));
       }
       else
       {