Fix OpenMP critical section contention in IndexBinaryHNSW search (#4909)

Summary:
Pull Request resolved: https://github.com/facebookresearch/faiss/pull/4909

## Problem

We recently found **87% of CPU** was being wasted on OpenMP lock contention in `FlatHammingDis::~FlatHammingDis`, not on useful computation.

The flame graph breakdown:
- 88.2% CPU in `openmp_worker` threads
- 87.2% in `FlatHammingDis::~FlatHammingDis` → `__kmpc_critical_with_hint` → `__kmp_acquire_queuing_lock` → `__sched_yield` (84% CPU spinning/yielding on lock)

## Root Cause

The `FlatHammingDis` destructor used `#pragma omp critical` to accumulate a single `size_t` counter (`hnsw_stats.ndis += ndis`). Unnamed `#pragma omp critical` sections share a **global serialization lock** — when all OpenMP threads exit the `#pragma omp parallel` block in `IndexBinaryHNSW::search()` simultaneously, they ALL enter the destructor at the same time, serializing on that single lock.

With N threads, this means N sequential lock acquisitions where each thread spins/yields waiting for its turn. This is O(N) serialization at the end of every search call.

In `IndexBinaryHNSWCagra::search()` with `base_level_only=true`, the situation is even worse: `FlatHammingDis` is created and destroyed **per query iteration** inside `#pragma omp parallel for`, causing `n × num_threads` critical section entries.

## Fix

Replace `#pragma omp critical` with `#pragma omp atomic`. Since `hnsw_stats.ndis += ndis` is a simple `size_t` addition, `#pragma omp atomic` compiles to a single hardware atomic instruction (`lock xadd` on x86-64) — orders of magnitude faster than a mutex-based critical section, with effectively zero contention.

For reference, the float HNSW path in `IndexHNSW.cpp` already uses the correct pattern: `#pragma omp for reduction(+: n1, n2, ndis, nhops)` with a single-threaded `hnsw_stats.combine()` call outside the parallel region.

## Impact

- Eliminates ~87% CPU waste from lock contention in binary HNSW search
- Affects all users of `IndexBinaryHNSW::search()` and `IndexBinaryHNSWCagra::search()`
- No change to search results or statistics accuracy — `#pragma omp atomic` provides the same correctness guarantees as `#pragma omp critical` for a single `+=` operation

Reviewed By: mnorris11

Differential Revision: D95910991

fbshipit-source-id: f14b81b95c7b9f9bd7299a319cc2578fe8af3844
This commit is contained in:
Pratyaksh Sharma
2026-03-11 22:56:21 -07:00
committed by meta-codesync[bot]
parent 0fcba2284c
commit 5b83ec6856
+2 -4
View File
@@ -293,10 +293,8 @@ struct FlatHammingDis : DistanceComputer {
}
~FlatHammingDis() override {
#pragma omp critical
{
hnsw_stats.ndis += ndis;
}
#pragma omp atomic
hnsw_stats.ndis += ndis;
}
};