Skip to content

Commit 615a20e

Browse files
Michael Norrisfacebook-github-bot
authored andcommitted
faiss HNSW: make graph construction deterministic by default (remove lock-based build) (#5486)
Summary: TLDR: makes the deterministic HNSW graph build the default (and only) float build path, and removes the legacy lock-based one. Inspired by ParlayANN. The deterministic build is reproducible AND faster than the lock-based build at every scale and thread count we measured. -- similarities to parlayANN: - add vertices in doubling batches against frozen snapshot - defer adding reciprocal edges immediately, add them later after parallel phase differences from ParlayANN: - original ParlanANN targets flat graphs like Vamana - re-uses Faiss HNSW pruning in `shrink_neighbor_list` --- AI (with a bunch of edits) explanation in more detail: -- What changed - `IndexHNSW::add` now always uses the deterministic, lock-free build. The lock-based `hnsw_add_vertices` (float) and the opt-in `deterministic_build` flag are removed. - The deterministic path now supports the CAGRA level-0 import configuration: `init_level0=false` skips the level-0-only bucket (level 0 is supplied by the imported CAGRA graph), and `keep_max_size_level0` fills the base layer to 2*M. So `IndexHNSWCagra` (CPU) and `GpuIndexCagra::copyTo(IndexHNSWCagra*)` build through the deterministic path. - The binary `IndexBinaryHNSW` keeps its own independent lock-based build (it has no deterministic variant). Background -- HNSW construction in Faiss was non-deterministic under parallel builds: multiple runs of `IndexHNSW::add` with the same data and seeds could produce different graphs, a problem for persistence, crash recovery, and replication (the ParlayANN motivation, https://arxiv.org/abs/2305.04359). Sources of non-determinism were: (1) the reciprocal-link write race in `add_links_starting_from_impl`; (2) floating-point distance ties resolved in heap/visitation order; (3) the entry-point bootstrap `#pragma omp critical` race. Algorithm (adapted from ParlayANN to Faiss's level-batched structure): - Per level bucket (highest first, deterministic shuffle), points are inserted in prefix-doubling sub-batches (batch sizes 1, 2, 4, ... capped at 2% of the index). - Phase A (`HNSW::compute_forward_links_deterministic`, parallel): each point greedily descends and computes its forward links against the immutable snapshot from the end of the previous sub-batch, writing only its own neighbor slots. Reciprocal-edge requests are collected, not applied, so this phase is race-free. - Phase B (`HNSW::merge_reverse_links_deterministic`, parallel): reverse edges are grouped by destination with a fixed-size 256-bucket radix partition on the low bits of `dest` (a small constant bucket count, independent of `ntotal` and thread count, so grouping stays O(edges) in memory), each bucket sorted by `(level, dest)` and merged in parallel. Every affected node is merged exactly once in a total order (distance, ties by id) and re-pruned with the same RNG heuristic. Because every `dest` maps to exactly one bucket, distinct nodes touch disjoint slots (no locks) and the merge is order- and thread-count-independent. The Phase-B parallel-for uses `schedule(static)` — the libomp dynamic dispatcher segfaults in some build configs (the pre-existing lock-based build carried the same warning). Guarantee: the resulting graph is reproducible across runs at a fixed thread count and, in practice, across thread counts (the merge is fully order-independent). Recall matches the previous default at every efSearch. ## Performance: build time (40M, d=128, M=32, efC=64, 166 threads) 10-round interleaved timing study (one deterministic + one lock-based build per round, so both see identical host conditions): deterministic per-round s: 285.58 275.03 280.84 272.62 273.73 272.61 272.05 272.90 269.87 272.29 lock-based per-round s: 306.23 352.97 322.76 294.23 339.32 303.04 291.46 341.96 359.31 282.92 deterministic: min=269.87 mean=274.75 median=272.76 max=285.58 std=4.53 lock-based: min=282.92 mean=319.42 median=314.50 max=359.31 std=26.12 det/lock: mean=0.860 (deterministic ~14% faster), median=0.867 The deterministic build is ~14% faster than the removed lock-based build at 40M and ~6x more stable run-to-run (std 4.53s vs 26.12s), since it does not depend on lock-contention timing. Peak RSS ~66GB vs ~56GB. Recall matches at every efSearch (byte-identical graph across builds). ## Performance: search time Back on the deterministic HEAD, tree clean. Here's the matched A/B — same 40M synthetic data, same machine (AMD Genoa, 166 cores), search_repeat=100, deterministic (my HEAD) vs lock-based (parent commit). Since my diff doesn't touch search() at all, any difference is purely graph structure + measurement noise. Search QPS: deterministic vs lock-based (40M synthetic, repeat=100) HNSW16 ┌──────────┬─────────────────┬─────────┬──────────┬───────┐ │ efSearch │ recall det/lock │ QPS det │ QPS lock │ Δ │ ├──────────┼─────────────────┼─────────┼──────────┼───────┤ │ 64 │ 0.828/0.820 │ 170,329 │ 177,995 │ −4.3% │ ├──────────┼─────────────────┼─────────┼──────────┼───────┤ │ 128 │ 0.866/0.862 │ 112,727 │ 110,727 │ +1.8% │ ├──────────┼─────────────────┼─────────┼──────────┼───────┤ │ 256 │ 0.886/0.888 │ 59,815 │ 56,784 │ +5.3% │ └──────────┴─────────────────┴─────────┴──────────┴───────┘ HNSW32 ┌──────────┬─────────────────┬─────────┬──────────┬───────┐ │ efSearch │ recall det/lock │ QPS det │ QPS lock │ Δ │ ├──────────┼─────────────────┼─────────┼──────────┼───────┤ │ 64 │ 0.935/0.930 │ 110,186 │ 108,411 │ +1.6% │ ├──────────┼─────────────────┼─────────┼──────────┼───────┤ │ 128 │ 0.958/0.953 │ 68,019 │ 66,308 │ +2.6% │ ├──────────┼─────────────────┼─────────┼──────────┼───────┤ │ 256 │ 0.965/0.960 │ 37,624 │ 35,828 │ +5.0% │ └──────────┴─────────────────┴─────────┴──────────┴───────┘ HNSW32,SQ8 ┌──────────┬─────────────────┬─────────┬──────────┬────────┐ │ efSearch │ recall det/lock │ QPS det │ QPS lock │ Δ │ ├──────────┼─────────────────┼─────────┼──────────┼────────┤ │ 64 │ 0.926/0.934 │ 220,713 │ 198,325 │ +11.3% │ ├──────────┼─────────────────┼─────────┼──────────┼────────┤ │ 128 │ 0.948/0.953 │ 117,504 │ 129,173 │ −9.0% │ ├──────────┼─────────────────┼─────────┼──────────┼────────┤ │ 256 │ 0.961/0.963 │ 58,582 │ 65,551 │ −10.6% │ └──────────┴─────────────────┴─────────┴──────────┴────────┘ (Low-ef points ef16/32 omitted from the verdict — even at 100 repeats their std is ~8–20%, too noisy; ef128/256 std is ~3–5%.) Verdict: no search-QPS regression - Pure HNSW (16, 32): QPS at parity — within ±5%, and actually slightly faster deterministic at the high-recall points (ef128/256), with equal-or-better recall. - HNSW32,SQ8: more scatter (±10%, mixed direction) — but it tracks small correlated recall differences (det ef256 is 0.961 vs 0.963), i.e. the two different graphs sit at slightly different recall/QPS operating points, not a systematic slowdown. Search code is identical, so this is graph-structure + noise, not a code regression. If you want it pinned down, a recall-matched (interpolated) comparison would remove the operating-point confound. - Bonus: the deterministic build was 2–3× faster in every case (e.g. HNSW32: 277 s vs 527 s; HNSW16: 164 s vs 429 s) — consistent with all prior results. ## Single-threaded (OMP_NUM_THREADS=1) Customers frequently build with OMP=1 or OpenMP disabled, so this case matters. Measured at 1M / d=128 / M=32 / efC=64, single-threaded: build time: lock-based 188.36s vs deterministic 176.40s (0.94x -> deterministic ~6% FASTER) peak RSS: 1.6 GB (both, identical) recall@10 ef 16/32/64/128: lock-based .8830/.9387/.9676/.9853 vs deterministic .8832/.9381/.9625/.9798 No single-threaded regression: the deterministic build is slightly faster (it avoids the per-node OpenMP lock ops), uses the same memory, and matches recall within noise. Note the lock-based build was already deterministic at a single thread, so single-threaded users lose nothing and gain a small speedup. ## Serialization compatibility No on-disk format change, verified in `index_read.cpp` / `index_write.cpp`: - `deterministic_build` was never serialized (zero references), so removing it is format-neutral. It was a runtime build flag, like `retain_locks`. - `write_HNSW` / `read_HNSW` and the `IndexHNSW` field layout are unchanged. The subtype fourcc tags, header, CAGRA block, graph CSR (entry_point / max_level / levels / offsets / neighbors / efC / efS), and storage are all as before. - `keep_max_size_level0` is still serialized only for the CAGRA subtype (`IHc2`/`IHNc`); `init_level0` is build-only (not serialized). - The deterministic build emits the same HNSW CSR structure (only neighbor content differs), so old indexes read unchanged and new indexes remain readable by older Faiss. - Verified by the `io_and_retest` serialize -> deserialize -> re-search round-trips in `test_graph_based.py` / `test_hnsw.cpp` (all pass). ## CAGRA API for HNSW build on multi-GPU (aka D106837134) — MAST verification Verified end-to-end on MAST (8x H100 Grand Teton, Approach D, 100M vectors) with this change in the build — the multi-GPU CAGRA -> HNSW graph-build time is comparable to the D106837134 baseline (no regression): all_neighbors build: 367.4s optimize: 231.7s copyTo: 18.4s serialize: 28.9s (66 GB) INDEX build -> serialize total: 661.7s (11.0 min) [D106837134 baseline: 721s] recall@10 (tiled 100M): ef64 0.7746, ef128 0.8830, ef256 0.9429 - This confirms this CPU-side change builds, links, and runs in the GPU CAGRA binary at scale and does not regress the pipeline. Note the Approach-D run uses copyTo(base_level_only=True), which imports the CAGRA graph directly as HNSW level 0 and skips add(), so it does not itself route through the deterministic add(). - The deterministic CAGRA level-0 import this change adds (the copyTo path with base_level_only=False: init_level0=false skips the level-0 bucket; keep_max_size_level0 fills the base layer) is covered by passing unit tests: `Test_IndexHNSWCagra_BaseLevelOnly_RangeSearch` (C++), `test_hnsw_no_init_level0`, and `test_hnsw_cagra_IP` / `_base_level_only` (Python). ## Behavioral note: level-0 base layer under keep_max_size_level0 (reviewers, please note) One deliberate difference from the removed lock-based build, in the CAGRA base-layer case only: the old build gated the "fill the level-0 list up to 2*M" behavior on the inserted point's OWN top level (`keep_max_size_level0 && pt_level == 0`), so a level>=1 node's level-0 list could be pruned below 2*M. The deterministic build gates on the LINK level (`keep_max_size_level0 && level == 0`), so EVERY node's level-0 list is filled to 2*M when `keep_max_size_level0` is set (not only the level-0-only points). This is a strict superset of the old coverage -- it fills exactly to the 2*M slot capacity (no overflow) and yields a fuller/denser base layer for CPU `IndexHNSWCagra`, which is what `GpuIndexCagra::copyFrom(IndexHNSWCagra*)` reads back. It is INERT for the default build (`keep_max_size_level0` defaults to false, so the gate is never true) and never affects a non-CAGRA graph. Called out explicitly so reviewers know the CPU `IndexHNSWCagra` base-layer graph is intentionally denser than the pre-diff build; worth a sanity check against GPU `copyFrom` expectations. Differential Revision: D112025877
1 parent 4d74915 commit 615a20e

11 files changed

Lines changed: 958 additions & 685 deletions

File tree

.github/actions/build_cmake/action.yml

Lines changed: 35 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,11 @@ inputs:
3232
description: 'Upload test artifacts. Prevents collisions when multiple jobs need to run build_cmake.'
3333
required: false
3434
default: 'true'
35+
sanitizers:
36+
description: 'Build with AddressSanitizer + UndefinedBehaviorSanitizer and run the tests under them.'
37+
required: false
38+
# THROWAWAY (do not land): forced ON so the sanitizer build/tests always run.
39+
default: 'ON'
3540
runs:
3641
using: composite
3742
steps:
@@ -201,7 +206,7 @@ runs:
201206
- name: Setup ccache
202207
uses: hendrikmuhs/ccache-action@v1
203208
with:
204-
key: ${{ runner.os }}-${{ runner.arch }}-${{ inputs.opt_level }}-gpu${{ inputs.gpu }}-cuvs${{ inputs.cuvs }}-rocm${{ inputs.rocm }}-svs${{ inputs.svs }}
209+
key: ${{ runner.os }}-${{ runner.arch }}-${{ inputs.opt_level }}-gpu${{ inputs.gpu }}-cuvs${{ inputs.cuvs }}-rocm${{ inputs.rocm }}-svs${{ inputs.svs }}-san${{ inputs.sanitizers }}
205210
max-size: 2G
206211
update-package-index: true
207212
- name: Setup macOS Metal environment
@@ -225,6 +230,8 @@ runs:
225230
-DFAISS_ENABLE_ROCM=${{ inputs.rocm }} \
226231
-DFAISS_OPT_LEVEL=${{ inputs.opt_level }} \
227232
-DFAISS_ENABLE_SVS=${{ inputs.svs }} \
233+
-DFAISS_ENABLE_ASAN=${{ inputs.sanitizers }} \
234+
-DFAISS_ENABLE_UBSAN=${{ inputs.sanitizers }} \
228235
-DFAISS_ENABLE_C_API=ON \
229236
-DPYTHON_EXECUTABLE=$CONDA/bin/python \
230237
-DCMAKE_BUILD_TYPE=Release \
@@ -251,6 +258,11 @@ runs:
251258
- name: C++ tests
252259
if: inputs.metal != 'ON'
253260
shell: bash
261+
env:
262+
# Leaks are expected (SWIG/BLAS/OpenMP own long-lived allocations), so
263+
# only hunt for memory errors / UB. Harmless on non-sanitizer builds.
264+
ASAN_OPTIONS: detect_leaks=0:abort_on_error=1
265+
UBSAN_OPTIONS: print_stacktrace=1:halt_on_error=1
254266
run: |
255267
conda list --show-channel-urls
256268
export GTEST_OUTPUT="xml:$(realpath .)/test-results/googletest/"
@@ -269,7 +281,7 @@ runs:
269281
shell: bash
270282
run: python3 -m pytest faiss/gpu_metal/test/test_metal_python.py -v
271283
- name: C++ perf benchmarks
272-
if: inputs.rocm == 'OFF' && inputs.metal != 'ON'
284+
if: inputs.rocm == 'OFF' && inputs.metal != 'ON' && inputs.sanitizers != 'ON'
273285
shell: bash
274286
run: |
275287
conda list --show-channel-urls
@@ -319,12 +331,31 @@ runs:
319331
# Confirm torch links against ROCm 7.2 (matches the system /opt/rocm-7.2.0).
320332
python -c "import torch; print('torch', torch.__version__, 'hip', torch.version.hip)"
321333
- name: Python tests (CPU only)
322-
if: inputs.gpu == 'OFF' && inputs.metal != 'ON'
334+
if: inputs.gpu == 'OFF' && inputs.metal != 'ON' && inputs.sanitizers != 'ON'
323335
shell: bash
324336
run: |
325337
conda list --show-channel-urls
326338
pytest --junitxml=test-results/pytest/results.xml tests/test_*.py
327339
pytest --junitxml=test-results/pytest/results-torch.xml tests/torch_*.py
340+
- name: Python tests (CPU only, sanitizers)
341+
if: inputs.gpu == 'OFF' && inputs.metal != 'ON' && inputs.sanitizers == 'ON'
342+
shell: bash
343+
env:
344+
ASAN_OPTIONS: detect_leaks=0:abort_on_error=1
345+
UBSAN_OPTIONS: print_stacktrace=1:halt_on_error=1
346+
run: |
347+
conda list --show-channel-urls
348+
# faiss.so / _swigfaiss*.so are built with -fsanitize=address, so a
349+
# non-instrumented CPython must load the ASan (then UBSan) runtime first
350+
# or the import aborts ("ASan runtime does not come first"). Preload them
351+
# from the same conda toolchain that built faiss.
352+
GCC=x86_64-conda-linux-gnu-gcc
353+
export LD_PRELOAD="$($GCC -print-file-name=libasan.so):$($GCC -print-file-name=libubsan.so)"
354+
# This job targets the HNSW build determinism bug; it is a dynamic-
355+
# dispatch build, so the NONE/AVX2/AVX512 variants of these suites all
356+
# run under the sanitizers. Broaden to tests/test_*.py once green.
357+
pytest --junitxml=test-results/pytest/results.xml \
358+
tests/test_graph_based.py tests/test_index_binary.py
328359
- name: Python tests (CPU + GPU)
329360
if: inputs.gpu == 'ON'
330361
shell: bash
@@ -349,7 +380,7 @@ runs:
349380
if: inputs.upload_artifacts == 'true'
350381
uses: actions/upload-artifact@v4
351382
with:
352-
name: test-results-arch=${{ runner.arch }}-opt=${{ inputs.opt_level }}-gpu=${{ inputs.gpu }}-cuvs=${{ inputs.cuvs }}-rocm=${{ inputs.rocm }}-svs=${{ inputs.svs }}
383+
name: test-results-arch=${{ runner.arch }}-opt=${{ inputs.opt_level }}-gpu=${{ inputs.gpu }}-cuvs=${{ inputs.cuvs }}-rocm=${{ inputs.rocm }}-svs=${{ inputs.svs }}-san=${{ inputs.sanitizers }}
353384
path: test-results
354385
- name: Check installed packages channel
355386
if: inputs.metal != 'ON'

0 commit comments

Comments
 (0)