goankur commented on PR #16145:
URL: https://github.com/apache/lucene/pull/16145#issuecomment-5985452697
I tested the current head (4721c75) against main on two workloads. Summary:
the PR fixes the mixed warm/cold case. But it slows hot term queries by up to
34%. Two small changes remove the slowdown and keep the fix.
**Setup.** g6.4xlarge (16 vCPU, 60 GB RAM, local NVMe), kernel 6.18.51, JDK
25. All arms are built on main `3bb5f43990`. Only the store classes differ.
- **Rerank:** 25M Cohere-v3 1024-d, 1-bit BBQ + fp32 rerank of 500
candidates per query (top-100 × overSample 5), fanout 100, single stream, 10k
queries, 2 reps. systemd cgroup `MemoryMax` of 10 GB (~101 GB of vectors, so
larger than RAM) and of 20 GB (**the working set fits**).
- **luceneutil wikimedium10m** (6.1 GiB index): each arm runs in 20 separate
JVMs, alternating with the baseline, and luceneutil compares the 20 results.
`fincore` confirms that the index is 100% resident. A main-vs-main run gives a
noise floor of ±3%.
- bpftrace counts `mincore` calls (from `isLoaded()` in Lucene's
`prefetch()`), which is the number of probes. It traces the syscall
system-wide, keyed by PID, and a `ps` loop maps each search JVM's PID to its
arm.
### Rerank, p99 ms
| | cgroup 10 GB | cgroup 20 GB (**working set fits**) |
|---|---|---|
| main | 172 | 132 |
| no backoff | 17.0 | 7.6 |
| this PR | 17.0 | 6.7 |
| PR + 2 changes, your gap schedule | 17.0 | 6.7–6.8 |
| PR + 2 changes, fixed gap 64 | 17.2–17.5 | 6.6 |
The PR fixes the case it targets. At 20 GB its p50 is 17% lower than with no
backoff (4.14 vs 4.97 ms), so the sampling works. One probe costs about 1.9 µs
here.
### Hot wikimedium10m, QPS change vs main
| | AndHighLow | LowTerm | HighTermTitleSort | probes / JVM |
|---|---|---|---|---|
| main | | | | 400 |
| no backoff | −23% | −16% | −36% | 454k |
| this PR | −23% | −22% | −34% | 243k |
| PR + 2 changes, your gap schedule | −4.7% (p=0.02) | −3.4% (p=0.16) |
−1.0% (p=0.76) | 18.4k |
| PR + 2 changes, fixed gap 64 | −2.6% (p=0.26) | +3.1% | −3.0% (p=0.37) |
5.6k |
With the PR, other term-heavy tasks (OrHighLow, MedTerm, AndHighMed,
OrHighMed, Prefix3) lose 8–15% at p<0.001. From a cold cache, the fixed gap 64
has no task below p=0.05, and your schedule has six (worst: Prefix3 −11.9%).
### Why the PR is slow on hot data
Your JMH results agree with mine where they overlap. But those harnesses use
one force-merged segment and one `StoredFields` per thread for the whole trial,
so they don't show the cause below. Isolated runs confirm both causes:
1. **Fresh clones always probe.** `shouldProbe(prefetchCount++)` uses a
counter per clone that starts at 0, and `BitMixer.mix(0) == 0`.
`SegmentTermsEnum` clones `termsIn` for each lookup, so every term lookup in
every segment probes. This cost does not depend on JVM lifetime. A fix for this
cause alone removes the HighTermTitleSort loss. But AndHighLow is still −18% to
−20%.
2. **Each new file probes every call until it sees 1024 hits in a row, also
when it is fully cached.** Keeping only this ramp (no sampling after it)
reproduces the rest: AndHighLow −17.5%, 29,696 probes per JVM (29 files ×
1024). Never probing is at noise level, so the probes cost, not the code. Every
NRT refresh or merge opens new files. The short luceneutil JVMs (~12 s) make
this cost larger than in a long-running server.
`mincore` latency also has a long tail. About 70% of calls take 1–2 µs, but
thousands of calls per run take 0.5–16 ms.
### Proposal
Keep N = 1024 and the reset on a miss. Change two things:
1. **Start the shared counter at N.** A new file then samples from its first
call. The first miss sends it back to a probe on every call until it sees 1024
hits in a row, as now. A cached file never pays the ramp. The cost: a newly
opened cold file can miss prefetches for up to one gap before a sample catches
the first miss.
2. **Sample with `ThreadLocalRandom` instead of the counter per clone.** Its
state lives in `Thread` fields, not in a `ThreadLocalMap`, so nothing is
allocated or retained. This also removes the `prefetchCount` field.
I measured both changes with your gap schedule and with a fixed gap of 64
(tables above):
- **Your schedule** matches the PR under memory pressure. But it keeps a
small ramp: a file starts at gap 16 and needs about 1024 more sampled hits
before the gap doubles. Hot AndHighLow is −4.7% (p=0.02), and cold-start
Prefix3 is −11.9%.
- **A fixed gap of 64** is at noise on hot and cold-start wikimedium and has
the lowest hot-rerank p99. It costs about 0.3 ms p99 under memory pressure
(17.2–17.5 vs 17.0 ms). I did not run your `prefetchOnly` benchmark with it;
there it probes 4× more often than your 256 cap in steady state.
My recommendation would be the fixed gap of 64:
```java
static final int SKIP = 64;
private final AtomicInteger consecutiveHits = new AtomicInteger(N); // was 0
boolean shouldProbe() {
return consecutiveHits.get() < N
|| (ThreadLocalRandom.current().nextInt() & (SKIP - 1)) == 0;
}
```
`onHit()` then stops writing at N instead of `HITS_AT_MAX_SKIP`. The full
core test suite passes with both changes.
### Unrelated bug
`slice(String, long, long, IOContext)` now calls `buildSlice(...)` directly.
It skips the offset/length bounds check in `slice(String, long, long)`, which
main calls.
**Not tested:** EBS or other network storage, concurrent load, NRT refresh.
I can share the scripts and the branch.
--
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]
