refactor: extract total_count_ to InnerIndexInterface base class#2157
refactor: extract total_count_ to InnerIndexInterface base class#2157LHT129 wants to merge 1 commit into
Conversation
Merge ProtectionsYour pull request matches the following merge protections and will not be merged until they are valid. 🟢 Require kind labelWonderful, this rule succeeded.
🟢 Require version labelWonderful, this rule succeeded.
|
There was a problem hiding this comment.
Code Review
This pull request refactors total_count_ into a shared atomic member variable in InnerIndexInterface, updating its usage across the BruteForce and WARP implementations. The review feedback identifies critical race conditions in the Add methods of both algorithms, where total_count_ is incremented before data insertion is complete, potentially exposing uninitialized data to concurrent searches. Additionally, the reviewer recommends caching total_count_.load() in WARP search and range search methods to prevent inconsistencies and reduce atomic load overhead.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
Move total_count_ from HGraph, BruteForce, and WARP into the InnerIndexInterface base class as std::atomic<uint64_t>. This eliminates code duplication, ensures consistent thread-safe access across all index implementations, and unifies the element count management. Signed-off-by: LHT129 <tianlan.lht@antgroup.com> Assisted-by: ClaudeCode:claude-opus-4-6
Summary
Move
total_count_from HGraph, BruteForce, and WARP into theInnerIndexInterfacebase class asstd::atomic<uint64_t>.Changes
std::atomic<uint64_t> total_count_{0}to the protected section ofInnerIndexInterfacetotal_count_member fromHGraph,BruteForce, andWARPBruteForceandWARPto use atomic operations (.load(),.store(), prefix++/--) for alltotal_count_accessesstd::atomic<uint64_t>so its usage is unchangedMotivation
uint64_t)Supersedes #1682 (closed due to directory restructure in #2127).
Closes: #1681