diff --git a/cpp/ghost_repro.cpp b/cpp/ghost_repro.cpp new file mode 100644 index 00000000..3ca97bca --- /dev/null +++ b/cpp/ghost_repro.cpp @@ -0,0 +1,103 @@ +// Regression test for the slot-reuse "ghost" bug in `index_dense_gt`. +// +// ORIGINAL BUG (usearch 2.25.3): the key->slot map `slot_lookup_` is a tombstoned +// open-addressing hash set (`flat_hash_multi_set_gt`, index_plugins.hpp). Its +// `try_emplace` used to grow the table only when the LIVE load factor reached 2/3 +// (`populated_slots_ * 3 >= capacity_slots_ * 2`) and it never rehashed to clear +// tombstones. Under churn (`remove` + `add`) that holds the live count roughly +// constant, the table never grew while every `remove` left another tombstone. +// Once `live + tombstones` filled the table (no empty slot remained), a lookup +// probe saturated after `capacity_slots_` steps and `equal_range` returned +// `end()` even for a key that was still live. `remove()` then took the empty-range +// branch and returned `completed == 0` WITHOUT an error -- the key was never +// reclaimed. The entry became a "ghost": `contains(key)` kept reporting it, +// `remove(key)` silently no-oped, and `size()` drifted above the true live count. +// +// THE FIX (index_plugins.hpp): the table now tracks tombstones (`deleted_slots_`) +// and the resize decision counts non-empty slots (live + tombstones). When that +// load reaches 2/3, `try_reserve` rehashes -- growing if the live count truly +// demands it, otherwise rehashing in place at the same capacity to reclaim the +// tombstones. An empty slot therefore always remains for probes to terminate on. +// +// This test asserts NO ghosts appear under heavy churn at either a tight or a +// generous reserve. Before the fix the tight-reserve case leaked thousands of +// ghosts; after the fix both report zero. +// +// Build (no external deps): +// g++ -std=c++17 -O2 -Iinclude -DUSEARCH_USE_SIMSIMD=0 -DUSEARCH_USE_FP16LIB=0 \ +// cpp/ghost_repro.cpp -o ghost_repro && ./ghost_repro + +#include +#include +#include +#include + +#include + +using namespace unum::usearch; +using index_t = index_dense_gt; + +static std::size_t run(std::size_t reserve) { + constexpr std::size_t dim = 64; + constexpr std::size_t live = 2000; + constexpr std::size_t churn = 8 * live; + + std::mt19937 rng(2026); + auto make_vec = [&]() { + std::vector v(dim); + std::uniform_real_distribution d(-1.0f, 1.0f); + for (auto& x : v) x = d(rng); + return v; + }; + + metric_punned_t metric(dim, metric_kind_t::cos_k, scalar_kind_t::f32_k); + index_dense_config_t config(16, 96, 48); + index_t index = std::move(index_t::make(metric, config).index); + index.try_reserve(reserve); + + std::vector live_keys; + std::int64_t next = 0; + for (std::size_t i = 0; i < live; ++i) { + auto v = make_vec(); + index.add(next, v.data()); + live_keys.push_back(next); + ++next; + } + + std::size_t remove_fails = 0, contained_but_failed = 0; + for (std::size_t c = 0; c < churn; ++c) { + std::size_t j = rng() % live_keys.size(); + std::int64_t victim = live_keys[j]; + bool present = index.contains(victim); + if (index.remove(victim).completed != 1) { + ++remove_fails; + if (present) ++contained_but_failed; + } + auto v = make_vec(); + index.add(next, v.data()); // reuses the freed slot + live_keys[j] = next; + ++next; + } + + std::size_t ghosts = index.size() - live; + std::printf(" reserve=%zu (cap_slots~%zu) churn=%zu | removeFails=%zu (contains()==true but remove fails: %zu) " + "| index.size()=%zu vs true live=%zu -> GHOSTS=%zu\n", + reserve, reserve * 3 / 2, churn, remove_fails, contained_but_failed, index.size(), live, ghosts); + return ghosts; +} + +int main() { + std::printf("usearch slot-reuse ghost regression test (v%d.%d.%d)\n", USEARCH_VERSION_MAJOR, USEARCH_VERSION_MINOR, + USEARCH_VERSION_PATCH); + + std::printf("[tight reserve] churn (16000) exceeds capacity_slots (~9000) -> tombstones must be reclaimed:\n"); + std::size_t ghosts_tight = run(/*reserve=*/2000 * 3); + + std::printf("[generous reserve] churn stays below capacity_slots:\n"); + std::size_t ghosts_generous = run(/*reserve=*/2000 + 8 * 2000 + 16); + + bool passed = ghosts_tight == 0 && ghosts_generous == 0; + std::printf("\n%s\n", passed ? "PASS: no ghosts under churn at either reserve." + : "FAIL: ghosts leaked under churn -- the slot-reuse bug has regressed."); + return passed ? 0 : 1; +} diff --git a/include/usearch/index_plugins.hpp b/include/usearch/index_plugins.hpp index e27eb859..21e3cde8 100644 --- a/include/usearch/index_plugins.hpp +++ b/include/usearch/index_plugins.hpp @@ -3746,6 +3746,8 @@ class flat_hash_multi_set_gt { char* data_ = nullptr; std::size_t buckets_ = 0; std::size_t populated_slots_ = 0; + /// @brief Number of tombstones (slots marked deleted but not yet reclaimed) + std::size_t deleted_slots_ = 0; /// @brief Number of slots std::size_t capacity_slots_ = 0; @@ -3805,9 +3807,11 @@ class flat_hash_multi_set_gt { if (!data_) usearch_raise_runtime_error("failed memory allocation"); - // Copy metadata + // Copy metadata. Only live entries are copied below, so the new table + // starts with no tombstones. buckets_ = other.buckets_; populated_slots_ = other.populated_slots_; + deleted_slots_ = 0; capacity_slots_ = other.capacity_slots_; // Initialize new buckets to empty @@ -3848,9 +3852,11 @@ class flat_hash_multi_set_gt { if (!data_) usearch_raise_runtime_error("failed memory allocation"); - // Copy metadata + // Copy metadata. Only live entries are copied below, so the new table + // starts with no tombstones. buckets_ = other.buckets_; populated_slots_ = other.populated_slots_; + deleted_slots_ = 0; capacity_slots_ = other.capacity_slots_; // Initialize new buckets to empty @@ -3880,6 +3886,7 @@ class flat_hash_multi_set_gt { if (data_) std::memset(data_, 0, buckets_ * bytes_per_bucket()); populated_slots_ = 0; + deleted_slots_ = 0; } void reset() noexcept { @@ -3889,11 +3896,16 @@ class flat_hash_multi_set_gt { data_ = nullptr; buckets_ = 0; populated_slots_ = 0; + deleted_slots_ = 0; capacity_slots_ = 0; } bool try_reserve(std::size_t capacity) noexcept { - if (capacity <= this->capacity()) + // Nothing to do only when the requested capacity already fits *and* there + // are no tombstones to reclaim. Tombstones must be cleared even without a + // grow request, otherwise they accumulate under churn until no empty slot + // remains and lookups can no longer terminate ("ghost" entries). + if (capacity <= this->capacity() && deleted_slots_ == 0) return true; // Calculate new sizes @@ -3913,7 +3925,16 @@ class flat_hash_multi_set_gt { checked_size_result_t new_slots = checked_mul(new_buckets_checked.value, slots_per_bucket()); if (!new_slots) return false; - checked_size_result_t new_bytes = checked_mul(new_buckets_checked.value, bytes_per_bucket()); + + // Never shrink: when we are only here to reclaim tombstones, a same-size + // rehash is enough. Clamp the target up to the current capacity. + std::size_t target_buckets = new_buckets_checked.value; + std::size_t target_slots = new_slots.value; + if (target_slots < capacity_slots_) { + target_buckets = buckets_; + target_slots = capacity_slots_; + } + checked_size_result_t new_bytes = checked_mul(target_buckets, bytes_per_bucket()); if (!new_bytes) return false; @@ -3934,7 +3955,7 @@ class flat_hash_multi_set_gt { // Rehash std::size_t hash_value = hasher(old_slot.element); - std::size_t new_slot_index = hash_value & (new_slots.value - 1); + std::size_t new_slot_index = hash_value & (target_slots - 1); // Linear probing to find an empty slot in new_data while (true) { @@ -3944,16 +3965,18 @@ class flat_hash_multi_set_gt { new_slot.header.populated |= new_slot.mask; break; } - new_slot_index = (new_slot_index + 1) & (new_slots.value - 1); + new_slot_index = (new_slot_index + 1) & (target_slots - 1); } } - // Deallocate old data and update pointers and sizes + // Deallocate old data and update pointers and sizes. Tombstones are not + // carried over by the rehash above, so the new table has none. if (data_) allocator_t{}.deallocate(data_, buckets_ * bytes_per_bucket()); data_ = new_data; - buckets_ = new_buckets_checked.value; - capacity_slots_ = new_slots.value; + buckets_ = target_buckets; + capacity_slots_ = target_slots; + deleted_slots_ = 0; return true; } @@ -4082,6 +4105,7 @@ class flat_hash_multi_set_gt { // Found a match, mark as deleted slot.header.deleted |= slot.mask; --populated_slots_; + ++deleted_slots_; popped_value = slot.element; return true; // Successfully removed } @@ -4117,6 +4141,7 @@ class flat_hash_multi_set_gt { // Found a match, mark as deleted slot.header.deleted |= slot.mask; --populated_slots_; + ++deleted_slots_; ++count; // Increment count of elements removed } } else { @@ -4247,8 +4272,12 @@ class flat_hash_multi_set_gt { } bool try_emplace(element_t const& element) noexcept { - // Check if we need to resize - if (populated_slots_ * 3u >= capacity_slots_ * 2u) + // Resize/rehash based on the number of non-empty slots (live entries plus + // tombstones), not just the live count. Counting only live entries lets + // tombstones pile up under churn until the table saturates, after which a + // probe can no longer reach a terminating empty slot. When tombstones + // dominate, `try_reserve` reclaims them in place without growing. + if ((populated_slots_ + deleted_slots_) * 3u >= capacity_slots_ * 2u) if (!try_reserve(populated_slots_ + 1)) return false; @@ -4260,9 +4289,12 @@ class flat_hash_multi_set_gt { while (true) { slot_ref_t slot = slot_ref(slot_index); if ((~slot.header.populated & slot.mask) | (slot.header.deleted & slot.mask)) { - // Found an empty or deleted slot + // Found an empty or deleted slot. Reusing a tombstone reclaims it. + bool reused_tombstone = (slot.header.populated & slot.header.deleted & slot.mask) != 0; populate_slot(slot, element); ++populated_slots_; + if (reused_tombstone) + --deleted_slots_; return true; } // Move to the next slot