Avoid double-advancing HNSW visited tables (#5596)

Summary:
The outer HNSW search loop called `vt->advance()` a second time. `HNSW::search`
already ends with `vt.advance()` at `impl/HNSW.cpp:1861`, so the caller in
`IndexHNSW.cpp` advanced the visited table once more per query. This diff
removes that call and adds a comment that names the owner of the advance.

The sibling call in `search_level_0` stays, because `search_level_0_impl` does
not advance.

**Microbenchmark.** The host holds 72 Cooper Lake cores. The build is
`mode/opt`. One core is pinned, and the two binaries interleave over 5 rounds.
The table reports the median of the medians. `IndexHNSWFlat`, `d=64`, `M=16`,
`use_visited_hashset=false` to select the versioned array, one query for each
iteration.

| ntotal | before | after | ratio |
|---|---|---|---|
| 100k | 44.2 us | 44.2 us | 1.00x |
| 400k | 63.8 us | 69.1 us | 0.92x |

**The change gives no measurable speed gain.** Both rows sit inside a 10% to 27%
spread. The arithmetic agrees: `visno` is a `uint8_t` that cycles from 1 to 254,
so the code clears the table once for every 250 advances. Two advances for each
query halve that interval, which costs about 800 extra bytes of memset for each
query against a search of 44 us to 69 us.

The value of the diff is the removal of a redundant call, not a speed gain.

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

Test Plan:
`buck2 test fbcode//faiss/tests:test_ivf_index fbcode//faiss/tests:test_custom_result_handler fbcode//faiss/tests:test_hnsw fbcode//faiss/tests:test_rabitq_simd_neon fbcode//faiss/tests:test_params_override fbcode//faiss/tests:test_hnsw_rabitq fbcode//faiss/tests:test_rabitq fbcode//faiss/tests:test_search_params fbcode//faiss/tests:test_io_corrupted fbcode//faiss/tests:test_rabitq_utils`, run at the top of the stack on aarch64: 173 pass, 0 fail, 3 skip.

`buck2 build fbcode//faiss:faiss` succeeds in the static aarch64 build and with `-c faiss.dynamic_dispatch=true`.

Reviewed By: alibeklfc

Differential Revision: D121846632

Pulled By: mnorris11

fbshipit-source-id: 8928c4d4648ce061e63c084ded9c74fa17ef66bd

faiss/IndexHNSW.cpp

@@ -475,6 +475,9 @@ void hnsw_search(
                         rq->stats.reset();
                     }
 
+                    // HNSW::search owns the per-query generation advance. The
+                    // reusable-table acquisition above only clears stale TLS
+                    // state before this thread starts processing queries.
                     HNSWStats stats =
                             hnsw.search(*dis, index, *res, *vt, params);
                     n1 += stats.n1;
@@ -486,7 +489,6 @@ void hnsw_search(
                         n_rabitq_refine += rq->stats.n_refine;
                     }
                     res->end();
-                    vt->advance();
                 } catch (...) {
                     omp_capture_exception(ex, [&] { interrupt = true; });
                 }

tests/test_hnsw.cpp

@@ -848,6 +848,26 @@ struct ScopedOmpThreads {
     }
 };
 
+TEST_F(HNSWTest, TEST_search_advances_one_generation_per_query) {
+    ScopedOmpThreads omp_guard;
+    omp_set_num_threads(1);
+    index->hnsw.use_visited_hashset = false;
+
+    auto& reusable = dynamic_cast<faiss::VisitedTableVector&>(
+            faiss::VisitedTable::get_reusable(index->ntotal, false));
+    std::fill(reusable.visited.begin(), reusable.visited.end(), 0);
+    reusable.visno = 10;
+
+    std::vector<faiss::idx_t> labels(k);
+    std::vector<float> distances(k);
+    index->search(1, xq->data(), k, distances.data(), labels.data());
+
+    // Acquiring the reusable table advances 10 -> 11 to clear stale state.
+    // HNSW::search then advances 11 -> 12 after the query. The outer wrapper
+    // must not perform a third advance.
+    EXPECT_EQ(12, reusable.visno);
+}
+
 TEST_F(HNSWTest, TEST_search_reuse_correctness) {
     ScopedOmpThreads omp_guard;