From 892b187cb6dfe2beaee01d8481ce610bab7b82b9 Mon Sep 17 00:00:00 2001 From: gouyt13clear Date: Sat, 31 Oct 2026 14:42:08 +0800 Subject: [PATCH 1/4] perf(core): flip negative-coordinate codes without a branch ex_bits_code_with_norm reverted the code of each non-positive coordinate behind a branch. Query signs are effectively random, so baseline x86-64 builds mispredicted about half of them (about 480 per 960-dimension query) and could not vectorize the loop. Select between the two values instead; the codes are identical by construction. --- include/rabitqlib/core/detail/rabitq_impl.hpp | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/include/rabitqlib/core/detail/rabitq_impl.hpp b/include/rabitqlib/core/detail/rabitq_impl.hpp index 80e40107..6fef536a 100644 --- a/include/rabitqlib/core/detail/rabitq_impl.hpp +++ b/include/rabitqlib/core/detail/rabitq_impl.hpp @@ -608,13 +608,13 @@ inline T ex_bits_code_with_norm( } } - // Revert codes for negative or zero dims + // Revert codes for negative or zero dims. A select instead of a branch avoids a + // misprediction per random-sign coordinate and lets baseline builds vectorize. int32_t mask = (1 << ex_bits) - 1; for (size_t j = 0; j < dim; ++j) { - if (residual_arr.data()[j] <= 0) { - TP tmp = ex_code[j]; - ex_code[j] = (~tmp) & mask; - } + const TP code = ex_code[j]; + const auto reverted = static_cast((~code) & mask); + ex_code[j] = residual_arr.data()[j] <= 0 ? reverted : code; } return ipnorm_inv; From 96e6cc5f48958ba8b01f84e5fc18c41a6f09474a Mon Sep 17 00:00:00 2001 From: gouyt13clear Date: Sat, 31 Oct 2026 14:42:08 +0800 Subject: [PATCH 2/4] fix(hnsw): keep per-ISA search builds from sharing generic code Each src/index/hnsw_search_.cpp instantiated the whole search, including query quantization, the visited-set pool and std containers, under AVX2 or AVX-512 flags. Functions the compiler did not inline became weak symbols that generic objects also define, and the linker keeps one copy for every caller, so a generic caller could run AVX code. Optimized GCC builds shared 7 such functions with different code and Clang builds 13. Generic code (HierarchicalNSW::search_knn_sorted_with) now prepares the query, computes centroid distances, acquires the visited set and search buffer, and formats results. Each ISA source only exports search_layers_, which traverses the graph; its Kernel type is in an anonymous namespace and the result queue is a local class, so everything instantiated with them keeps internal linkage. The per-backend search_knn_sorted_ entry points keep their signatures for tests. Search results are bit-identical. --- .../rabitqlib/index/detail/hnsw_dispatch.hpp | 69 +++++++ include/rabitqlib/index/hnsw/hnsw.hpp | 82 +++------ src/index/hnsw.cpp | 78 ++++++++ src/index/hnsw_dispatch.cpp | 59 ++++-- src/index/hnsw_search_avx2.cpp | 20 +- src/index/hnsw_search_avx512_core.cpp | 20 +- src/index/hnsw_search_avx512_popcnt.cpp | 20 +- src/index/hnsw_search_generic.cpp | 20 +- src/index/hnsw_search_impl.hpp | 174 ++++++++++-------- src/index/hnsw_search_neon.cpp | 20 +- 10 files changed, 388 insertions(+), 174 deletions(-) diff --git a/include/rabitqlib/index/detail/hnsw_dispatch.hpp b/include/rabitqlib/index/detail/hnsw_dispatch.hpp index d65a2646..453873ff 100644 --- a/include/rabitqlib/index/detail/hnsw_dispatch.hpp +++ b/include/rabitqlib/index/detail/hnsw_dispatch.hpp @@ -4,8 +4,18 @@ #include #include +#include "rabitqlib/index/detail/visited_set_fwd.hpp" #include "rabitqlib/index/types.hpp" +namespace rabitqlib { +template +class SplitSingleQuery; +namespace buffer { +template +class SearchBuffer; +} // namespace buffer +} // namespace rabitqlib + namespace rabitqlib::hnsw { class HierarchicalNSW; namespace detail { @@ -29,5 +39,64 @@ std::vector> search_knn_sorted_avx512_popcnt( HierarchicalNSW& index, const float* rotated_query, size_t k, size_t ef_search = 0 ); +// Graph traversal for one prepared query, compiled once per ISA. Query preparation, +// the visited set, the ef_search-sized search_pool and result formatting stay in +// generic code so that only these functions are built with ISA flags. Fills +// `nearest` (capacity k) in ascending distance order and returns the number of results. +using SearchLayersFn = size_t (*)( + const HierarchicalNSW& index, + const SplitSingleQuery& query_wrapper, + const float* q_to_centroids, + VisitedSet& visited, + buffer::SearchBuffer& search_pool, + size_t k, + AnnCandidate* nearest +); +size_t search_layers_generic( + const HierarchicalNSW& index, + const SplitSingleQuery& query_wrapper, + const float* q_to_centroids, + VisitedSet& visited, + buffer::SearchBuffer& search_pool, + size_t k, + AnnCandidate* nearest +); +size_t search_layers_neon( + const HierarchicalNSW& index, + const SplitSingleQuery& query_wrapper, + const float* q_to_centroids, + VisitedSet& visited, + buffer::SearchBuffer& search_pool, + size_t k, + AnnCandidate* nearest +); +size_t search_layers_avx2( + const HierarchicalNSW& index, + const SplitSingleQuery& query_wrapper, + const float* q_to_centroids, + VisitedSet& visited, + buffer::SearchBuffer& search_pool, + size_t k, + AnnCandidate* nearest +); +size_t search_layers_avx512_core( + const HierarchicalNSW& index, + const SplitSingleQuery& query_wrapper, + const float* q_to_centroids, + VisitedSet& visited, + buffer::SearchBuffer& search_pool, + size_t k, + AnnCandidate* nearest +); +size_t search_layers_avx512_popcnt( + const HierarchicalNSW& index, + const SplitSingleQuery& query_wrapper, + const float* q_to_centroids, + VisitedSet& visited, + buffer::SearchBuffer& search_pool, + size_t k, + AnnCandidate* nearest +); + } // namespace detail } // namespace rabitqlib::hnsw diff --git a/include/rabitqlib/index/hnsw/hnsw.hpp b/include/rabitqlib/index/hnsw/hnsw.hpp index 97c32896..3f2946de 100644 --- a/include/rabitqlib/index/hnsw/hnsw.hpp +++ b/include/rabitqlib/index/hnsw/hnsw.hpp @@ -1,7 +1,6 @@ // HNSW is developed from the [HNSW library](https://github.com/nmslib/hnswlib) #pragma once -#include #include #include #include @@ -36,7 +35,8 @@ struct DistanceEstimate; namespace rabitqlib::hnsw { namespace detail { struct QuantQuery; -} +struct HnswSearchAccess; +} // namespace detail template using maxheap = std::priority_queue; @@ -169,52 +169,8 @@ class HierarchicalNSW { // Borrows construct() input only while its workers run; cleared on success or failure. const float* construction_data_{nullptr}; - // Bounded priority queue implemented as a sorted vector. - class BoundedKNN { - public: - explicit BoundedKNN(size_t capacity) : capacity_(capacity) {} - - // Insert a candidate in sorted order (ascending by distance). - void insert(const AnnCandidate& candidate) { - auto it = std::upper_bound(queue_.begin(), queue_.end(), candidate); - queue_.insert(it, candidate); - // If we exceed capacity, drop the worst candidate (largest distance). - if (queue_.size() > capacity_) { - queue_.pop_back(); - } - } - - // Returns the worst (largest distance) candidate. - [[nodiscard]] const AnnCandidate& worst() const { return queue_.back(); } - - [[nodiscard]] size_t size() const { return queue_.size(); } - - [[nodiscard]] const std::vector>& candidates() const { - return queue_; - } - - private: - size_t capacity_; - // Sorted in ascending order by distance so that the worst is at the back. - std::vector> queue_; - }; - friend struct HnswTestAccess; - friend std::vector> detail::search_knn_sorted_neon( - HierarchicalNSW&, const float*, size_t, size_t - ); - friend std::vector> detail::search_knn_sorted_generic( - HierarchicalNSW&, const float*, size_t, size_t - ); - friend std::vector> detail::search_knn_sorted_avx2( - HierarchicalNSW&, const float*, size_t, size_t - ); - friend std::vector> detail::search_knn_sorted_avx512_core( - HierarchicalNSW&, const float*, size_t, size_t - ); - friend std::vector> detail::search_knn_sorted_avx512_popcnt( - HierarchicalNSW&, const float*, size_t, size_t - ); + friend struct detail::HnswSearchAccess; // `remove` sets the high bit of a point's stored cluster ID. construct and load // keep num_clusters_ at or below 2^31, so a real cluster ID never has it. @@ -407,10 +363,18 @@ class HierarchicalNSW { *(reinterpret_cast(links)) = count; } - // ANN Search + // ANN Search. search_knn_sorted_with prepares the query in generic code and runs + // one ISA's search_layers_direct (instantiated in src/index/hnsw_search_*.cpp). + std::vector> search_knn_sorted_with( + detail::SearchLayersFn search_layers, + const float* rotated_query, + size_t k, + size_t ef_search + ) const; + template void estimate_bin_direct( - const std::vector& q_to_centroids, + const float* q_to_centroids, const SplitSingleQuery& query_wrapper, PID candidate_id, DistanceEstimate& estimate @@ -418,25 +382,31 @@ class HierarchicalNSW { template void estimate_full_direct( - const std::vector& q_to_centroids, + const float* q_to_centroids, const SplitSingleQuery& query_wrapper, PID candidate_id, DistanceEstimate& estimate ) const; template - std::vector> search_knn_sorted_direct( - const float* rotated_query, size_t k, size_t ef_search + size_t search_layers_direct( + const SplitSingleQuery& query_wrapper, + const float* q_to_centroids, + VisitedSet& visited, + buffer::SearchBuffer& search_pool, + size_t k, + AnnCandidate* nearest ) const; - template + template void search_base_layer_direct( PID entry_point_id, - size_t ef_search, size_t k, const SplitSingleQuery& query_wrapper, - const std::vector& q_to_centroids, - BoundedKNN& nearest_candidates + const float* q_to_centroids, + VisitedSet& visited, + buffer::SearchBuffer& search_pool, + Results& nearest_candidates ) const; // Raw-vector distance used by construct; add scores links with quantized_distance. diff --git a/src/index/hnsw.cpp b/src/index/hnsw.cpp index 17df2a52..d70bc774 100644 --- a/src/index/hnsw.cpp +++ b/src/index/hnsw.cpp @@ -930,6 +930,84 @@ std::vector>> HierarchicalNSW::search( return results; } +std::vector> HierarchicalNSW::search_knn_sorted_with( + detail::SearchLayersFn search_layers, + const float* rotated_query, + size_t k, + size_t ef_search +) const { + std::vector> result; + if (num_stored_points_ == 0) { + return result; + } + if (ef_search == 0) { + ef_search = kDefaultEfSearch; + } + + SplitSingleQuery query_wrapper( + rotated_query, padded_dim_, ex_bits_, query_config_, metric_type_ + ); + + // Preprocess - get the distance from query to all centroids + std::vector q_to_centroids(num_clusters_); + + if (metric_type_ == METRIC_L2) { + for (size_t i = 0; i < num_clusters_; ++i) { + q_to_centroids[i] = std::sqrt(raw_dist_func_( + rotated_query, + reinterpret_cast(centroids_memory_.get()) + (i * padded_dim_), + padded_dim_ + )); + } + } else if (metric_type_ == METRIC_IP) { + q_to_centroids.resize(2 * num_clusters_); + // First half stores dot(query, centroid); second half stores ||query - centroid||. + for (size_t i = 0; i < num_clusters_; ++i) { + q_to_centroids[i] = dot_product( + rotated_query, + reinterpret_cast(centroids_memory_.get()) + (i * padded_dim_), + padded_dim_ + ); + q_to_centroids[i + num_clusters_] = std::sqrt(euclidean_sqr( + rotated_query, + reinterpret_cast(centroids_memory_.get()) + (i * padded_dim_), + padded_dim_ + )); + } + } + + auto visited = visited_set_pool_->acquire(); + // Keep the ef_search best estimates in a bounded sorted buffer. + buffer::SearchBuffer search_pool(std::max(ef_search, k)); + std::vector> nearest(k); + const size_t num_results = search_layers( + *this, + query_wrapper, + q_to_centroids.data(), + *visited, + search_pool, + k, + nearest.data() + ); + visited_set_pool_->release(std::move(visited)); + + result.reserve(num_results); + for (size_t i = 0; i < num_results; ++i) { + result.emplace_back(nearest[i].distance, external_label(nearest[i].id)); + } + // Candidate selection uses distance only. Preserve that selection, then match + // the former heap's (distance, external label) ordering within each tie group. + for (auto first = result.begin(); first != result.end();) { + auto last = first + 1; + while (last != result.end() && last->first == first->first) { + ++last; + } + std::sort(first, last); + first = last; + } + return result; +} + size_t HierarchicalNSW::remove(const PID* labels, size_t num_points) { if (data_level0_memory_ == nullptr || centroids_memory_ == nullptr || rotator_ == nullptr || num_clusters_ == 0) { diff --git a/src/index/hnsw_dispatch.cpp b/src/index/hnsw_dispatch.cpp index 10de3828..810f2faa 100644 --- a/src/index/hnsw_dispatch.cpp +++ b/src/index/hnsw_dispatch.cpp @@ -5,33 +5,68 @@ #include #include "../simd/dispatch_resolver.hpp" +#include "hnsw_search_impl.hpp" #include "rabitqlib/index/types.hpp" #include "rabitqlib/utils/cpu_features.hpp" namespace rabitqlib::hnsw::detail { -namespace { -using SearchKnnFn = - std::vector> (*)(HierarchicalNSW&, const float*, size_t, size_t); -} // namespace std::vector> search_knn_sorted( HierarchicalNSW& index, const float* rotated_query, size_t k, size_t ef_search ) { #if defined(__x86_64__) || defined(_M_X64) // The core variant uses AVX2 warmup; the popcount variant has its own stricter tier. - static const SearchKnnFn kSearchKnnFn = - cpu::has_avx512_popcnt() ? search_knn_sorted_avx512_popcnt + static const SearchLayersFn kSearchLayersFn = + cpu::has_avx512_popcnt() ? search_layers_avx512_popcnt : RABITQ_RESOLVE( - search_knn_sorted_avx512_core, - search_knn_sorted_avx2, - search_knn_sorted_generic, + search_layers_avx512_core, + search_layers_avx2, + search_layers_generic, cpu::has_avx512_core() && cpu::has_avx2() ); #else - static const SearchKnnFn kSearchKnnFn = simd::resolve_optional_kernel( - search_knn_sorted_neon, search_knn_sorted_generic, cpu::has_neon() + static const SearchLayersFn kSearchLayersFn = simd::resolve_optional_kernel( + search_layers_neon, search_layers_generic, cpu::has_neon() ); #endif - return kSearchKnnFn(index, rotated_query, k, ef_search); + return HnswSearchAccess::search(index, kSearchLayersFn, rotated_query, k, ef_search); } + +std::vector> search_knn_sorted_generic( + HierarchicalNSW& index, const float* rotated_query, size_t k, size_t ef_search +) { + return HnswSearchAccess::search( + index, search_layers_generic, rotated_query, k, ef_search + ); +} + +#if defined(__x86_64__) || defined(_M_X64) +std::vector> search_knn_sorted_avx2( + HierarchicalNSW& index, const float* rotated_query, size_t k, size_t ef_search +) { + return HnswSearchAccess::search(index, search_layers_avx2, rotated_query, k, ef_search); +} + +std::vector> search_knn_sorted_avx512_core( + HierarchicalNSW& index, const float* rotated_query, size_t k, size_t ef_search +) { + return HnswSearchAccess::search( + index, search_layers_avx512_core, rotated_query, k, ef_search + ); +} + +std::vector> search_knn_sorted_avx512_popcnt( + HierarchicalNSW& index, const float* rotated_query, size_t k, size_t ef_search +) { + return HnswSearchAccess::search( + index, search_layers_avx512_popcnt, rotated_query, k, ef_search + ); +} +#else +std::vector> search_knn_sorted_neon( + HierarchicalNSW& index, const float* rotated_query, size_t k, size_t ef_search +) { + return HnswSearchAccess::search(index, search_layers_neon, rotated_query, k, ef_search); +} +#endif } // namespace rabitqlib::hnsw::detail diff --git a/src/index/hnsw_search_avx2.cpp b/src/index/hnsw_search_avx2.cpp index 0bccdb29..24dbb9ca 100644 --- a/src/index/hnsw_search_avx2.cpp +++ b/src/index/hnsw_search_avx2.cpp @@ -1,7 +1,5 @@ #include #include -#include -#include #include "hnsw_search_avx2_kernels.hpp" #include "hnsw_search_impl.hpp" // IWYU pragma: keep (backend template definitions) @@ -10,6 +8,9 @@ namespace rabitqlib::hnsw::detail { +namespace { +// Internal linkage keeps every template instantiated with this kernel local to +// this ISA's translation unit. struct HnswAvx2Kernel { static inline float warmup_ip_x0_q_512( const uint64_t* data, @@ -28,11 +29,20 @@ struct HnswAvx2Kernel { return hnsw_mask_ip_x0_q_avx2(query, data, padded_dim); } }; +} // namespace -std::vector> search_knn_sorted_avx2( - HierarchicalNSW& index, const float* rotated_query, size_t k, size_t ef_search +size_t search_layers_avx2( + const HierarchicalNSW& index, + const SplitSingleQuery& query_wrapper, + const float* q_to_centroids, + VisitedSet& visited, + buffer::SearchBuffer& search_pool, + size_t k, + AnnCandidate* nearest ) { - return index.search_knn_sorted_direct(rotated_query, k, ef_search); + return HnswSearchAccess::search_layers( + index, query_wrapper, q_to_centroids, visited, search_pool, k, nearest + ); } } // namespace rabitqlib::hnsw::detail diff --git a/src/index/hnsw_search_avx512_core.cpp b/src/index/hnsw_search_avx512_core.cpp index 485f76f7..f9cc4ef0 100644 --- a/src/index/hnsw_search_avx512_core.cpp +++ b/src/index/hnsw_search_avx512_core.cpp @@ -1,7 +1,5 @@ #include #include -#include -#include #include "hnsw_search_avx2_kernels.hpp" #include "hnsw_search_avx512_kernels.hpp" @@ -11,6 +9,9 @@ namespace rabitqlib::hnsw::detail { +namespace { +// Internal linkage keeps every template instantiated with this kernel local to +// this ISA's translation unit. struct HnswAvx512CoreKernel { static inline float warmup_ip_x0_q_512( const uint64_t* data, @@ -29,12 +30,19 @@ struct HnswAvx512CoreKernel { return hnsw_mask_ip_x0_q_avx512(query, data, padded_dim); } }; +} // namespace -std::vector> search_knn_sorted_avx512_core( - HierarchicalNSW& index, const float* rotated_query, size_t k, size_t ef_search +size_t search_layers_avx512_core( + const HierarchicalNSW& index, + const SplitSingleQuery& query_wrapper, + const float* q_to_centroids, + VisitedSet& visited, + buffer::SearchBuffer& search_pool, + size_t k, + AnnCandidate* nearest ) { - return index.search_knn_sorted_direct( - rotated_query, k, ef_search + return HnswSearchAccess::search_layers( + index, query_wrapper, q_to_centroids, visited, search_pool, k, nearest ); } diff --git a/src/index/hnsw_search_avx512_popcnt.cpp b/src/index/hnsw_search_avx512_popcnt.cpp index 630a4e3b..19e77cbf 100644 --- a/src/index/hnsw_search_avx512_popcnt.cpp +++ b/src/index/hnsw_search_avx512_popcnt.cpp @@ -1,7 +1,5 @@ #include #include -#include -#include #include "hnsw_search_avx512_kernels.hpp" #include "hnsw_search_impl.hpp" // IWYU pragma: keep (backend template definitions) @@ -10,6 +8,9 @@ namespace rabitqlib::hnsw::detail { +namespace { +// Internal linkage keeps every template instantiated with this kernel local to +// this ISA's translation unit. struct HnswAvx512PopcntKernel { static inline float warmup_ip_x0_q_512( const uint64_t* data, @@ -28,12 +29,19 @@ struct HnswAvx512PopcntKernel { return hnsw_mask_ip_x0_q_avx512(query, data, padded_dim); } }; +} // namespace -std::vector> search_knn_sorted_avx512_popcnt( - HierarchicalNSW& index, const float* rotated_query, size_t k, size_t ef_search +size_t search_layers_avx512_popcnt( + const HierarchicalNSW& index, + const SplitSingleQuery& query_wrapper, + const float* q_to_centroids, + VisitedSet& visited, + buffer::SearchBuffer& search_pool, + size_t k, + AnnCandidate* nearest ) { - return index.search_knn_sorted_direct( - rotated_query, k, ef_search + return HnswSearchAccess::search_layers( + index, query_wrapper, q_to_centroids, visited, search_pool, k, nearest ); } diff --git a/src/index/hnsw_search_generic.cpp b/src/index/hnsw_search_generic.cpp index 9d7ef5f5..7d3760c2 100644 --- a/src/index/hnsw_search_generic.cpp +++ b/src/index/hnsw_search_generic.cpp @@ -1,7 +1,5 @@ #include #include -#include -#include #include "hnsw_search_impl.hpp" // IWYU pragma: keep (backend template definitions) #include "rabitqlib/core/detail/simd/space_dispatch.hpp" @@ -11,6 +9,9 @@ namespace rabitqlib::hnsw::detail { +namespace { +// Internal linkage keeps every template instantiated with this kernel local to +// this ISA's translation unit. struct HnswGenericKernel { static inline float warmup_ip_x0_q_512( const uint64_t* data, @@ -31,11 +32,20 @@ struct HnswGenericKernel { return simd::mask_ip_x0_q_generic(query, data, padded_dim); } }; +} // namespace -std::vector> search_knn_sorted_generic( - HierarchicalNSW& index, const float* rotated_query, size_t k, size_t ef_search +size_t search_layers_generic( + const HierarchicalNSW& index, + const SplitSingleQuery& query_wrapper, + const float* q_to_centroids, + VisitedSet& visited, + buffer::SearchBuffer& search_pool, + size_t k, + AnnCandidate* nearest ) { - return index.search_knn_sorted_direct(rotated_query, k, ef_search); + return HnswSearchAccess::search_layers( + index, query_wrapper, q_to_centroids, visited, search_pool, k, nearest + ); } } // namespace rabitqlib::hnsw::detail diff --git a/src/index/hnsw_search_impl.hpp b/src/index/hnsw_search_impl.hpp index a7f552d2..e271db6a 100644 --- a/src/index/hnsw_search_impl.hpp +++ b/src/index/hnsw_search_impl.hpp @@ -2,7 +2,6 @@ #include #include -#include #include #include #include @@ -11,17 +10,47 @@ #include "rabitqlib/core/metric.hpp" #include "rabitqlib/core/query.hpp" #include "rabitqlib/index/detail/buffer.hpp" -#include "rabitqlib/index/detail/visited_pool.hpp" +#include "rabitqlib/index/detail/hnsw_dispatch.hpp" +#include "rabitqlib/index/detail/visited_set.hpp" #include "rabitqlib/index/hnsw/hnsw.hpp" #include "rabitqlib/index/types.hpp" #include "rabitqlib/utils/memory.hpp" -#include "rabitqlib/utils/space.hpp" namespace rabitqlib::hnsw { +namespace detail { +// Grants the dispatch and per-ISA translation units access to the search members. +struct HnswSearchAccess { + static std::vector> search( + const HierarchicalNSW& index, + SearchLayersFn search_layers, + const float* rotated_query, + size_t k, + size_t ef_search + ) { + return index.search_knn_sorted_with(search_layers, rotated_query, k, ef_search); + } + + template + static size_t search_layers( + const HierarchicalNSW& index, + const SplitSingleQuery& query_wrapper, + const float* q_to_centroids, + VisitedSet& visited, + buffer::SearchBuffer& search_pool, + size_t k, + AnnCandidate* nearest + ) { + return index.search_layers_direct( + query_wrapper, q_to_centroids, visited, search_pool, k, nearest + ); + } +}; +} // namespace detail + template inline void HierarchicalNSW::estimate_bin_direct( - const std::vector& q_to_centroids, + const float* q_to_centroids, const SplitSingleQuery& query_wrapper, PID candidate_id, DistanceEstimate& estimate @@ -52,7 +81,7 @@ inline void HierarchicalNSW::estimate_bin_direct( template inline void HierarchicalNSW::estimate_full_direct( - const std::vector& q_to_centroids, + const float* q_to_centroids, const SplitSingleQuery& query_wrapper, PID candidate_id, DistanceEstimate& estimate @@ -88,49 +117,14 @@ inline void HierarchicalNSW::estimate_full_direct( } template -inline std::vector> HierarchicalNSW::search_knn_sorted_direct( - const float* rotated_query, size_t k, size_t ef_search +inline size_t HierarchicalNSW::search_layers_direct( + const SplitSingleQuery& query_wrapper, + const float* q_to_centroids, + VisitedSet& visited, + buffer::SearchBuffer& search_pool, + size_t k, + AnnCandidate* nearest ) const { - std::vector> result; - if (num_stored_points_ == 0) { - return result; - } - if (ef_search == 0) { - ef_search = kDefaultEfSearch; - } - - SplitSingleQuery query_wrapper( - rotated_query, padded_dim_, ex_bits_, query_config_, metric_type_ - ); - - // Preprocess - get the distance from query to all centroids - std::vector q_to_centroids(num_clusters_); - - if (metric_type_ == METRIC_L2) { - for (size_t i = 0; i < num_clusters_; ++i) { - q_to_centroids[i] = std::sqrt(raw_dist_func_( - rotated_query, - reinterpret_cast(centroids_memory_.get()) + (i * padded_dim_), - padded_dim_ - )); - } - } else if (metric_type_ == METRIC_IP) { - q_to_centroids.resize(2 * num_clusters_); - // First half stores dot(query, centroid); second half stores ||query - centroid||. - for (size_t i = 0; i < num_clusters_; ++i) { - q_to_centroids[i] = dot_product( - rotated_query, - reinterpret_cast(centroids_memory_.get()) + (i * padded_dim_), - padded_dim_ - ); - q_to_centroids[i + num_clusters_] = std::sqrt(euclidean_sqr( - rotated_query, - reinterpret_cast(centroids_memory_.get()) + (i * padded_dim_), - padded_dim_ - )); - } - } - PID current_id = entry_point_id_; DistanceEstimate current_estimate; @@ -164,46 +158,72 @@ inline std::vector> HierarchicalNSW::search_knn_sorted_dir } } + // Bounded priority queue implemented as a sorted vector. A local class has no + // linkage, so the vector code it instantiates is never shared between ISA builds. + class BoundedKNN { + public: + struct Entry { + PID id; + float distance; + }; + + explicit BoundedKNN(size_t capacity) : capacity_(capacity) {} + + // Insert a candidate in sorted order (ascending by distance), after equal ones. + void insert(PID id, float distance) { + auto it = std::upper_bound( + queue_.begin(), + queue_.end(), + distance, + [](float value, const Entry& entry) { return value < entry.distance; } + ); + queue_.insert(it, Entry{id, distance}); + // If we exceed capacity, drop the worst candidate (largest distance). + if (queue_.size() > capacity_) { + queue_.pop_back(); + } + } + + // Returns the worst (largest distance) candidate. + [[nodiscard]] const Entry& worst() const { return queue_.back(); } + + [[nodiscard]] size_t size() const { return queue_.size(); } + + [[nodiscard]] const std::vector& candidates() const { return queue_; } + + private: + size_t capacity_; + // Sorted in ascending order by distance so that the worst is at the back. + std::vector queue_; + }; + BoundedKNN nearest_candidates(k); search_base_layer_direct( current_id, - std::max(ef_search, k), k, query_wrapper, q_to_centroids, + visited, + search_pool, nearest_candidates ); - result.reserve(nearest_candidates.size()); + size_t count = 0; for (const auto& candidate : nearest_candidates.candidates()) { - result.emplace_back(candidate.distance, external_label(candidate.id)); - } - // Candidate selection uses distance only. Preserve that selection, then match - // the former heap's (distance, external label) ordering within each tie group. - for (auto first = result.begin(); first != result.end();) { - auto last = first + 1; - while (last != result.end() && last->first == first->first) { - ++last; - } - std::sort(first, last); - first = last; + nearest[count++] = AnnCandidate(candidate.id, candidate.distance); } - return result; + return count; } -template +template inline void HierarchicalNSW::search_base_layer_direct( PID entry_point_id, - size_t ef_search, size_t k, const SplitSingleQuery& query_wrapper, - const std::vector& q_to_centroids, - BoundedKNN& nearest_candidates + const float* q_to_centroids, + VisitedSet& visited, + buffer::SearchBuffer& search_pool, + Results& nearest_candidates ) const { - auto visited = visited_set_pool_->acquire(); - - // Keep the ef_search best estimates in a bounded sorted buffer. - buffer::SearchBuffer search_pool(ef_search); - float dist_k = 1e10F; DistanceEstimate start_estimate; @@ -220,12 +240,12 @@ inline void HierarchicalNSW::search_base_layer_direct( // Insert initial candidate. A removed point is still explored, never returned. if (!is_removed(entry_point_id)) { - nearest_candidates.insert(AnnCandidate(entry_point_id, estimated_distance)); + nearest_candidates.insert(entry_point_id, estimated_distance); dist_k = estimated_distance; } search_pool.insert(entry_point_id, estimated_distance); - visited->set(entry_point_id); + visited.set(entry_point_id); const size_t prefetch_size = (((padded_dim_ / 8) + 63) / 64) + 1; constexpr size_t kPrefetchLookahead = 4; // Number of neighbors to prefetch in advance. @@ -249,10 +269,10 @@ inline void HierarchicalNSW::search_base_layer_direct( ); } - if (visited->get(candidate_id)) { + if (visited.get(candidate_id)) { continue; } - visited->set(candidate_id); + visited.set(candidate_id); DistanceEstimate candidate_estimate; estimate_bin_direct( @@ -271,9 +291,7 @@ inline void HierarchicalNSW::search_base_layer_direct( q_to_centroids, query_wrapper, candidate_id, candidate_estimate ); } - nearest_candidates.insert( - AnnCandidate(candidate_id, candidate_estimate.distance) - ); + nearest_candidates.insert(candidate_id, candidate_estimate.distance); dist_k = nearest_candidates.worst().distance; } @@ -286,8 +304,6 @@ inline void HierarchicalNSW::search_base_layer_direct( ); } } - - visited_set_pool_->release(std::move(visited)); } } // namespace rabitqlib::hnsw diff --git a/src/index/hnsw_search_neon.cpp b/src/index/hnsw_search_neon.cpp index d613e074..b5220ef2 100644 --- a/src/index/hnsw_search_neon.cpp +++ b/src/index/hnsw_search_neon.cpp @@ -1,7 +1,5 @@ #include #include -#include -#include #include "../simd/hnsw_neon_kernels.hpp" #include "hnsw_search_impl.hpp" // IWYU pragma: keep (backend template definitions) @@ -10,6 +8,9 @@ namespace rabitqlib::hnsw::detail { +namespace { +// Internal linkage keeps every template instantiated with this kernel local to +// this ISA's translation unit. struct HnswNeonKernel { static inline float warmup_ip_x0_q_512( const uint64_t* data, @@ -32,11 +33,20 @@ struct HnswNeonKernel { ); } }; +} // namespace -std::vector> search_knn_sorted_neon( - HierarchicalNSW& index, const float* rotated_query, size_t k, size_t ef_search +size_t search_layers_neon( + const HierarchicalNSW& index, + const SplitSingleQuery& query_wrapper, + const float* q_to_centroids, + VisitedSet& visited, + buffer::SearchBuffer& search_pool, + size_t k, + AnnCandidate* nearest ) { - return index.search_knn_sorted_direct(rotated_query, k, ef_search); + return HnswSearchAccess::search_layers( + index, query_wrapper, q_to_centroids, visited, search_pool, k, nearest + ); } } // namespace rabitqlib::hnsw::detail From e23a2a2366fe94b0e28a6afb19d85d02bda9b8f3 Mon Sep 17 00:00:00 2001 From: gouyt13clear Date: Sat, 31 Oct 2026 14:42:08 +0800 Subject: [PATCH 3/4] ci: check that ISA builds share no differing weak functions scripts/check-isa-symbols.sh recompiles the library sources from compile_commands.json and fails when a weak function is defined under different ISA flag sets with different code. CI runs it on the portable Release builds of the Clang static-analysis job and the GCC installed-consumer job. docs/development.md describes the check, its Release-only scope and the HNSW structure, and AGENTS.md lists it for SIMD changes. --- .github/scripts/ci_changes.py | 3 + .github/scripts/test_ci_changes.py | 1 + .github/workflows/test.yaml | 4 + AGENTS.md | 2 +- docs/development.md | 36 ++++++- scripts/check-isa-symbols.sh | 160 +++++++++++++++++++++++++++++ 6 files changed, 201 insertions(+), 5 deletions(-) create mode 100755 scripts/check-isa-symbols.sh diff --git a/.github/scripts/ci_changes.py b/.github/scripts/ci_changes.py index 4c054a15..a3a3d9d6 100644 --- a/.github/scripts/ci_changes.py +++ b/.github/scripts/ci_changes.py @@ -234,6 +234,9 @@ def classify(paths): selected.add("python_quality") elif path == "scripts/check-tidy.sh": selected.add("cpp_tidy") + elif path == "scripts/check-isa-symbols.sh": + # Run by the static-analysis and installed-consumer jobs. + selected.update(("cpp", "cpp_tidy")) elif path != "scripts/check-includes.sh": selected.add("cpp_format") else: diff --git a/.github/scripts/test_ci_changes.py b/.github/scripts/test_ci_changes.py index df2008aa..be376042 100644 --- a/.github/scripts/test_ci_changes.py +++ b/.github/scripts/test_ci_changes.py @@ -222,6 +222,7 @@ def test_language_specific_changes(self): "scripts/check-tidy.sh": {"shell", "cpp_tidy", "python_quality"}, "scripts/check-format.sh": {"shell", "cpp_format", "python_quality"}, "scripts/check-includes.sh": {"shell"}, + "scripts/check-isa-symbols.sh": {"shell", "cpp", "cpp_tidy"}, ".clang-format": {"cpp_format"}, ".clang-tidy": {"cpp_tidy"}, ".github/workflows/test.yaml": { diff --git a/.github/workflows/test.yaml b/.github/workflows/test.yaml index 37c6a232..c18b00b1 100644 --- a/.github/workflows/test.yaml +++ b/.github/workflows/test.yaml @@ -57,6 +57,8 @@ jobs: -Dpybind11_DIR="$pybind11_DIR" - name: Run clang-tidy run: ./scripts/check-tidy.sh build-tidy + - name: Check ISA-specific symbols with Clang + run: ./scripts/check-isa-symbols.sh build-tidy shellcheck: needs: changes @@ -348,6 +350,8 @@ jobs: cmake --build build-install - name: Test portable Release build run: ctest --test-dir build-install --output-on-failure --no-tests=error + - name: Check ISA-specific symbols with GCC + run: ./scripts/check-isa-symbols.sh build-install - name: Download pinned Intel SDE run: | curl --fail --location --retry 3 --max-time 120 \ diff --git a/AGENTS.md b/AGENTS.md index db39008e..b8767b24 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -38,7 +38,7 @@ Tool versions and installation instructions are maintained in [docs/development. | Python sources | `./scripts/check-python.sh` and affected Python tests via `python -m pytest` | | Bindings or Python-visible C++ behavior | C++ and Python checks above; rebuild/install the package in the existing environment before testing, then verify the imported package/extension paths and that the extension includes the current changes | | Python API signatures, type declarations or accepted-input contracts | Keep `python_bindings/__init__.pyi` and runtime docstrings synchronized; run the [editor/type checks](tests/README.md#python-editor-support), including the explicitly selected mypy and Pyright tests outside `tests/python` | -| SIMD, metrics, packing, layouts, or index formats | Before building, audit every backend and consumer sharing the contract; build and run relevant reference/compatibility tests with `RABITQ_ENABLE_NATIVE_OPTIMIZATION=OFF` | +| SIMD, metrics, packing, layouts, or index formats | Before building, audit every backend and consumer sharing the contract; build and run relevant reference/compatibility tests with `RABITQ_ENABLE_NATIVE_OPTIMIZATION=OFF`; run `./scripts/check-isa-symbols.sh` on that build when ISA-flagged sources or headers they include change | | Hot-path algorithm/allocation changes or performance claims | Correctness tests first, then a representative benchmark reporting dataset, CPU, compiler, ISA, threads, latency/throughput, and quality; a correctness fix may omit benchmarks with a documented justification, without making an unmeasured performance claim | | Examples | Build or run the affected example | | Shell scripts | ShellCheck on affected scripts; see contributor instructions | diff --git a/docs/development.md b/docs/development.md index c5d6832a..797ff6c1 100644 --- a/docs/development.md +++ b/docs/development.md @@ -152,6 +152,26 @@ shellcheck scripts/*.sh sample/run_cpp.sh sample/run_python.sh This matches CI's script set. Also pass any changed shell scripts outside `scripts/` to `shellcheck` explicitly. +### ISA symbol check + +Sources compiled with ISA flags must not emit a weak function that a generic source also +emits with different code: the linker keeps one copy for every caller, so generic code +could execute AVX instructions. CI runs the check on portable Release builds with GCC and +Clang: + +```bash +./scripts/check-isa-symbols.sh build-tidy # any CMake build directory +``` + +It recompiles the library sources from `compile_commands.json` with that build's flags +(extra arguments such as `-O1` are appended) and fails when copies of a weak function +differ between ISA flag sets. Only the Release (`-O3`) configuration is clean and checked. +Lower optimization levels emit helpers the compiler otherwise inlines: Debug (`-O0`) +emits many standard-library and Eigen functions per source, and GCC at `-O1`/`-O2` +still shares Eigen assignment loops between the FastScan estimator sources and, at `-O1`, +the visited set's hash-table insertion between HNSW search sources. The script only sees +`src/`, needs GNU binutils, and does not cover MSVC or ARM-only builds. + ### Static analysis clang-tidy performs semantic checks and is kept separate from clang-format. @@ -753,8 +773,8 @@ index records. IVF/HNSW/SymphonyQG public index headers contain declarations and small accessors; they must not import encoding, scoring, rotator implementation, search scratch, Eigen -or hnswlib. Operations compile in `src/index/{ivf,hnsw,qg}.cpp`; HNSW's per-ISA templates -live in `src/index/hnsw_search_impl.hpp`. Preserve the advanced `hnsw_quant.hpp` +or hnswlib. Operations compile in `src/index/{ivf,hnsw,qg}.cpp`; HNSW's per-ISA graph traversal +templates live in `src/index/hnsw_search_impl.hpp`. Preserve the advanced `hnsw_quant.hpp` query-cache header. Installed-consumer tests poison implementation/vendor headers to check index header independence; `CoreModuleBoundary` also rejects batch declarations copied into core without an upward include. @@ -922,8 +942,16 @@ a short-vector improvement does not justify applying it to wider vectors. Put calculations and scratch-storage helpers outside the dispatcher. Shared implementation headers use internal linkage so independently compiled backends retain their own bodies. -This includes public headers reached from ISA-flagged sources, such as the per-ISA HNSW -search files: keep their non-template helpers `static inline` rather than plain `inline`. +This includes public headers reached from ISA-flagged sources: keep their non-template +helpers `static inline` rather than plain `inline`. Template code instantiated in an +ISA-flagged source must depend on a type local to that source, as the HNSW kernels do: +`src/index/hnsw_search_*.cpp` keep their `Kernel` types in an anonymous namespace and +only traverse the graph. Query preparation, centroid distances, acquiring the visited set +and search buffer, and result formatting run in generic code +(`HierarchicalNSW::search_knn_sorted_with`). Calls inside a `Kernel` template that do +not depend on `Kernel`, such as visited-set and search-buffer operations, stay safe only +while the compiler inlines them. `scripts/check-isa-symbols.sh` checks Release builds; see +[ISA symbol check](#isa-symbol-check). The AVX FHT intrinsics in `src/simd/fht_kernels.hpp` use the private kernel namespace for the same reason. Rotation uses this implementation on GCC, Clang, and MSVC; no assembler is required. Keep the FFHT attribution and MIT license in the diff --git a/scripts/check-isa-symbols.sh b/scripts/check-isa-symbols.sh new file mode 100755 index 00000000..c6061a6b --- /dev/null +++ b/scripts/check-isa-symbols.sh @@ -0,0 +1,160 @@ +#!/usr/bin/env bash + +# Fail when an inline or template function is emitted as a weak symbol by +# translation units compiled with different ISA flags and the copies differ. +# The linker keeps one copy for every caller, so a generic caller could run +# AVX code. Usage: check-isa-symbols.sh BUILD_DIR [EXTRA_COMPILE_FLAG...] +# BUILD_DIR needs compile_commands.json; extra flags (for example -O0) are +# appended to every compilation. Requires objdump and nm (GNU binutils). + +set -euo pipefail + +repo_root="$(git rev-parse --show-toplevel)" +readonly repo_root + +if (($# < 1)); then + echo "usage: $0 BUILD_DIR [EXTRA_COMPILE_FLAG...]" >&2 + exit 2 +fi +readonly build_dir="$1" +shift + +if [[ ! -f "$build_dir/compile_commands.json" ]]; then + echo "error: $build_dir/compile_commands.json was not found" >&2 + echo "configure CMake with CMAKE_EXPORT_COMPILE_COMMANDS=ON first" >&2 + exit 1 +fi + +for tool in nm objdump; do + if ! command -v "$tool" >/dev/null 2>&1; then + echo "error: $tool was not found" >&2 + exit 1 + fi +done + +object_dir="$(mktemp -d)" +if [[ -z "$object_dir" || ! -d "$object_dir" ]]; then + echo "error: could not create a temporary object directory" >&2 + exit 1 +fi +trap 'rm -rf -- "$object_dir"' EXIT + +"${PYTHON:-python3}" - "$build_dir/compile_commands.json" "$repo_root" "$object_dir" "$@" <<'PYCODE' +import collections +import concurrent.futures +import json +import os +import re +import shlex +import subprocess +import sys + +database, repo_root, object_dir, *extra_flags = sys.argv[1:] +source_pattern = re.compile(re.escape(repo_root) + r"/src/.+\.cpp$") +isa_flag = re.compile(r"^(-m(avx|fma|sse|popcnt|bmi|f16c|arch=)|/arch:|-march=)") + + +def compile_unit(index, entry): + arguments = entry.get("arguments") or shlex.split(entry["command"]) + arguments = list(arguments) + output = os.path.join(object_dir, f"{index}.o") + position = arguments.index("-o") + arguments[position + 1] = output + result = subprocess.run( + arguments + list(extra_flags), cwd=entry["directory"], capture_output=True, text=True + ) + if result.returncode != 0: + sys.exit(f"error: compiling {entry['file']} failed:\n{result.stderr}") + isa = " ".join(sorted(a for a in arguments if isa_flag.match(a))) or "generic" + return output, isa, os.path.relpath(entry["file"], repo_root) + + +entries = [] +for entry in json.load(open(database)): + path = os.path.abspath(os.path.join(entry["directory"], entry["file"])) + if source_pattern.match(path): + entries.append(dict(entry, file=path)) +if not entries: + sys.exit("error: no first-party library sources in the compilation database") + +with concurrent.futures.ThreadPoolExecutor(os.cpu_count()) as pool: + units = list(pool.map(lambda item: compile_unit(*item), enumerate(entries))) +if len({isa for _, isa, _ in units}) < 2: + sys.exit("error: the build has no ISA-specific translation units to compare") + + +def weak_functions(obj): + listing = subprocess.run( + ["nm", "--defined-only", obj], capture_output=True, text=True, check=True + ).stdout + return {line.split()[2] for line in listing.splitlines() if line.split()[1:2] == ["W"]} + + +def function_bodies(obj): + # Normalize addresses so copies compare by instructions, not placement. + text = subprocess.run( + ["objdump", "-d", "--no-show-raw-insn", "-r", obj], + capture_output=True, text=True, check=True, + ).stdout + bodies, name, lines = {}, None, [] + for line in text.splitlines(): + if line.startswith("Disassembly of section"): + if name is not None: + bodies[name] = "\n".join(lines) + name, lines = None, [] + continue + header = re.match(r"^[0-9a-f]+ <(.+)>:$", line) + if header: + if name is not None: + bodies[name] = "\n".join(lines) + name, lines = header.group(1), [] + continue + if name is None or not line.strip(): + continue + # Weak functions live in their own COMDAT sections, so branch targets are + # already section-relative. Keep immediates: copies that differ only in a + # constant are different code. Only placement-dependent names are normalized. + line = re.sub(r"^\s*[0-9a-f]+:\s*", "", line) + line = re.sub(r"<[^>]*>", "<>", line) + line = re.sub(r"\.L\w+", ".L", line) + line = re.sub(r"(\.(?:rodata|data|bss)[\w.]*)[+-]0x[0-9a-f]+", r"\1+#", line) + lines.append(line.strip()) + if name is not None: + bodies[name] = "\n".join(lines) + return bodies + + +definitions = collections.defaultdict(list) +for obj, isa, source in units: + for symbol in weak_functions(obj): + definitions[symbol].append((obj, isa, source)) + +shared = { + symbol: owners + for symbol, owners in definitions.items() + if len({isa for _, isa, _ in owners}) > 1 +} +needed = sorted({obj for owners in shared.values() for obj, _, _ in owners}) +with concurrent.futures.ThreadPoolExecutor(os.cpu_count()) as pool: + disassembly = dict(zip(needed, pool.map(function_bodies, needed))) + +failures = [] +for symbol, owners in sorted(shared.items()): + copies = {} + for obj, isa, source in owners: + copies.setdefault(isa, (disassembly[obj].get(symbol), source)) + if len({body for body, _ in copies.values()}) > 1: + demangled = subprocess.run( + ["c++filt", symbol], capture_output=True, text=True + ).stdout.strip() or symbol + failures.append((demangled, sorted((isa, source) for isa, (_, source) in copies.items()))) + +if failures: + print(f"error: {len(failures)} weak functions differ between ISA builds:", file=sys.stderr) + for symbol, owners in failures: + print(f" {symbol}", file=sys.stderr) + for isa, source in owners: + print(f" [{isa}] {source}", file=sys.stderr) + sys.exit(1) +print(f"ISA symbol check passed for {len(units)} translation units") +PYCODE From e7ac1416cb974f797e64e45235d8b1cd7dff8768 Mon Sep 17 00:00:00 2001 From: gouyt13clear Date: Sat, 31 Oct 2026 15:02:08 +0800 Subject: [PATCH 4/4] fix(hnsw): compile the visited set's spill path once GCC 13 does not inline the std::unordered_set insertion behind HashBasedVisitedSet::set, so each per-ISA HNSW search source emitted its own copy of the hash-table code, which the ISA symbol check reported in CI. Move the spill-over lookup and insertion into src/index/visited_set_hash.cpp; the direct-table fast path stays inline. The check now passes with GCC 11 through 14 and Clang 23, and at -O1/-O2 only two pre-existing Eigen loops in the FastScan estimators remain. --- CMakeLists.txt | 1 + docs/development.md | 12 ++++++------ include/rabitqlib/index/detail/visited_set_hash.hpp | 9 +++++++-- src/index/visited_set_hash.cpp | 13 +++++++++++++ 4 files changed, 27 insertions(+), 8 deletions(-) create mode 100644 src/index/visited_set_hash.cpp diff --git a/CMakeLists.txt b/CMakeLists.txt index 958f8432..ad0097c0 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -252,6 +252,7 @@ if(RABITQ_BUILD_INDEX) src/index/pipnn.cpp src/index/hnsw_dispatch.cpp src/index/hnsw_search_generic.cpp + src/index/visited_set_hash.cpp ${RABITQ_INDEX_ARCH_SOURCES} ) add_library(rabitqlib::index ALIAS rabitq_index) diff --git a/docs/development.md b/docs/development.md index 797ff6c1..36b0b170 100644 --- a/docs/development.md +++ b/docs/development.md @@ -168,9 +168,8 @@ It recompiles the library sources from `compile_commands.json` with that build's differ between ISA flag sets. Only the Release (`-O3`) configuration is clean and checked. Lower optimization levels emit helpers the compiler otherwise inlines: Debug (`-O0`) emits many standard-library and Eigen functions per source, and GCC at `-O1`/`-O2` -still shares Eigen assignment loops between the FastScan estimator sources and, at `-O1`, -the visited set's hash-table insertion between HNSW search sources. The script only sees -`src/`, needs GNU binutils, and does not cover MSVC or ARM-only builds. +still shares two Eigen assignment loops between the FastScan estimator sources. The +script only sees `src/`, needs GNU binutils, and does not cover MSVC or ARM-only builds. ### Static analysis @@ -948,9 +947,10 @@ ISA-flagged source must depend on a type local to that source, as the HNSW kerne `src/index/hnsw_search_*.cpp` keep their `Kernel` types in an anonymous namespace and only traverse the graph. Query preparation, centroid distances, acquiring the visited set and search buffer, and result formatting run in generic code -(`HierarchicalNSW::search_knn_sorted_with`). Calls inside a `Kernel` template that do -not depend on `Kernel`, such as visited-set and search-buffer operations, stay safe only -while the compiler inlines them. `scripts/check-isa-symbols.sh` checks Release builds; see +(`HierarchicalNSW::search_knn_sorted_with`). The visited set's hash-table spill path is +compiled once in `src/index/visited_set_hash.cpp`. Other calls inside a `Kernel` template +that do not depend on `Kernel`, such as search-buffer operations, stay safe only while +the compiler inlines them. `scripts/check-isa-symbols.sh` checks Release builds; see [ISA symbol check](#isa-symbol-check). The AVX FHT intrinsics in `src/simd/fht_kernels.hpp` use the private kernel namespace for the same reason. Rotation uses this implementation on GCC, Clang, and MSVC; diff --git a/include/rabitqlib/index/detail/visited_set_hash.hpp b/include/rabitqlib/index/detail/visited_set_hash.hpp index 1b4a678d..4db69e4c 100644 --- a/include/rabitqlib/index/detail/visited_set_hash.hpp +++ b/include/rabitqlib/index/detail/visited_set_hash.hpp @@ -40,6 +40,11 @@ class HashBasedVisitedSet { [[nodiscard]] auto hash1(const PID value) const { return value & mask_; } + // The spill-over lookups are compiled once in src/index/visited_set_hash.cpp, so + // ISA-specific search sources never instantiate the hash table themselves. + [[nodiscard]] bool spilled(PID internal_id) const; + void spill(PID internal_id); + void initialize_table(const size_t table_size) { // Allocate before changing the mask so failure preserves the previous table. table_ = std::vector>(table_size); @@ -94,7 +99,7 @@ class HashBasedVisitedSet { if (val == internal_id) { return true; } - return (val != kPidMax && stl_hash_.find(internal_id) != stl_hash_.end()); + return val != kPidMax && spilled(internal_id); } void set(PID internal_id) { @@ -105,7 +110,7 @@ class HashBasedVisitedSet { if (val == kPidMax) { val = internal_id; } else { - stl_hash_.emplace(internal_id); + spill(internal_id); } } }; diff --git a/src/index/visited_set_hash.cpp b/src/index/visited_set_hash.cpp new file mode 100644 index 00000000..da9d2dad --- /dev/null +++ b/src/index/visited_set_hash.cpp @@ -0,0 +1,13 @@ +#include "rabitqlib/index/detail/visited_set_hash.hpp" + +#include "rabitqlib/index/types.hpp" + +namespace rabitqlib { + +bool HashBasedVisitedSet::spilled(PID internal_id) const { + return stl_hash_.find(internal_id) != stl_hash_.end(); +} + +void HashBasedVisitedSet::spill(PID internal_id) { stl_hash_.emplace(internal_id); } + +} // namespace rabitqlib