Skip to content

Commit d5d19f3

Browse files
committed
Bound CAGRA hash table sizing loops so they cannot spin forever
calc_hashmap_params() sizes its hash tables by growing a bit length until the requested table fits, checking the supported maximum only after the loop. hashmap::get_size() is `1U << bitlen`, which is undefined once bitlen reaches 32 and in practice wraps back to a small value, so a request needing more than 2^31 entries left the loop condition permanently true. The bit length grew without bound, the post-loop RAFT_EXPECTS was never reached, and the search hung on the host instead of returning an error. Bound each of the four sizing loops by the same maximum its post-loop check already uses, so an oversized request exits the loop and raises the existing error. The limits are hoisted into local constants so the loop bound and the check cannot drift apart. check_params() only caps itopk_size at 1024 for SINGLE_CTA, so MULTI_CTA and MULTI_KERNEL were the reachable paths. Both are covered by a new regression test; note that a regression there resurfaces as a test timeout rather than a failed assertion. Verified on an RTX PRO 6000: itopk_size 1,100,000,000 previously hung indefinitely and now raises the hash_bitlen error in a few seconds, while valid searches are unaffected. A standalone sweep of the old and new loop logic over all four loops agrees on every in-range input. Closes #2523 Signed-off-by: Shaunak Kapur <shaunakk@nvidia.com>
1 parent 329de99 commit d5d19f3

3 files changed

Lines changed: 159 additions & 12 deletions

File tree

cpp/src/neighbors/detail/cagra/search_plan.cuh

Lines changed: 23 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* SPDX-FileCopyrightText: Copyright (c) 2023-2026, NVIDIA CORPORATION.
2+
* SPDX-FileCopyrightText: Copyright (c) 2023-2026, NVIDIA CORPORATION & AFFILIATES. All rights reserved.
33
* SPDX-License-Identifier: Apache-2.0
44
*/
55

