From b588eba319d22b804365771b2c403b162dcad977 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 11 Jun 2026 11:52:10 +0000 Subject: [PATCH 1/5] Erase zero-count entries in AliveVariableHash to stop unbounded memory growth MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit AliveVariableHash::decrement() left entries in the counts map at count 0, so the map grew with every value ref ever seen during tracing rather than with the number of currently-alive variables. Because the trace contexts are thread_local and reset() skipped clearing when the hash was 0 (exactly the state a balanced trace ends in), these dead entries survived reset() and accumulated across trace iterations and across traced functions for the lifetime of the thread. Erasing entries when their count reaches zero is hash-neutral: a zero count contributes (id * HASH_MULTIPLIER) * 0 = 0 to the XOR hash, which is identical to an absent entry, so snapshot hashes — and therefore trace deduplication and generated IR — are unchanged. Also make reset() check map emptiness instead of the hash, which removes the latent edge case where live entries XOR-colliding to hash 0 would leak reference counts into the next trace iteration. Add a size() accessor and unit tests covering erasure, hash-neutrality, and reset behavior in a new tracing-tests suite. https://claude.ai/code/session_01XTX9MjHW7Bgq2uSrFMXhiX --- .../tracing/ExceptionBasedTraceContext.hpp | 32 +++++++- nautilus/test/CMakeLists.txt | 1 + .../tracing-tests/AliveVariableHashTest.cpp | 78 +++++++++++++++++++ nautilus/test/tracing-tests/CMakeLists.txt | 15 ++++ 4 files changed, 122 insertions(+), 4 deletions(-) create mode 100644 nautilus/test/tracing-tests/AliveVariableHashTest.cpp create mode 100644 nautilus/test/tracing-tests/CMakeLists.txt diff --git a/nautilus/src/nautilus/tracing/ExceptionBasedTraceContext.hpp b/nautilus/src/nautilus/tracing/ExceptionBasedTraceContext.hpp index 4618d1700..2f65f6e21 100644 --- a/nautilus/src/nautilus/tracing/ExceptionBasedTraceContext.hpp +++ b/nautilus/src/nautilus/tracing/ExceptionBasedTraceContext.hpp @@ -96,13 +96,23 @@ class AliveVariableHash { * The hash is updated by XOR-ing out the old contribution ((id * HASH_MULTIPLIER) * old_count) * and XOR-ing in the new contribution ((id * HASH_MULTIPLIER) * new_count). * + * Entries that reach a count of zero are erased. This is hash-neutral (a zero count + * contributes 0 to the XOR hash, identical to an absent entry) and keeps the map size + * proportional to the number of currently-alive variables. Without the erase, the map + * grows with every value ref ever seen and — because the trace context is thread_local — + * persists across trace iterations and across traced functions. + * * @param id Variable identifier (32-bit value) */ inline void decrement(uint32_t id) noexcept { - uint32_t& c = counts[id]; + const auto it = counts.try_emplace(id, 0).first; + uint32_t& c = it->second; alive_hash ^= (id * HASH_MULTIPLIER) * c; --c; alive_hash ^= (id * HASH_MULTIPLIER) * c; + if (c == 0) { + counts.erase(it); + } } /** @@ -118,17 +128,31 @@ class AliveVariableHash { return alive_hash; } + /** + * @brief Returns the number of tracked entries. + * + * Since zero-count entries are erased by decrement(), this is the number of + * currently-alive variables (variables with a non-zero reference count). + * + * @return Number of entries in the underlying map + */ + inline size_t size() const noexcept { + return counts.size(); + } + /** * @brief Resets all reference counts and hash to initial state. * * This efficiently clears all counts without creating a temporary object. - * Optimized: if hash is already 0, we assume counts are already empty and skip the clear. + * Since decrement() erases entries when they reach zero, a balanced trace iteration + * leaves the map empty and this is a no-op. The emptiness check (rather than a hash + * check) also guards against the rare XOR collision where live entries hash to 0. */ inline void reset() noexcept { - if (alive_hash != 0) { + if (!counts.empty()) { counts.clear(); - alive_hash = 0; } + alive_hash = 0; } }; diff --git a/nautilus/test/CMakeLists.txt b/nautilus/test/CMakeLists.txt index 20a8d7548..066c83efa 100644 --- a/nautilus/test/CMakeLists.txt +++ b/nautilus/test/CMakeLists.txt @@ -14,6 +14,7 @@ add_subdirectory(benchmark) add_subdirectory(execution-tests) if (ENABLE_TRACING) add_subdirectory(ir-pass-tests) + add_subdirectory(tracing-tests) endif () add_subdirectory(val-tests) diff --git a/nautilus/test/tracing-tests/AliveVariableHashTest.cpp b/nautilus/test/tracing-tests/AliveVariableHashTest.cpp new file mode 100644 index 000000000..233ebe403 --- /dev/null +++ b/nautilus/test/tracing-tests/AliveVariableHashTest.cpp @@ -0,0 +1,78 @@ +#include "nautilus/tracing/ExceptionBasedTraceContext.hpp" +#include + +namespace nautilus::tracing { + +TEST_CASE("AliveVariableHash erases entries that reach a zero count", "[AliveVariableHash]") { + AliveVariableHash avh; + REQUIRE(avh.size() == 0); + REQUIRE(avh.hash() == 0); + + avh.increment(1); + avh.increment(2); + avh.increment(3); + REQUIRE(avh.size() == 3); + + avh.decrement(2); + REQUIRE(avh.size() == 2); + + avh.decrement(1); + avh.decrement(3); + REQUIRE(avh.size() == 0); + REQUIRE(avh.hash() == 0); +} + +TEST_CASE("AliveVariableHash erasure is hash-neutral", "[AliveVariableHash]") { + // A variable that was seen and fully released must leave the hash identical + // to one that was never seen. Snapshots key on this hash, so any difference + // would change trace deduplication and the generated IR. + AliveVariableHash seenAndReleased; + seenAndReleased.increment(7); + seenAndReleased.increment(42); + seenAndReleased.decrement(42); + + AliveVariableHash neverSeen; + neverSeen.increment(7); + + REQUIRE(seenAndReleased.hash() == neverSeen.hash()); + REQUIRE(seenAndReleased.size() == neverSeen.size()); +} + +TEST_CASE("AliveVariableHash re-incrementing a released id reproduces the original hash", "[AliveVariableHash]") { + AliveVariableHash avh; + avh.increment(5); + const uint64_t hashWithFive = avh.hash(); + + avh.decrement(5); + avh.increment(5); + REQUIRE(avh.hash() == hashWithFive); + REQUIRE(avh.size() == 1); +} + +TEST_CASE("AliveVariableHash hash reflects reference counts", "[AliveVariableHash]") { + AliveVariableHash once; + once.increment(9); + + AliveVariableHash twice; + twice.increment(9); + twice.increment(9); + + REQUIRE(once.hash() != twice.hash()); + REQUIRE(twice.size() == 1); + + twice.decrement(9); + REQUIRE(twice.hash() == once.hash()); +} + +TEST_CASE("AliveVariableHash reset clears live entries", "[AliveVariableHash]") { + AliveVariableHash avh; + avh.increment(1); + avh.increment(1); + avh.increment(2); + + avh.reset(); + REQUIRE(avh.size() == 0); + REQUIRE(avh.hash() == 0); +} + +} // namespace nautilus::tracing diff --git a/nautilus/test/tracing-tests/CMakeLists.txt b/nautilus/test/tracing-tests/CMakeLists.txt new file mode 100644 index 000000000..f56acaad7 --- /dev/null +++ b/nautilus/test/tracing-tests/CMakeLists.txt @@ -0,0 +1,15 @@ +include(CTest) +include(Catch) + +add_executable(nautilus-tracing-unit-tests + AliveVariableHashTest.cpp +) + +target_link_libraries(nautilus-tracing-unit-tests PRIVATE nautilus Catch2::Catch2WithMain nautilus-sanitizer-suppressions) +target_include_directories(nautilus-tracing-unit-tests PRIVATE + $ + $ +) + +list(APPEND CMAKE_MODULE_PATH ${catch2_SOURCE_DIR}/extras) +catch_discover_tests(nautilus-tracing-unit-tests EXTRA_ARGS --allow-running-no-tests) From 3a6172a9d5c094664beac4ed7e28492e3513bfc2 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 11 Jun 2026 12:06:58 +0000 Subject: [PATCH 2/5] Add churn test mimicking the monotonic value-ref tracing pattern Value refs are assigned monotonically and never reused, so during tracing the map sees a stream of ids that are incremented once and decremented to zero shortly after. Verify that this churn neither perturbs the hash nor grows the map while a long-lived entry survives it. https://claude.ai/code/session_01XTX9MjHW7Bgq2uSrFMXhiX --- .../tracing-tests/AliveVariableHashTest.cpp | 22 ++++++++++++++++++- 1 file changed, 21 insertions(+), 1 deletion(-) diff --git a/nautilus/test/tracing-tests/AliveVariableHashTest.cpp b/nautilus/test/tracing-tests/AliveVariableHashTest.cpp index 233ebe403..9a65358a4 100644 --- a/nautilus/test/tracing-tests/AliveVariableHashTest.cpp +++ b/nautilus/test/tracing-tests/AliveVariableHashTest.cpp @@ -3,7 +3,7 @@ namespace nautilus::tracing { -TEST_CASE("AliveVariableHash erases entries that reach a zero count", "[AliveVariableHash]") { +TEST_CASE("AliveVariableHash tracks the number of alive variables", "[AliveVariableHash]") { AliveVariableHash avh; REQUIRE(avh.size() == 0); REQUIRE(avh.hash() == 0); @@ -64,6 +64,26 @@ TEST_CASE("AliveVariableHash hash reflects reference counts", "[AliveVariableHas REQUIRE(twice.hash() == once.hash()); } +TEST_CASE("AliveVariableHash id churn neither perturbs the hash nor grows the map", "[AliveVariableHash]") { + // Monotonically increasing ids that immediately die mimic the tracing access + // pattern: value refs are assigned monotonically and never reused. The churn must + // not affect the hash or the size while a long-lived entry survives it. + AliveVariableHash avh; + avh.increment(1000000); + const uint64_t hashWithSurvivor = avh.hash(); + + for (uint32_t id = 0; id < 10000; ++id) { + avh.increment(id); + avh.decrement(id); + REQUIRE(avh.hash() == hashWithSurvivor); + } + REQUIRE(avh.size() == 1); + + avh.decrement(1000000); + REQUIRE(avh.size() == 0); + REQUIRE(avh.hash() == 0); +} + TEST_CASE("AliveVariableHash reset clears live entries", "[AliveVariableHash]") { AliveVariableHash avh; avh.increment(1); From 425269ee10a4d3fe6ccbf78fb6adfc3138e8f7fc Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 11 Jun 2026 12:40:11 +0000 Subject: [PATCH 3/5] Back AliveVariableHash with a free-list node pool to recover tracing speed Erasing zero-count entries bounds the map to O(alive variables) but routes one malloc/free pair per traced value through the general-purpose allocator, which slowed tracing of large functions by up to ~1.3x. Replace the default allocator with an internal pool that hands map nodes out of bump-allocated 16KB chunks and recycles freed nodes via per-size free lists (the allocator is rebound to different types - nodes and bucket arrays - so size classes must not be mixed). This recovers most of the regression: the tracing benchmark suite runs within ~4% of the pre-fix baseline (down from ~17% with plain erasure) while the pool retains a single 16KB chunk per trace context across the entire suite. A bump-only arena would not fit this workload: without per-node reclamation, the insert/erase churn would regrow memory with every value ref ever seen - the leak this fix removes. https://claude.ai/code/session_01XTX9MjHW7Bgq2uSrFMXhiX --- .../tracing/ExceptionBasedTraceContext.hpp | 114 +++++++++++++++++- 1 file changed, 113 insertions(+), 1 deletion(-) diff --git a/nautilus/src/nautilus/tracing/ExceptionBasedTraceContext.hpp b/nautilus/src/nautilus/tracing/ExceptionBasedTraceContext.hpp index 2f65f6e21..304e04782 100644 --- a/nautilus/src/nautilus/tracing/ExceptionBasedTraceContext.hpp +++ b/nautilus/src/nautilus/tracing/ExceptionBasedTraceContext.hpp @@ -10,13 +10,17 @@ #include "tag/Tag.hpp" #include "tag/TagRecorder.hpp" #include +#include #include +#include #include #include #include #include +#include #include #include +#include namespace nautilus { class NautilusFunctionDefinition; @@ -55,6 +59,9 @@ inline size_t getStaticVarValue(const StaticVarHolder& holder) { * - Each variable ID is mixed with a constant multiplier for better hash distribution * - The hash incorporates both variable identity (ID) and reference count * - Uses unordered_map for sparse storage (only allocates for variables that exist) + * - Map nodes come from an internal free-list pool (see NodePool); entries are erased + * when their count reaches zero, so memory stays bounded by the peak number of + * alive variables instead of growing with every value ref ever seen * * Performance characteristics: * - increment(): O(1) average - hash map lookup + two XOR operations, two multiplications @@ -66,7 +73,112 @@ inline size_t getStaticVarValue(const StaticVarHolder& holder) { class AliveVariableHash { static constexpr uint64_t HASH_MULTIPLIER = 0x9e3779b97f4a7c15; // Golden ratio constant for good mixing - std::unordered_map counts; + // Free-list node pool backing the counts map. The tracing workload inserts and + // erases one map node per value lifetime; routing those through the general-purpose + // allocator costs a malloc/free pair per traced value, which measurably slows + // tracing of large functions. The pool hands blocks out of bump-allocated chunks + // and recycles freed blocks via per-size free lists (the allocator is rebound to + // different types - nodes and bucket arrays - so size classes must not be mixed). + // Memory stays bounded by the peak number of alive variables. + class NodePool { + static constexpr std::size_t CHUNK_SIZE = 16 * 1024; + static constexpr std::size_t BLOCK_ALIGN = alignof(std::max_align_t); + static constexpr std::size_t MAX_POOLED_SIZE = 4 * BLOCK_ALIGN; + static constexpr std::size_t NUM_SIZE_CLASSES = MAX_POOLED_SIZE / BLOCK_ALIGN; + + std::vector chunks; + std::array freeHeads {}; + std::byte* cursor = nullptr; + std::byte* chunkEnd = nullptr; + + static constexpr std::size_t roundedSize(std::size_t size) noexcept { + return (size + BLOCK_ALIGN - 1) & ~(BLOCK_ALIGN - 1); + } + + public: + NodePool() = default; + NodePool(const NodePool&) = delete; + NodePool& operator=(const NodePool&) = delete; + ~NodePool() { + for (void* chunk : chunks) { + std::free(chunk); + } + } + + void* allocate(std::size_t size) { + const std::size_t step = roundedSize(size); + if (step > MAX_POOLED_SIZE) { + return ::operator new(size); + } + void*& freeHead = freeHeads[step / BLOCK_ALIGN - 1]; + if (freeHead != nullptr) { + void* block = freeHead; + freeHead = *static_cast(block); + return block; + } + if (cursor == nullptr || cursor + step > chunkEnd) { + void* chunk = std::malloc(CHUNK_SIZE); + if (chunk == nullptr) { + throw std::bad_alloc(); + } + chunks.push_back(chunk); + cursor = static_cast(chunk); + chunkEnd = cursor + CHUNK_SIZE; + } + void* block = cursor; + cursor += step; + return block; + } + + void deallocate(void* block, std::size_t size) noexcept { + const std::size_t step = roundedSize(size); + if (step > MAX_POOLED_SIZE) { + ::operator delete(block); + return; + } + void*& freeHead = freeHeads[step / BLOCK_ALIGN - 1]; + *static_cast(block) = freeHead; + freeHead = block; + } + }; + + template + struct PoolAllocator { + using value_type = T; + NodePool* pool; + + explicit PoolAllocator(NodePool* pool) noexcept : pool(pool) { + } + template + PoolAllocator(const PoolAllocator& other) noexcept : pool(other.pool) { + } + template + bool operator==(const PoolAllocator& other) const noexcept { + return pool == other.pool; + } + + T* allocate(std::size_t n) { + if (n == 1) { + return static_cast(pool->allocate(sizeof(T))); + } + // Bucket arrays (n > 1) are few and resize rarely; the heap handles them. + return static_cast(::operator new(n * sizeof(T))); + } + void deallocate(T* p, std::size_t n) noexcept { + if (n == 1) { + pool->deallocate(p, sizeof(T)); + } else { + ::operator delete(p); + } + } + }; + + using CountsMap = std::unordered_map, std::equal_to, + PoolAllocator>>; + + NodePool pool; // Must be declared before counts so it outlives the map's nodes. + CountsMap counts {0, std::hash {}, std::equal_to {}, + PoolAllocator> {&pool}}; uint64_t alive_hash = 0; public: From 6f6f81b52450e183d7f8ffc73ce901b44e9affaf Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 9 Jul 2026 05:36:43 +0000 Subject: [PATCH 4/5] Back AliveVariableHash's counts map with common::Arena instead of a bespoke pool Replaces the ad-hoc free-list NodePool/PoolAllocator with nautilus's existing bump-pointer Arena (already used for Block/TraceOperation). Arena only supports bulk reclamation, so node-level erase() stays for hash correctness while the actual memory reclaim happens in reset(), which resume() already calls after every symbolic-execution iteration. Bucket arrays keep going through the regular heap so a softReset() can never invalidate memory the map still references. --- .../tracing/ExceptionBasedTraceContext.hpp | 119 +++++------------- 1 file changed, 28 insertions(+), 91 deletions(-) diff --git a/nautilus/src/nautilus/tracing/ExceptionBasedTraceContext.hpp b/nautilus/src/nautilus/tracing/ExceptionBasedTraceContext.hpp index 304e04782..3d7595350 100644 --- a/nautilus/src/nautilus/tracing/ExceptionBasedTraceContext.hpp +++ b/nautilus/src/nautilus/tracing/ExceptionBasedTraceContext.hpp @@ -9,10 +9,8 @@ #include "nautilus/tracing/TracingInterface.hpp" #include "tag/Tag.hpp" #include "tag/TagRecorder.hpp" -#include #include #include -#include #include #include #include @@ -59,9 +57,11 @@ inline size_t getStaticVarValue(const StaticVarHolder& holder) { * - Each variable ID is mixed with a constant multiplier for better hash distribution * - The hash incorporates both variable identity (ID) and reference count * - Uses unordered_map for sparse storage (only allocates for variables that exist) - * - Map nodes come from an internal free-list pool (see NodePool); entries are erased - * when their count reaches zero, so memory stays bounded by the peak number of - * alive variables instead of growing with every value ref ever seen + * - Map nodes are bump-allocated from an internal common::Arena and entries are erased + * when their count reaches zero, so the live entry set stays bounded by the peak + * number of alive variables instead of growing with every value ref ever seen. The + * arena itself is bulk-reclaimed by reset(), which resume() calls after every + * symbolic-execution iteration, so per-iteration node churn does not accumulate. * * Performance characteristics: * - increment(): O(1) average - hash map lookup + two XOR operations, two multiplications @@ -73,112 +73,47 @@ inline size_t getStaticVarValue(const StaticVarHolder& holder) { class AliveVariableHash { static constexpr uint64_t HASH_MULTIPLIER = 0x9e3779b97f4a7c15; // Golden ratio constant for good mixing - // Free-list node pool backing the counts map. The tracing workload inserts and - // erases one map node per value lifetime; routing those through the general-purpose - // allocator costs a malloc/free pair per traced value, which measurably slows - // tracing of large functions. The pool hands blocks out of bump-allocated chunks - // and recycles freed blocks via per-size free lists (the allocator is rebound to - // different types - nodes and bucket arrays - so size classes must not be mixed). - // Memory stays bounded by the peak number of alive variables. - class NodePool { - static constexpr std::size_t CHUNK_SIZE = 16 * 1024; - static constexpr std::size_t BLOCK_ALIGN = alignof(std::max_align_t); - static constexpr std::size_t MAX_POOLED_SIZE = 4 * BLOCK_ALIGN; - static constexpr std::size_t NUM_SIZE_CLASSES = MAX_POOLED_SIZE / BLOCK_ALIGN; - - std::vector chunks; - std::array freeHeads {}; - std::byte* cursor = nullptr; - std::byte* chunkEnd = nullptr; - - static constexpr std::size_t roundedSize(std::size_t size) noexcept { - return (size + BLOCK_ALIGN - 1) & ~(BLOCK_ALIGN - 1); - } - - public: - NodePool() = default; - NodePool(const NodePool&) = delete; - NodePool& operator=(const NodePool&) = delete; - ~NodePool() { - for (void* chunk : chunks) { - std::free(chunk); - } - } - - void* allocate(std::size_t size) { - const std::size_t step = roundedSize(size); - if (step > MAX_POOLED_SIZE) { - return ::operator new(size); - } - void*& freeHead = freeHeads[step / BLOCK_ALIGN - 1]; - if (freeHead != nullptr) { - void* block = freeHead; - freeHead = *static_cast(block); - return block; - } - if (cursor == nullptr || cursor + step > chunkEnd) { - void* chunk = std::malloc(CHUNK_SIZE); - if (chunk == nullptr) { - throw std::bad_alloc(); - } - chunks.push_back(chunk); - cursor = static_cast(chunk); - chunkEnd = cursor + CHUNK_SIZE; - } - void* block = cursor; - cursor += step; - return block; - } - - void deallocate(void* block, std::size_t size) noexcept { - const std::size_t step = roundedSize(size); - if (step > MAX_POOLED_SIZE) { - ::operator delete(block); - return; - } - void*& freeHead = freeHeads[step / BLOCK_ALIGN - 1]; - *static_cast(block) = freeHead; - freeHead = block; - } - }; - + // Allocator adapter that backs the counts map's nodes with nautilus's standard + // common::Arena (the same bump-pointer pool used for Block / TraceOperation). + // Arena only supports bulk reclamation, not freeing a single node, so deallocate() + // for a node is a no-op: the actual reclaim happens when AliveVariableHash::reset() + // calls arena.softReset(), which resume() does after every symbolic-execution + // iteration. Bucket arrays (n > 1, resized rarely) go through the regular heap + // instead, so a softReset() can never invalidate memory the map still references. template - struct PoolAllocator { + struct ArenaAllocator { using value_type = T; - NodePool* pool; + Arena* arena; - explicit PoolAllocator(NodePool* pool) noexcept : pool(pool) { + explicit ArenaAllocator(Arena* arena) noexcept : arena(arena) { } template - PoolAllocator(const PoolAllocator& other) noexcept : pool(other.pool) { + ArenaAllocator(const ArenaAllocator& other) noexcept : arena(other.arena) { } template - bool operator==(const PoolAllocator& other) const noexcept { - return pool == other.pool; + bool operator==(const ArenaAllocator& other) const noexcept { + return arena == other.arena; } T* allocate(std::size_t n) { if (n == 1) { - return static_cast(pool->allocate(sizeof(T))); + return static_cast(arena->allocate(sizeof(T), alignof(T))); } - // Bucket arrays (n > 1) are few and resize rarely; the heap handles them. return static_cast(::operator new(n * sizeof(T))); } void deallocate(T* p, std::size_t n) noexcept { - if (n == 1) { - pool->deallocate(p, sizeof(T)); - } else { + if (n != 1) { ::operator delete(p); } } }; using CountsMap = std::unordered_map, std::equal_to, - PoolAllocator>>; + ArenaAllocator>>; - NodePool pool; // Must be declared before counts so it outlives the map's nodes. + Arena arena; // Must be declared before counts so it outlives the map's nodes. CountsMap counts {0, std::hash {}, std::equal_to {}, - PoolAllocator> {&pool}}; + ArenaAllocator> {&arena}}; uint64_t alive_hash = 0; public: @@ -255,16 +190,18 @@ class AliveVariableHash { /** * @brief Resets all reference counts and hash to initial state. * - * This efficiently clears all counts without creating a temporary object. * Since decrement() erases entries when they reach zero, a balanced trace iteration - * leaves the map empty and this is a no-op. The emptiness check (rather than a hash - * check) also guards against the rare XOR collision where live entries hash to 0. + * already leaves the map empty; the emptiness check (rather than a hash check) also + * guards against the rare XOR collision where live entries hash to 0. clear() runs + * first so every node is destroyed (and its arena-backed deallocate() no-op invoked) + * before softReset() bulk-invalidates the arena's bump-allocated node storage. */ inline void reset() noexcept { if (!counts.empty()) { counts.clear(); } alive_hash = 0; + arena.softReset(); } }; From ddb6437d81d77b4dc1e99476c56064ad1b2f4a78 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 9 Jul 2026 06:20:19 +0000 Subject: [PATCH 5/5] Drop the Arena-backed allocator for AliveVariableHash's counts map A shared trace Arena can't be soft-reset per iteration without destroying prior iterations' Block/TraceOperation data, and a private Arena adds complexity for a benefit that isn't worth the risk here. Fall back to a plain std::unordered_map with the default allocator; the two correctness fixes (erase on decrement, reset() checks emptiness) are what actually bound memory, independent of the allocator backing the map. --- .../tracing/ExceptionBasedTraceContext.hpp | 58 ++----------------- 1 file changed, 6 insertions(+), 52 deletions(-) diff --git a/nautilus/src/nautilus/tracing/ExceptionBasedTraceContext.hpp b/nautilus/src/nautilus/tracing/ExceptionBasedTraceContext.hpp index 3d7595350..e738ca1b1 100644 --- a/nautilus/src/nautilus/tracing/ExceptionBasedTraceContext.hpp +++ b/nautilus/src/nautilus/tracing/ExceptionBasedTraceContext.hpp @@ -15,7 +15,6 @@ #include #include #include -#include #include #include #include @@ -57,11 +56,8 @@ inline size_t getStaticVarValue(const StaticVarHolder& holder) { * - Each variable ID is mixed with a constant multiplier for better hash distribution * - The hash incorporates both variable identity (ID) and reference count * - Uses unordered_map for sparse storage (only allocates for variables that exist) - * - Map nodes are bump-allocated from an internal common::Arena and entries are erased - * when their count reaches zero, so the live entry set stays bounded by the peak - * number of alive variables instead of growing with every value ref ever seen. The - * arena itself is bulk-reclaimed by reset(), which resume() calls after every - * symbolic-execution iteration, so per-iteration node churn does not accumulate. + * - Entries are erased when their count reaches zero, so the map stays bounded by the + * peak number of alive variables instead of growing with every value ref ever seen * * Performance characteristics: * - increment(): O(1) average - hash map lookup + two XOR operations, two multiplications @@ -73,47 +69,7 @@ inline size_t getStaticVarValue(const StaticVarHolder& holder) { class AliveVariableHash { static constexpr uint64_t HASH_MULTIPLIER = 0x9e3779b97f4a7c15; // Golden ratio constant for good mixing - // Allocator adapter that backs the counts map's nodes with nautilus's standard - // common::Arena (the same bump-pointer pool used for Block / TraceOperation). - // Arena only supports bulk reclamation, not freeing a single node, so deallocate() - // for a node is a no-op: the actual reclaim happens when AliveVariableHash::reset() - // calls arena.softReset(), which resume() does after every symbolic-execution - // iteration. Bucket arrays (n > 1, resized rarely) go through the regular heap - // instead, so a softReset() can never invalidate memory the map still references. - template - struct ArenaAllocator { - using value_type = T; - Arena* arena; - - explicit ArenaAllocator(Arena* arena) noexcept : arena(arena) { - } - template - ArenaAllocator(const ArenaAllocator& other) noexcept : arena(other.arena) { - } - template - bool operator==(const ArenaAllocator& other) const noexcept { - return arena == other.arena; - } - - T* allocate(std::size_t n) { - if (n == 1) { - return static_cast(arena->allocate(sizeof(T), alignof(T))); - } - return static_cast(::operator new(n * sizeof(T))); - } - void deallocate(T* p, std::size_t n) noexcept { - if (n != 1) { - ::operator delete(p); - } - } - }; - - using CountsMap = std::unordered_map, std::equal_to, - ArenaAllocator>>; - - Arena arena; // Must be declared before counts so it outlives the map's nodes. - CountsMap counts {0, std::hash {}, std::equal_to {}, - ArenaAllocator> {&arena}}; + std::unordered_map counts; uint64_t alive_hash = 0; public: @@ -190,18 +146,16 @@ class AliveVariableHash { /** * @brief Resets all reference counts and hash to initial state. * + * This efficiently clears all counts without creating a temporary object. * Since decrement() erases entries when they reach zero, a balanced trace iteration - * already leaves the map empty; the emptiness check (rather than a hash check) also - * guards against the rare XOR collision where live entries hash to 0. clear() runs - * first so every node is destroyed (and its arena-backed deallocate() no-op invoked) - * before softReset() bulk-invalidates the arena's bump-allocated node storage. + * leaves the map empty and this is a no-op. The emptiness check (rather than a hash + * check) also guards against the rare XOR collision where live entries hash to 0. */ inline void reset() noexcept { if (!counts.empty()) { counts.clear(); } alive_hash = 0; - arena.softReset(); } };