From a74867cf56a73234ad5d957e40c35c9616d4d273 Mon Sep 17 00:00:00 2001 From: Michael Norris Date: Mon, 14 Sep 2026 13:13:11 -0700 Subject: [PATCH] faiss HNSW: delete the flag and the lock-based graph build (#5611) Summary: Pull Request resolved: https://github.com/facebookresearch/faiss/pull/5611 D112025877 added the `faiss::hnsw_deterministic_build` flag. The flag chose between the lock-based graph build and the deterministic graph build. This diff deletes the flag. The deterministic build is now the only build. Reviewed By: trang-nm-nguyen Differential Revision: D118342471 fbshipit-source-id: d3c5e5238aca2bcda463eb27959ec22827222c15 --- benchs/bench_hnsw.py | 41 ------- faiss/IndexBinaryHNSW.cpp | 166 ++----------------------- faiss/IndexBinaryHNSW.h | 6 - faiss/IndexHNSW.cpp | 193 +++--------------------------- faiss/IndexHNSW.h | 6 - faiss/impl/HNSW.cpp | 76 ------------ faiss/impl/HNSW.h | 19 +-- faiss/python/__init__.pyi | 1 - faiss/python/swigfaiss.swig | 2 - tests/bench_hnsw_deterministic.py | 1 - tests/test_graph_based.py | 30 +---- 11 files changed, 32 insertions(+), 509 deletions(-) diff --git a/benchs/bench_hnsw.py b/benchs/bench_hnsw.py index 5c425a09d..831d65c40 100644 --- a/benchs/bench_hnsw.py +++ b/benchs/bench_hnsw.py @@ -191,44 +191,3 @@ if "nsg" in todo: print("search_L", search_L, end=" ") index.nsg.search_L = search_L evaluate(index) - - -if "hnsw_locks" in todo: - - ntotal, _ = xb.shape - batch_size = ntotal // 100 - print( - f"Testing HNSW Flat: add with {batch_size=}, " - "with and without retaining locks" - ) - - # Unbatched - t0 = time.time() - index = faiss.IndexHNSWFlat(d, 32) - index.add(xb) - t1 = time.time() - print( - f"\t single bulk add(): {index.ntotal} added in {t1 - t0:6.3f}s" - f" = {index.ntotal / (t1 - t0):.0f}/s" - ) - - for retain_locks in [False, True]: - index = faiss.IndexHNSWFlat(d, 32) - index.retain_locks = retain_locks - - t0 = time.time() - t1 = None - t2 = None - for i in range(0, len(xb), batch_size): - t1 = time.time() - index.add(xb[i : i + batch_size]) - t2 = time.time() - if i > 2 and t2 - t0 > 2: - break - - assert t1 and t2 - dt = t2 - t0 - print( - f"\t {retain_locks=:1}: {index.ntotal} added in {t2 - t0:6.3f}s" - f" = {index.ntotal / (t2 - t0):.0f}/s" - ) diff --git a/faiss/IndexBinaryHNSW.cpp b/faiss/IndexBinaryHNSW.cpp index 7b050560e..2d19d21d2 100644 --- a/faiss/IndexBinaryHNSW.cpp +++ b/faiss/IndexBinaryHNSW.cpp @@ -7,7 +7,6 @@ #include -#include #include #include #include @@ -39,140 +38,6 @@ namespace faiss { -/************************************************************** - * add / search blocks of descriptors - **************************************************************/ - -namespace { - -void hnsw_add_vertices( - IndexBinaryHNSW& index_hnsw, - size_t n0, - size_t n, - const uint8_t* x, - bool verbose, - bool preset_levels = false) { - HNSW& hnsw = index_hnsw.hnsw; - size_t ntotal = n0 + n; - double t0 = getmillisecs(); - if (verbose) { - printf("hnsw_add_vertices: adding %zd elements on top of %zd " - "(preset_levels=%d)\n", - n, - n0, - int(preset_levels)); - } - - int max_level = hnsw.prepare_level_tab(n, preset_levels); - - if (verbose) { - printf(" max_level = %d\n", max_level); - } - - auto& locks = index_hnsw.locks; - locks.prepare(ntotal); - - // add vectors from highest to lowest level - std::vector hist; - std::vector order(n); - - { // make buckets with vectors of the same level - - // build histogram - for (size_t i = 0; i < n; i++) { - HNSW::storage_idx_t pt_id = - static_cast(i + n0); - int pt_level = hnsw.levels[pt_id] - 1; - while (pt_level >= static_cast(hist.size())) { - hist.push_back(0); - } - hist[pt_level]++; - } - - // accumulate - std::vector offsets(hist.size() + 1, 0); - for (size_t i = 0; i < hist.size() - 1; i++) { - offsets[i + 1] = offsets[i] + hist[i]; - } - - // bucket sort - for (size_t i = 0; i < n; i++) { - HNSW::storage_idx_t pt_id = - static_cast(i + n0); - int pt_level = hnsw.levels[pt_id] - 1; - order[offsets[pt_level]++] = pt_id; - } - } - - { // perform add - RandomGenerator rng2(789); - - size_t i1 = static_cast(n); - - for (int pt_level = static_cast(hist.size()) - 1; - pt_level >= int(!index_hnsw.init_level0); - pt_level--) { - size_t i0 = i1 - hist[pt_level]; - - if (verbose) { - printf("Adding %zu elements at level %d\n", i1 - i0, pt_level); - } - - // random permutation to get rid of dataset order bias - for (size_t j = i0; j < i1; j++) { - std::swap( - order[j], - order[j + rng2.rand_int(static_cast(i1 - j))]); - } - -#pragma omp parallel - { - std::unique_ptr vt = VisitedTable::create(ntotal); - - std::unique_ptr dis( - index_hnsw.get_distance_computer()); - bool do_display = verbose && omp_get_thread_num() == 0; - size_t prev_display = 0; - -#pragma omp for schedule(dynamic) - for (int64_t i = i0; i < i1; i++) { - HNSW::storage_idx_t pt_id = order[i]; - dis->set_query( - (float*)(x + (pt_id - n0) * index_hnsw.code_size)); - - hnsw.add_with_locks( - *dis, - pt_level, - pt_id, - locks, - *vt, - index_hnsw.keep_max_size_level0 && (pt_level == 0)); - - if (do_display && i - i0 > prev_display + 10000) { - prev_display = i - i0; - printf(" %zu / %zu\r", i - i0, i1 - i0); - fflush(stdout); - } - } - } - i1 = i0; - } - if (index_hnsw.init_level0) { - FAISS_ASSERT(i1 == 0); - } else { - FAISS_ASSERT((i1 - hist[0]) == 0); - } - } - if (verbose) { - printf("Done in %.3f ms\n", getmillisecs() - t0); - } - if (!index_hnsw.retain_locks) { - locks.clear(); - } -} - -} // anonymous namespace - /************************************************************** * IndexBinaryHNSW implementation **************************************************************/ @@ -272,28 +137,23 @@ void IndexBinaryHNSW::add(idx_t n, const uint8_t* x) { bool preset_levels = hnsw.levels.size() == static_cast(ntotal); - if (hnsw_deterministic_build) { - hnsw_add_vertices_deterministic( - hnsw, - n0, - n, - d, - init_level0, - keep_max_size_level0, - preset_levels, - verbose, - [this] { return get_distance_computer(); }, - [this, x, n0](DistanceComputer& dc, HNSW::storage_idx_t pt_id) { - dc.set_query((const float*)(x + (pt_id - n0) * code_size)); - }); - } else { - hnsw_add_vertices(*this, n0, n, x, verbose, preset_levels); - } + hnsw_add_vertices_deterministic( + hnsw, + n0, + n, + d, + init_level0, + keep_max_size_level0, + preset_levels, + verbose, + [this] { return get_distance_computer(); }, + [this, x, n0](DistanceComputer& dc, HNSW::storage_idx_t pt_id) { + dc.set_query((const float*)(x + (pt_id - n0) * code_size)); + }); } void IndexBinaryHNSW::reset() { hnsw.reset(); - locks.clear(); storage->reset(); ntotal = 0; } diff --git a/faiss/IndexBinaryHNSW.h b/faiss/IndexBinaryHNSW.h index 2e4049d3b..9bca05a31 100644 --- a/faiss/IndexBinaryHNSW.h +++ b/faiss/IndexBinaryHNSW.h @@ -11,7 +11,6 @@ #include #include -#include #include namespace faiss { @@ -41,11 +40,6 @@ struct IndexBinaryHNSW : IndexBinary { // used when GpuIndexBinaryCagra::copyFrom(IndexBinaryHNSW*) is called. bool keep_max_size_level0 = false; - // Per-node locks for HNSW graph construction. - LockVector locks; - // locks are freed after each call to add() unless this flag is set. - bool retain_locks = false; - explicit IndexBinaryHNSW(); explicit IndexBinaryHNSW(int d, int M = 32); explicit IndexBinaryHNSW(IndexBinary* storage, int M = 32); diff --git a/faiss/IndexHNSW.cpp b/faiss/IndexHNSW.cpp index 45708e6a2..1a9312cc3 100644 --- a/faiss/IndexHNSW.cpp +++ b/faiss/IndexHNSW.cpp @@ -9,7 +9,6 @@ #include -#include #include #include #include @@ -31,6 +30,7 @@ #include #include #include +#include #include #include #include @@ -42,8 +42,6 @@ using NodeDistFarther = HNSW::NodeDistFarther; HNSWStats hnsw_stats; -bool hnsw_deterministic_build = false; - /************************************************************** * add / search blocks of descriptors **************************************************************/ @@ -63,158 +61,6 @@ DistanceComputer* storage_distance_computer(const Index* storage) { return storage->get_distance_computer(); } -void hnsw_add_vertices( - IndexHNSW& index_hnsw, - size_t n0, - size_t n, - const float* x, - bool verbose, - bool preset_levels = false) { - size_t d = index_hnsw.d; - HNSW& hnsw = index_hnsw.hnsw; - size_t ntotal = n0 + n; - double t0 = getmillisecs(); - if (verbose) { - printf("hnsw_add_vertices: adding %zd elements on top of %zd " - "(preset_levels=%d)\n", - n, - n0, - int(preset_levels)); - } - - if (n == 0) { - return; - } - - int max_level = hnsw.prepare_level_tab(n, preset_levels); - - if (verbose) { - printf(" max_level = %d\n", max_level); - } - - auto& locks = index_hnsw.locks; - locks.prepare(ntotal); - - // add vectors from highest to lowest level - std::vector hist; - std::vector order(n); - - { // make buckets with vectors of the same level - - // build histogram - for (size_t i = 0; i < n; i++) { - storage_idx_t pt_id = static_cast(i + n0); - int pt_level = hnsw.levels[pt_id] - 1; - while (pt_level >= static_cast(hist.size())) { - hist.push_back(0); - } - hist[pt_level]++; - } - - // accumulate - std::vector offsets(hist.size() + 1, 0); - for (size_t i = 0; i < hist.size() - 1; i++) { - offsets[i + 1] = offsets[i] + hist[i]; - } - - // bucket sort - for (size_t i = 0; i < n; i++) { - storage_idx_t pt_id = static_cast(i + n0); - int pt_level = hnsw.levels[pt_id] - 1; - order[offsets[pt_level]++] = pt_id; - } - } - - idx_t check_period = InterruptCallback::get_period_hint( - max_level * index_hnsw.d * hnsw.efConstruction); - - { // perform add - RandomGenerator rng2(789); - - size_t i1 = static_cast(n); - - for (int pt_level = static_cast(hist.size()) - 1; - pt_level >= int(!index_hnsw.init_level0); - pt_level--) { - size_t i0 = i1 - hist[pt_level]; - - if (verbose) { - printf("Adding %zu elements at level %d\n", i1 - i0, pt_level); - } - - // random permutation to get rid of dataset order bias - for (size_t j = i0; j < i1; j++) { - std::swap( - order[j], - order[j + rng2.rand_int(static_cast(i1 - j))]); - } - - bool interrupt = false; - -#pragma omp parallel if (i1 > i0 + 100) - { - std::unique_ptr vt = - VisitedTable::create(ntotal, hnsw.use_visited_hashset); - - std::unique_ptr dis( - storage_distance_computer(index_hnsw.storage)); - bool do_display = verbose && omp_get_thread_num() == 0; - size_t prev_display = 0; - size_t counter = 0; - - // here we should do schedule(dynamic) but this segfaults for - // some versions of LLVM. The performance impact should not be - // too large when (i1 - i0) / num_threads >> 1 -#pragma omp for schedule(static) - for (int64_t i = i0; i < i1; i++) { - storage_idx_t pt_id = order[i]; - dis->set_query(x + (pt_id - n0) * d); - - // cannot break - if (interrupt) { - continue; - } - - hnsw.add_with_locks( - *dis, - pt_level, - pt_id, - locks, - *vt, - index_hnsw.keep_max_size_level0 && (pt_level == 0)); - - if (do_display && i - i0 > prev_display + 10000) { - prev_display = i - i0; - printf(" %zu / %zu\r", i - i0, i1 - i0); - fflush(stdout); - } - if (counter % check_period == 0) { - if (InterruptCallback::is_interrupted()) { - interrupt = true; - } - } - counter++; - } - } - if (interrupt) { - FAISS_THROW_MSG("computation interrupted"); - } - i1 = i0; - } - if (index_hnsw.init_level0) { - FAISS_ASSERT(i1 == 0); - } else { - FAISS_ASSERT((i1 - hist[0]) == 0); - } - } - if (verbose) { - printf("Done in %.3f ms\n", getmillisecs() - t0); - } - if (!index_hnsw.retain_locks) { - locks.clear(); - } -} - } // namespace // Deterministic HNSW graph build inspired by ParlayANN: points are bucketed by @@ -708,28 +554,23 @@ void IndexHNSW::add(idx_t n, const float* x) { bool preset_levels = hnsw.levels.size() == static_cast(ntotal); - if (hnsw_deterministic_build) { - hnsw_add_vertices_deterministic( - hnsw, - n0, - n, - d, - init_level0, - keep_max_size_level0, - preset_levels, - verbose, - [this] { return storage_distance_computer(storage); }, - [this, x, n0](DistanceComputer& dc, HNSW::storage_idx_t pt_id) { - dc.set_query(x + (pt_id - n0) * d); - }); - } else { - hnsw_add_vertices(*this, n0, n, x, verbose, preset_levels); - } + hnsw_add_vertices_deterministic( + hnsw, + n0, + n, + d, + init_level0, + keep_max_size_level0, + preset_levels, + verbose, + [this] { return storage_distance_computer(storage); }, + [this, x, n0](DistanceComputer& dc, HNSW::storage_idx_t pt_id) { + dc.set_query(x + (pt_id - n0) * d); + }); } void IndexHNSW::reset() { hnsw.reset(); - locks.clear(); storage->reset(); ntotal = 0; } @@ -920,7 +761,7 @@ void IndexHNSW::init_level_0_from_entry_points( int n, const storage_idx_t* points, const storage_idx_t* nearests) { - locks.prepare(ntotal); + LockVector locks(ntotal); #pragma omp parallel { @@ -950,10 +791,6 @@ void IndexHNSW::init_level_0_from_entry_points( if (verbose) { printf("\n"); } - - if (!retain_locks) { - locks.clear(); - } } void IndexHNSW::reorder_links() { diff --git a/faiss/IndexHNSW.h b/faiss/IndexHNSW.h index 42d2406e2..ab13715b7 100644 --- a/faiss/IndexHNSW.h +++ b/faiss/IndexHNSW.h @@ -18,7 +18,6 @@ #include #include #include -#include #include namespace faiss { @@ -53,11 +52,6 @@ struct IndexHNSW : Index { // See impl/VisitedTable.h. std::optional use_visited_hashset; - // Per-node locks for HNSW graph construction. - LockVector locks; - // locks are freed after each call to add() unless this flag is set. - bool retain_locks = false; - explicit IndexHNSW(int d = 0, int M = 32, MetricType metric = METRIC_L2); explicit IndexHNSW(Index* storage, int M = 32); diff --git a/faiss/impl/HNSW.cpp b/faiss/impl/HNSW.cpp index da6ea14bc..136818648 100644 --- a/faiss/impl/HNSW.cpp +++ b/faiss/impl/HNSW.cpp @@ -969,84 +969,8 @@ void merge_reverse_links_impl( } } -template -void add_with_locks_impl( - HNSW& hnsw, - DistanceComputer& ptdis, - int pt_level, - int pt_id, - LockVector& locks, - VisitedTable& vt, - bool keep_max_size_level0) { - storage_idx_t nearest = hnsw.entry_point; - if (nearest == -1) { // avoid locking after the first point. -#pragma omp critical - if (hnsw.entry_point == -1) { // double-check under lock. - hnsw.max_level = pt_level; - hnsw.entry_point = pt_id; - // leave nearest = -1 to trigger early exit after critical block. - } else { - // else: Another thread set the entry point. - nearest = hnsw.entry_point; - } - } - - if (nearest < 0) { - return; - } - - locks.lock(pt_id); - - int level = hnsw.max_level; // level at which we start adding neighbors - float d_nearest = ptdis(nearest); - - // greedy search on upper levels - for (; level > pt_level; level--) { - greedy_update_nearest_impl(hnsw, ptdis, level, nearest, d_nearest); - } - - for (; level >= 0; level--) { - add_links_starting_from_impl( - hnsw, - ptdis, - pt_id, - nearest, - d_nearest, - level, - locks, - vt, - keep_max_size_level0); - } - - locks.unlock(pt_id); - -#pragma omp critical - { - if (pt_level > hnsw.max_level) { - hnsw.max_level = pt_level; - hnsw.entry_point = pt_id; - } - } -} - } // namespace -void HNSW::add_with_locks( - DistanceComputer& ptdis, - int pt_level, - int pt_id, - LockVector& locks, - VisitedTable& vt, - bool keep_max_size_level0) { - if (is_similarity) { - add_with_locks_impl( - *this, ptdis, pt_level, pt_id, locks, vt, keep_max_size_level0); - } else { - add_with_locks_impl( - *this, ptdis, pt_level, pt_id, locks, vt, keep_max_size_level0); - } -} - void HNSW::compute_forward_links_deterministic( DistanceComputer& ptdis, int pt_level, diff --git a/faiss/impl/HNSW.h b/faiss/impl/HNSW.h index dc442c802..39acb6525 100644 --- a/faiss/impl/HNSW.h +++ b/faiss/impl/HNSW.h @@ -212,16 +212,6 @@ struct HNSW { VisitedTable& vt, bool keep_max_size_level0 = false); - /** add point pt_id on all levels <= pt_level and build the link - * structure for them. */ - void add_with_locks( - DistanceComputer& ptdis, - int pt_level, - int pt_id, - LockVector& locks, - VisitedTable& vt, - bool keep_max_size_level0 = false); - /** Deterministic build, phase A: write pt_id's forward links against an * immutable snapshot, touching only pt_id's own slots. Reciprocal edges * are collected in `pt_reverse_edges` for phase B, not applied. Requires @@ -288,7 +278,8 @@ struct HNSW { void permute_entries(const idx_t* map); }; -/** Deterministic, lock-free HNSW graph build, shared by IndexHNSW and +/** Deterministic, lock-free HNSW graph build. This is the only graph build + * that add() uses, and it is shared by IndexHNSW and * IndexBinaryHNSW. The callbacks let both share the algorithm: * `make_distance_computer()` returns a fresh DistanceComputer per thread * (caller-owned) and `set_query(dc, pt_id)` points it at pt_id's vector. */ @@ -329,12 +320,6 @@ struct HNSWStats { // global var that collects them all FAISS_API extern HNSWStats hnsw_stats; -/** Use the lock-free deterministic graph build in add() rather than the - * lock-based one. Transitional: it will become the only path. - * - * NOT thread-safe: set before calling add(). Sampled once per add(). */ -FAISS_API extern bool hnsw_deterministic_build; - /// Internal HNSW algorithm helpers. These are not part of the public API; they /// are exposed here only so that unit tests (and a few cross-TU callers such as /// the Panorama search variant) can reach them. diff --git a/faiss/python/__init__.pyi b/faiss/python/__init__.pyi index bc1e24ed9..152349526 100644 --- a/faiss/python/__init__.pyi +++ b/faiss/python/__init__.pyi @@ -4384,7 +4384,6 @@ class _SwigGlobals: distance_compute_blas_database_bs: int distance_compute_min_k_reservoir: int index_factory_verbose: int - hnsw_deterministic_build: bool cvar: _SwigGlobals diff --git a/faiss/python/swigfaiss.swig b/faiss/python/swigfaiss.swig index b1eb71602..d3d18e2f1 100644 --- a/faiss/python/swigfaiss.swig +++ b/faiss/python/swigfaiss.swig @@ -612,8 +612,6 @@ void gpu_sync_all_devices() %include %include %include -%ignore faiss::IndexHNSW::locks; -%ignore faiss::IndexBinaryHNSW::locks; %include %include diff --git a/tests/bench_hnsw_deterministic.py b/tests/bench_hnsw_deterministic.py index 724789469..69c71f7f3 100644 --- a/tests/bench_hnsw_deterministic.py +++ b/tests/bench_hnsw_deterministic.py @@ -251,7 +251,6 @@ def main() -> None: # thread count. faiss.omp_set_num_threads(faiss.omp_get_max_threads()) threads = faiss.omp_get_max_threads() - faiss.cvar.hnsw_deterministic_build = True print("faiss HNSW deterministic-build benchmark") rss = RSSPeak() diff --git a/tests/test_graph_based.py b/tests/test_graph_based.py index dabf46b77..f314aa41a 100644 --- a/tests/test_graph_based.py +++ b/tests/test_graph_based.py @@ -49,17 +49,9 @@ class TestHNSW(unittest.TestCase): nb = 20000 xb = np.random.RandomState(123).random((nb, d)).astype("float32") - # build() mutates the process-global OpenMP thread count and the - # deterministic-build flag; restore both so they do not leak into - # subsequent tests. + # build() mutates the process-global OpenMP thread count; restore it so + # it does not leak into subsequent tests. self.addCleanup(faiss.omp_set_num_threads, faiss.omp_get_max_threads()) - self.addCleanup( - setattr, - faiss.cvar, - "hnsw_deterministic_build", - faiss.cvar.hnsw_deterministic_build, - ) - faiss.cvar.hnsw_deterministic_build = True def build(nthreads): faiss.omp_set_num_threads(nthreads) @@ -97,24 +89,6 @@ class TestHNSW(unittest.TestCase): seg = g8a[int(offs[i]) : int(offs[i + 1])] self.assertNotIn(i, seg[seg >= 0].tolist()) - def test_deterministic_build_recall(self): - # Same recall bar as test_hnsw (which exercises the lock-based default), - # so the deterministic graph is not merely reproducible but as good. - self.addCleanup( - setattr, - faiss.cvar, - "hnsw_deterministic_build", - faiss.cvar.hnsw_deterministic_build, - ) - faiss.cvar.hnsw_deterministic_build = True - - index = faiss.IndexHNSWFlat(self.xq.shape[1], 16) - index.add(self.xb) - Dhnsw, Ihnsw = index.search(self.xq, 1) - - self.assertGreaterEqual((self.Iref == Ihnsw).sum(), 460) - self.io_and_retest(index, Dhnsw, Ihnsw) - def test_range_search(self): index_flat = faiss.IndexFlat(self.xb.shape[1]) index_flat.add(self.xb)