libexpr: cache more allocation sizes

we now use a single cache set for a number of sizes from one to eight
words. this also matches small attrsets, but perf impact seems small.

Change-Id: Icf16b329b98a20fcc9fe75e6395e148f0852c798

lix/libexpr/attr-set.cc

@@ -20,7 +20,7 @@ Bindings * EvalMemory::allocBindings(size_t capacity)
         throw Error("attribute set of size %d is too big", capacity);
     stats.nrAttrsets++;
     stats.nrAttrsInAttrsets += capacity;
-    return new (gcAllocBytes(sizeof(Bindings) + sizeof(Attr) * capacity)) Bindings();
+    return new (allocBytes(sizeof(Bindings) + sizeof(Attr) * capacity)) Bindings();
 }
 
 

lix/libexpr/eval-inline.hh

@@ -9,55 +9,65 @@
 namespace nix {
 
 [[gnu::always_inline]]
-Value * EvalMemory::allocValue()
+void * EvalMemory::allocBytes(size_t size)
 {
 #if HAVE_BOEHMGC
     /* We use the boehm batch allocator to speed up allocations of Values (of which there are many).
        GC_malloc_many returns a linked list of objects of the given size, where the first word
        of each object is also the pointer to the next object in the list. This also means that we
        have to explicitly clear the first word of every object we take. */
-    if (!*valueAllocCache) {
-        *valueAllocCache = GC_malloc_many(sizeof(Value));
-        if (!*valueAllocCache) throw std::bad_alloc();
-    }
+    // NOTE: we purposely do not allocate 0 byte blocks on caches; we never allocate
+    // zero bytes anyway, and it makes cache index calculation a little bit simpler.
+    const auto cacheIdx = (size - 1) / CACHE_INCREMENT;
+    if (cacheIdx < CACHES) {
+        const auto roundedSize = (cacheIdx + 1) * CACHE_INCREMENT;
+        auto & cache = gcCache[cacheIdx];
+        if (!cache) {
+            cache = GC_malloc_many(roundedSize);
+            if (!cache) {
+                throw std::bad_alloc();
+            }
+        }
 
-    /* GC_NEXT is a convenience macro for accessing the first word of an object.
-       Take the first list item, advance the list to the next item, and clear the next pointer. */
-    void * p = *valueAllocCache;
-    *valueAllocCache = GC_NEXT(p);
-    GC_NEXT(p) = nullptr;
-#else
-    void * p = gcAllocBytes(sizeof(Value));
+        /* GC_NEXT is a convenience macro for accessing the first word of an object.
+           Take the first list item, advance the list to the next item, and clear the next pointer.
+         */
+        void * p = cache;
+        cache = GC_NEXT(p);
+        GC_NEXT(p) = nullptr;
+        return p;
+    }
 #endif
 
-    stats.nrValues++;
-    return static_cast<Value *>(p);
+    return gcAllocBytes(size);
 }
 
+/// `gcAllocType`, but using allocation caches to amortize allocation overhead.
+template<typename T>
+[[gnu::always_inline]]
+T * EvalMemory::allocType(size_t n)
+{
+    // NOLINTNEXTLINE(bugprone-sizeof-expression)
+    return static_cast<T *>(allocBytes(checkedArrayAllocSize(sizeof(T), n)));
+}
+
+[[gnu::always_inline]]
+Value * EvalMemory::allocValue()
+{
+    static_assert(CACHES * CACHE_INCREMENT >= sizeof(Value));
+    stats.nrValues++;
+    return static_cast<Value *>(allocBytes(sizeof(Value)));
+}
 
 [[gnu::always_inline]]
 Env & EvalMemory::allocEnv(size_t size)
 {
+    static_assert(CACHES * CACHE_INCREMENT >= sizeof(Env) + sizeof(Value *));
+
     stats.nrEnvs++;
     stats.nrValuesInEnvs += size;
 
-    Env * env;
-
-#if HAVE_BOEHMGC
-    if (size == 1) {
-        /* see allocValue for explanations. */
-        if (!*env1AllocCache) {
-            *env1AllocCache = GC_malloc_many(sizeof(Env) + sizeof(Value *));
-            if (!*env1AllocCache) throw std::bad_alloc();
-        }
-
-        void * p = *env1AllocCache;
-        *env1AllocCache = GC_NEXT(p);
-        GC_NEXT(p) = nullptr;
-        env = static_cast<Env *>(p);
-    } else
-#endif
-        env = static_cast<Env *>(gcAllocBytes(sizeof(Env) + size * sizeof(Value *)));
+    Env * env = static_cast<Env *>(allocBytes(sizeof(Env) + size * sizeof(Value *)));
 
     /* We assume that env->values has been cleared by the allocator; maybeThunk() and lookupVar fromWith expect this. */
 

lix/libexpr/eval.cc

@@ -254,10 +254,18 @@ StaticSymbols::StaticSymbols(SymbolTable & symbols)
 }
 
 EvalMemory::EvalMemory()
-    : valueAllocCache(std::allocate_shared<void *>(TraceableAllocator<void *>(), nullptr))
-    , env1AllocCache(std::allocate_shared<void *>(TraceableAllocator<void *>(), nullptr))
 {
     assert(libexprInitialised);
+#if HAVE_BOEHMGC
+    GC_add_roots(gcCache, gcCache + CACHES);
+#endif
+}
+
+EvalMemory::~EvalMemory()
+{
+#if HAVE_BOEHMGC
+    GC_remove_roots(gcCache, gcCache + CACHES);
+#endif
 }
 
 EvalBuiltins::EvalBuiltins(
@@ -850,8 +858,9 @@ Value EvalMemory::newList(size_t size)
 {
     Value v;
     v.mkList(size);
-    if (size > 2)
-        v._bigList.elems = gcAllocType<Value *>(size);
+    if (size > 2) {
+        v._bigList.elems = allocType<Value *>(size);
+    }
     stats.nrListElems += size;
     return v;
 }

lix/libexpr/eval.hh

@@ -224,15 +224,13 @@ struct StaticSymbols
 
 class EvalMemory
 {
-    /**
-     * Allocation cache for GC'd Value objects.
-     */
-    std::shared_ptr<void *> valueAllocCache;
+    static constexpr size_t CACHES = 8;
+    static constexpr size_t CACHE_INCREMENT = sizeof(void *);
 
     /**
-     * Allocation cache for size-1 Env objects.
+     * Allocation caches for small values.
      */
-    std::shared_ptr<void *> env1AllocCache;
+    void * gcCache[CACHES] = {};
 
 public:
     struct Statistics
@@ -246,12 +244,17 @@ public:
     };
 
     EvalMemory();
+    ~EvalMemory();
 
     EvalMemory(const EvalMemory &) = delete;
     EvalMemory(EvalMemory &&) = delete;
     EvalMemory & operator=(const EvalMemory &) = delete;
     EvalMemory & operator=(EvalMemory &&) = delete;
 
+    inline void * allocBytes(size_t size);
+    template<typename T>
+    inline T * allocType(size_t n = 1);
+
     inline Value * allocValue();
     inline Env & allocEnv(size_t size);
 

lix/libexpr/gc-alloc.hh

@@ -95,6 +95,24 @@ inline void * gcAllocBytes(size_t n)
     return ptr;
 }
 
+[[gnu::always_inline]]
+inline size_t checkedArrayAllocSize(size_t size, size_t howMany)
+{
+    // NOTE: size_t * size_t, which can definitely overflow.
+    // Unsigned integer overflow is definitely a bug, but isn't undefined
+    // behavior, so we can just check if we overflowed after the fact.
+    // However, people can and do request zero sized allocations, so we need
+    // to check that neither of our multiplicands were zero before complaining
+    // about it.
+    auto checkedSz = checked::Checked<size_t>(howMany) * size;
+    if (checkedSz.overflowed()) {
+        // Congrats, you done did an overflow.
+        throw std::bad_alloc();
+    }
+
+    return checkedSz.valueWrapping();
+}
+
 /// Typed, safe wrapper around calloc() (transparently GC-enabled). Allocates
 /// enough for the requested count of the specified type. Also checks for
 /// nullptr (and throws @ref std::bad_alloc), and casts the void pointer to
@@ -103,21 +121,7 @@ template<typename T>
 [[gnu::always_inline]]
 inline T * gcAllocType(size_t howMany = 1)
 {
-    // NOTE: size_t * size_t, which can definitely overflow.
-    // Unsigned integer overflow is definitely a bug, but isn't undefined
-    // behavior, so we can just check if we overflowed after the fact.
-    // However, people can and do request zero sized allocations, so we need
-    // to check that neither of our multiplicands were zero before complaining
-    // about it.
-    // NOLINTNEXTLINE(bugprone-sizeof-expression): yeah we only seem to alloc pointers with this. the calculation *is* correct though!
-    auto checkedSz = checked::Checked<size_t>(howMany) * sizeof(T);
-    size_t sz = checkedSz.valueWrapping();
-    if (checkedSz.overflowed()) {
-        // Congrats, you done did an overflow.
-        throw std::bad_alloc();
-    }
-
-    return static_cast<T *>(gcAllocBytes(sz));
+    return static_cast<T *>(gcAllocBytes(checkedArrayAllocSize(sizeof(T), howMany)));
 }
 
 /// GC-transparently allocates a buffer for a C-string of @ref size *bytes*,