@@ -265,12 +265,18 @@ struct search_plan_impl : public search_plan_impl_base {
265265
// table that each CTA has in the shared memory. This hash table is not
266266
// shared among CTAs. This hash table is reset and restored in each iteration.
267267
//
268-
const uint32_t max_visited_nodes = mc_itopk_size + (graph_degree * 2);
269-
small_hash_bitlen = 8; // 256
270-
while (max_visited_nodes > hashmap::get_size(small_hash_bitlen) * max_fill_rate) {
268+
const uint32_t max_visited_nodes = mc_itopk_size + (graph_degree * 2);
269+
constexpr size_t max_small_hash_bitlen = 14; // 16K
270+
small_hash_bitlen = 8; // 256
271+
// Stop at the supported maximum rather than growing without bound: hashmap::get_size()
272+
// is 1U << bitlen, which is undefined (and wraps to a small value) once bitlen reaches
273+
// 32, so an unbounded loop would never satisfy its exit condition again.
274+
while (small_hash_bitlen <= max_small_hash_bitlen &&
275+
max_visited_nodes > hashmap::get_size(small_hash_bitlen) * max_fill_rate) {
271276
small_hash_bitlen += 1;
272277
}
273-
RAFT_EXPECTS(small_hash_bitlen <= 14, "small_hash_bitlen cannot be largen than 14 (16K)");
278+
RAFT_EXPECTS(small_hash_bitlen <= max_small_hash_bitlen,
279+
"small_hash_bitlen cannot be largen than 14 (16K)");
274280
//
275281
// [traversed_hash_table]
276282
// Whether a node has ever been used as the starting point for a traversal
@@ -279,13 +285,15 @@ struct search_plan_impl : public search_plan_impl_base {
279285
//
280286
const auto max_traversed_nodes =
281287
mc_num_cta_per_query * max((size_t)mc_itopk_size, max_iterations);
282-
unsigned min_bitlen = 11; // 2K
288+
unsigned min_bitlen = 11; // 2K
289+
constexpr int64_t max_hash_bitlen_mc = 25; // 32M
283290
if (min_bitlen < hashmap_min_bitlen) { min_bitlen = hashmap_min_bitlen; }
284291
hash_bitlen = min_bitlen;
285-
while (max_traversed_nodes > hashmap::get_size(hash_bitlen) * max_fill_rate) {
292+
while (hash_bitlen <= max_hash_bitlen_mc &&
293+
max_traversed_nodes > hashmap::get_size(hash_bitlen) * max_fill_rate) {
286294
hash_bitlen += 1;
287295
}
288-
RAFT_EXPECTS(hash_bitlen <= 25, "hash_bitlen cannot be largen than 25 (32M)");
296+
RAFT_EXPECTS(hash_bitlen <= max_hash_bitlen_mc, "hash_bitlen cannot be largen than 25 (32M)");
289297
} else {
290298
while (hashmap_mode == hash_mode::AUTO || hashmap_mode == hash_mode::SMALL) {
291299
//
@@ -300,7 +308,8 @@ struct search_plan_impl : public search_plan_impl_base {
300308
unsigned max_bitlen = 13; // 8K
301309
if (min_bitlen < hashmap_min_bitlen) { min_bitlen = hashmap_min_bitlen; }
302310
hash_bitlen = min_bitlen;
303-
while (max_visited_nodes > hashmap::get_size(hash_bitlen) * max_fill_rate) {
311+
while (hash_bitlen <= max_bitlen &&
312+
max_visited_nodes > hashmap::get_size(hash_bitlen) * max_fill_rate) {
304313
hash_bitlen += 1;
305314
}
306315
if (hash_bitlen > max_bitlen) {
@@ -337,13 +346,15 @@ struct search_plan_impl : public search_plan_impl_base {
337346
// maximum fill rate of the hash table.
338347
//
339348
uint32_t max_visited_nodes = itopk_size + (search_width * graph_degree * max_iterations);
340-
unsigned min_bitlen = 11; // 2K
349+
unsigned min_bitlen = 11; // 2K
350+
constexpr int64_t max_hash_bitlen = 20; // 1M
341351
if (min_bitlen < hashmap_min_bitlen) { min_bitlen = hashmap_min_bitlen; }
342352
hash_bitlen = min_bitlen;
343-
while (max_visited_nodes > hashmap::get_size(hash_bitlen) * max_fill_rate) {
353+
while (hash_bitlen <= max_hash_bitlen &&
354+
max_visited_nodes > hashmap::get_size(hash_bitlen) * max_fill_rate) {
344355
hash_bitlen += 1;
345356
}
346-
RAFT_EXPECTS(hash_bitlen <= 20,
357+
RAFT_EXPECTS(hash_bitlen <= max_hash_bitlen,
347358
"hash_bitlen cannot be largen than 20 (1M). You can decrease itopk_size, "
348359
"search_width or max_iterations to reduce the required hashmap size.");
349360
}

cpp/tests/CMakeLists.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -211,6 +211,7 @@ ConfigureTest(
211211
neighbors/ann_cagra/bug_graph_smaller_than_dataset.cu
212212
neighbors/ann_cagra/bug_iterative_cagra_build.cu
213213
neighbors/ann_cagra/bug_issue_93_reproducer.cu
214+
neighbors/ann_cagra/bug_issue_2523_hashmap_bitlen.cu
214215
GPUS 1
215216
PERCENT 100
216217
)
Lines changed: 135 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,135 @@
1+
/*
2+
* SPDX-FileCopyrightText: Copyright (c) 2026, NVIDIA CORPORATION & AFFILIATES. All rights reserved.
3+
* SPDX-License-Identifier: Apache-2.0
4+
*/
5+
6+
#include <gtest/gtest.h>
7+
8+
#include "../cagra_padded_build_helpers.cuh"
9+
#include <cuvs/neighbors/cagra.hpp>
10+
11+
#include <raft/core/device_mdarray.hpp>
12+
#include <raft/core/device_resources.hpp>
13+
#include <raft/core/error.hpp>
14+
#include <raft/random/rng.cuh>
15+
16+
#include <cstdint>
17+
#include <utility>
18+
19+
namespace cuvs::neighbors::cagra {
20+
21+
/**
22+
* @brief Regression test for issue #2523: hash table sizing must not loop forever.
23+
*
24+
* The hash tables in search_plan_impl::calc_hashmap_params() are sized by loops that grow
25+
* a bit length until the requested table fits, with the supported maximum checked only
26+
* after the loop. hashmap::get_size() is `1U << bitlen`, which is undefined once bitlen
27+
* reaches 32 and in practice wraps back to a small value, so a request that needs a table
28+
* larger than 2^31 entries made the loop condition permanently true. The bit length grew
29+
* without bound, the post-loop RAFT_EXPECTS was never reached, and the search hung on the
30+
* host instead of failing.
31+
*
32+
* check_params() only caps itopk_size at 1024 for SINGLE_CTA, so MULTI_CTA and MULTI_KERNEL
33+
* are the two algorithms that can reach the sizing loops with an oversized request.
34+
*
35+
* These searches must now raise the existing "hash_bitlen cannot be larger than ..." error
36+
* rather than hanging. A regression reappears as a test timeout, not a failed assertion.
37+
*/
38+
class cagra_hashmap_bitlen_no_hang_test : public ::testing::Test {
39+
public:
40+
using data_type = float;
41+
using index_type = uint32_t;
42+
43+
protected:
44+
// Large enough that the traversed-node hash table would need more than 2^31 entries,
45+
// which is what previously drove the sizing loop past the wrap point.
46+
constexpr static size_t oversized_itopk = 1'100'000'000;
47+
constexpr static size_t valid_itopk = 64;
48+
49+
constexpr static int64_t n_dataset = 1000;
50+
constexpr static int64_t n_dim = 32;
51+
constexpr static int64_t n_queries = 4;
52+
constexpr static int64_t k = 10;
53+
54+
void SetUp() override
55+
{
56+
dataset.emplace(raft::make_device_matrix<data_type, int64_t>(res, n_dataset, n_dim));
57+
queries.emplace(raft::make_device_matrix<data_type, int64_t>(res, n_queries, n_dim));
58+
neighbors.emplace(raft::make_device_matrix<index_type, int64_t>(res, n_queries, k));
59+
distances.emplace(raft::make_device_matrix<data_type, int64_t>(res, n_queries, k));
60+
61+
raft::random::RngState r(1234ULL);
62+
raft::random::uniform(
63+
res, r, dataset->data_handle(), n_dataset * n_dim, data_type(-1), data_type(1));
64+
raft::random::uniform(
65+
res, r, queries->data_handle(), n_queries * n_dim, data_type(-1), data_type(1));
66+
67+
cagra::index_params index_params;
68+
index_params.graph_degree = 32;
69+
index_params.intermediate_graph_degree = 64;
70+
71+
padded_.emplace(res, raft::make_const_mdspan(dataset->view()));
72+
index_.emplace(cagra::build(res, index_params, padded_->view));
73+
raft::resource::sync_stream(res);
74+
}
75+
76+
void TearDown() override
77+
{
78+
index_.reset();
79+
padded_.reset();
80+
dataset.reset();
81+
queries.reset();
82+
neighbors.reset();
83+
distances.reset();
84+
raft::resource::sync_stream(res);
85+
}
86+
87+
void search_with(cagra::search_algo algo, size_t itopk_size, uint32_t max_iterations)
88+
{
89+
cagra::search_params search_params;
90+
search_params.algo = algo;
91+
search_params.itopk_size = itopk_size;
92+
search_params.search_width = 8;
93+
search_params.max_iterations = max_iterations;
94+
95+
cagra::search(res,
96+
search_params,
97+
*index_,
98+
raft::make_const_mdspan(queries->view()),
99+
neighbors->view(),
100+
distances->view());
101+
raft::resource::sync_stream(res);
102+
}
103+
104+
raft::resources res;
105+
std::optional<cuvs::neighbors::test::padded_device_matrix_for_cagra<data_type>> padded_{};
106+
std::optional<cagra::index<data_type, index_type>> index_ = std::nullopt;
107+
std::optional<raft::device_matrix<data_type, int64_t>> dataset = std::nullopt;
108+
std::optional<raft::device_matrix<data_type, int64_t>> queries = std::nullopt;
109+
std::optional<raft::device_matrix<index_type, int64_t>> neighbors = std::nullopt;
110+
std::optional<raft::device_matrix<data_type, int64_t>> distances = std::nullopt;
111+
};
112+
113+
// MULTI_CTA sizes the shared traversed-node table from itopk_size and search_width.
114+
// This is the path confirmed to hang before the fix.
115+
TEST_F(cagra_hashmap_bitlen_no_hang_test, MultiCtaOversizedItopkThrows)
116+
{
117+
EXPECT_THROW(search_with(cagra::search_algo::MULTI_CTA, oversized_itopk, 0), raft::exception);
118+
}
119+
120+
// MULTI_KERNEL reaches the small-hash loop first and then the normal-hash loop.
121+
// max_iterations is pinned so the sizing inputs do not depend on the auto-derived value.
122+
TEST_F(cagra_hashmap_bitlen_no_hang_test, MultiKernelOversizedItopkThrows)
123+
{
124+
EXPECT_THROW(search_with(cagra::search_algo::MULTI_KERNEL, oversized_itopk, 32), raft::exception);
125+
}
126+
127+
// Guards against the bound rejecting requests that were previously accepted.
128+
TEST_F(cagra_hashmap_bitlen_no_hang_test, ValidItopkStillSucceeds)
129+
{
130+
EXPECT_NO_THROW(search_with(cagra::search_algo::MULTI_CTA, valid_itopk, 0));
131+
EXPECT_NO_THROW(search_with(cagra::search_algo::MULTI_KERNEL, valid_itopk, 0));
132+
EXPECT_NO_THROW(search_with(cagra::search_algo::SINGLE_CTA, valid_itopk, 0));
133+
}
134+
135+
} // namespace cuvs::neighbors::cagra

0 commit comments

Comments
 (0)