goankur commented on PR #16656: URL: https://github.com/apache/lucene/pull/16656#issuecomment-5786111862
Thanks jimczi - You were right, and the baseline was the problem. Measured: across a full 10,000-query run the JVM issues **23 `madvise` calls in total**, versus **~228 per query** once prefetch is wired through. Everything below is re-measured on that basis. **Changes (branch off `main`, no new SPI):** Code is on a branch rather than a PR to keep the queue clean: https://github.com/apache/lucene/compare/main...goankur:lucene:prefetch-rerank-clean. It excludes the measurement-only switches; the 10.49 ms row in table below additionally requires the #16145 backoff fix. - `KnnVectorValues#prefetch(int ord)`; array form loops over it. I kept its `<=1` skip (a lone ord is read immediately) — `TestOffHeapVectorValues` pins that, and the ring calls the single-ord form directly. - Forward `prefetch` in `ScalarQuantizedVectorValues`, `ScalarQuantizedFloat16VectorValues`, `NormalizedFloatVectorValues`. This was the silent break. - `DoubleValues#prefetch(int doc)` (default no-op), implemented by `FullPrecisionFloatVectorSimilarityValuesSource` over a separate `copy()` view so prefetching runs ahead without disturbing the scoring iterator. - `RescoreTopNQuery`: ring of doc IDs across leaves, prefetch ahead of scoring — heap stays flat, no `count × dim` materialization. Deferral is disabled when `valuesSource.needsScores()`, since `DoubleValuesSource.fromScorer` reads the scorer's current score. - **Prefetch issuance moved onto `indexSearcher.getTaskExecutor()`.** The second phase was entirely single-threaded while `AbstractKnnVectorQuery` already fans the first phase over that executor. This was the largest single win. - Test: `TestRescoreTopNQueryPrefetch` counts `IndexInput#prefetch` per extension through slices/clones and asserts `.vec` is prefetched ~once per candidate. It fails with `.vec=0` if any forwarding override is dropped. **Method:** g6.4xlarge, kernel 6.1, local NVMe; 25M Cohere-v3 1024-d, 1-bit BBQ + fp32 rerank; `overSample 5 / fanout 100`, `nquery=10000`, `topK=100`; each run in a 10 GB systemd scope against ~101 GB of vectors, page cache dropped first; recall **0.967 in every row**. `madvise`/page-fault/syscall counts via bpftrace, device via `iostat -x` (`aqu-sz`, `rareq-sz`). MGLRU is off (`lru_gen/enabled` = `0x0000`). **Single-stream p99:** | read path | p99 | aqu-sz | avg read | |---|---|---|---| | stock mmap (QD 1) | 252.0 ms | — | 27.8 KB | | mmap + batched prefetch + backoff enabled (default) | 221.0 ms | — | 19.7 KB | | mmap + batched prefetch + backoff disabled | 17.09 ms | 2.91 | 4.22 KB | | **mmap + batched + parallel prefetch** (backoff disabled) | **10.49 ms** | 10.47 | 4.22 KB | | O_DIRECT + 32-thread pool (this PR) | 13.13 ms | 6.65 | 4.33 KB | **Concurrent, 100 QPS Poisson, 32 handler threads / 32-deep queue, 0 shed** (prefetch rows with backoff disabled): | read path | p50 | p99 | |---|---|---| | mmap + batched + parallel prefetch | 7.72 | **31.83 ms** | | mmap + batched prefetch, single-threaded issuance | 11.87 | 34.53 ms | | O_DIRECT + 32-thread pool | 13.52 | 45.72 ms | **Your specific points:** - **Read amplification:** `--enable-native-access` was on, so `MADV_RANDOM` was active. The 27.6 KB reproduces (27.8 KB) but it's an artifact of nothing prefetching — with prefetch active, reads are **4.22 KB**. Per-inode attribution also shows stock pulls **91 GB `.vex` vs 88 GB `.vec`** over 10k queries (20.7 MB/query), i.e. half that traffic is graph and O_DIRECT-on-`.vec` never addressed it. - **Eviction:** O_DIRECT does avoid the fills (~0 vs 0.38 MB/query of `.vex` re-reads), but the latency cost is small: raising the cgroup 10→16 GB cut major faults **92×** (35 → 0.38/query) and p99 only **12%**. So the page cache behaves as you said. - **#16145 backoff:** live and expensive — **221 → 17.09 ms** (13×). Worth noting it's invisible in a pure-cold microbench (#16279 shows 3.389 vs 3.394 ops/ms with it disabled) because every miss resets the counter; it only bites when the pattern mixes hits and misses, which is the real rerank pattern. - **#16279:** ran it, 4 KB reads, 100 GB file, cold. `ffiPread` T01 **0.573** vs your 0.58; `mmapRandom` 0.314; `ffiPreadDirectIO` 0.643 / 2.587 / 8.219 (T01/T04/T16); `mmapRandomBatchedPrefetch` **3.394 / 9.636 / 9.852**. `MADV_RANDOM` makes no measurable difference once prefetch is on. - **`prefetch(int ord)` PR:** I already built parts of it [in my private branch]( https://github.com/apache/lucene/compare/main...goankur:lucene:prefetch-rerank-clean) so please take a look and advise the changes that would be suitable for the new PR. **Tried and discarded:** burst size (prefetch 64/128/512 candidates before scoring) is flat, 16.55/16.91/16.77 ms — in-flight depth is not issue-rate limited. Coalescing is inapplicable here (~16 candidates per segment, ~206 MB apart) and reducing `madvise` count is actively harmful: prefetching every 2nd candidate halved calls and made p99 **7.7× worse** (17.1 → 131.4 ms, faults 35 → 389/query). Each 12.2 µs `madvise` buys away an ~88 µs fault. **Where this leaves the PR:** the `VectorBatch`/`ParallelVectorReadable` SPI isn't justified — prefetch on the existing API wins in both regimes. I'd like to withdraw it and instead split out (1) your `prefetch(int ord)` + wrapper forwarding, (2) executor-parallel issuance in `RescoreTopNQuery`, (3) the 4 KB `Lucene99FlatVectorsWriter` alignment, which is orthogonal and helps every path. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
