From b36a19e50c44ee8489b384ef3b43628ae7f52a1b Mon Sep 17 00:00:00 2001 From: eldritch horrors Date: Sun, 28 Sep 2025 00:02:20 +0200 Subject: [PATCH] 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 | 2 +- lix/libexpr/eval-inline.hh | 71 +++++++++++++++++++++----------------- lix/libexpr/eval.cc | 17 ++++++--- lix/libexpr/eval.hh | 15 ++++---- lix/libexpr/gc-alloc.hh | 34 ++++++++++-------- 5 files changed, 82 insertions(+), 57 deletions(-) diff --git a/lix/libexpr/attr-set.cc b/lix/libexpr/attr-set.cc index 880f9d316..ec0c3ad5a 100644 --- a/lix/libexpr/attr-set.cc +++ b/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(); } diff --git a/lix/libexpr/eval-inline.hh b/lix/libexpr/eval-inline.hh index a645c0aa9..ade03b448 100644 --- a/lix/libexpr/eval-inline.hh +++ b/lix/libexpr/eval-inline.hh @@ -9,55 +9,64 @@ 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(p); + return gcAllocBytes(size); } +/// `gcAllocType`, but using allocation caches to amortize allocation overhead. +template +[[gnu::always_inline]] +T * EvalMemory::allocType(size_t n) +{ + return static_cast(allocBytes(checkedArrayAllocSize(sizeof(T), n))); +} + +[[gnu::always_inline]] +Value * EvalMemory::allocValue() +{ + static_assert(CACHES * CACHE_INCREMENT >= sizeof(Value)); + stats.nrValues++; + return static_cast(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(p); - } else -#endif - env = static_cast(gcAllocBytes(sizeof(Env) + size * sizeof(Value *))); + Env * env = static_cast(allocBytes(sizeof(Env) + size * sizeof(Value *))); /* We assume that env->values has been cleared by the allocator; maybeThunk() and lookupVar fromWith expect this. */ diff --git a/lix/libexpr/eval.cc b/lix/libexpr/eval.cc index 5a62c1402..1010edd05 100644 --- a/lix/libexpr/eval.cc +++ b/lix/libexpr/eval.cc @@ -254,10 +254,18 @@ StaticSymbols::StaticSymbols(SymbolTable & symbols) } EvalMemory::EvalMemory() - : valueAllocCache(std::allocate_shared(TraceableAllocator(), nullptr)) - , env1AllocCache(std::allocate_shared(TraceableAllocator(), 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(size); + if (size > 2) { + v._bigList.elems = allocType(size); + } stats.nrListElems += size; return v; } diff --git a/lix/libexpr/eval.hh b/lix/libexpr/eval.hh index 02cfa7367..15c6e9a20 100644 --- a/lix/libexpr/eval.hh +++ b/lix/libexpr/eval.hh @@ -224,15 +224,13 @@ struct StaticSymbols class EvalMemory { - /** - * Allocation cache for GC'd Value objects. - */ - std::shared_ptr 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 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 + inline T * allocType(size_t n = 1); + inline Value * allocValue(); inline Env & allocEnv(size_t size); diff --git a/lix/libexpr/gc-alloc.hh b/lix/libexpr/gc-alloc.hh index 9816e2869..230044456 100644 --- a/lix/libexpr/gc-alloc.hh +++ b/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(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 [[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(howMany) * sizeof(T); - size_t sz = checkedSz.valueWrapping(); - if (checkedSz.overflowed()) { - // Congrats, you done did an overflow. - throw std::bad_alloc(); - } - - return static_cast(gcAllocBytes(sz)); + return static_cast(gcAllocBytes(checkedArrayAllocSize(sizeof(T), howMany))); } /// GC-transparently allocates a buffer for a C-string of @ref size *bytes*,