wangzhigang1999 commented on code in PR #95:
URL:
https://github.com/apache/paimon-vector-index/pull/95#discussion_r3951327879
##########
core/benches/ann_bench.rs:
##########
@@ -999,8 +999,48 @@ fn run_query_case(
("DISKANN", DiskAnnRawVectorEncoding::F16) => "f16",
_ => "none",
};
+ // The original passes above retain their first-pass I/O and recall
semantics.
+ // Optional CI timing starts after those complete sweeps have warmed each
reader.
+ let steady_min_ms: u64 = read_env("ANN_STEADY_MIN_MS", 0)?;
+ if steady_min_ms > 0 && !storage.latency.is_zero() {
+ return Err("ANN_STEADY_MIN_MS requires local_ssd_warm_cache".into());
+ }
+ let mut steady_sequential_queries = 0usize;
+ let mut steady_batch_queries = 0usize;
+ let mut steady_latencies = Vec::new();
+ let mut steady_sequential_elapsed = Duration::ZERO;
+ let mut steady_batch_elapsed = Duration::ZERO;
+ if steady_min_ms > 0 {
+ let minimum = Duration::from_millis(steady_min_ms);
+ let started = Instant::now();
+ while started.elapsed() < minimum {
+ for query in dataset.queries.chunks_exact(config.d) {
+ let query_started = Instant::now();
+ std::hint::black_box(reader.search(query, search)?);
+ steady_latencies.push(query_started.elapsed());
+ }
Review Comment:
Fixed in 2a76386: retain latency samples only for the first complete warm
sequential sweep (nq entries). QPS still measures all sweeps for the configured
minimum duration. The report and README state the P95 sampling scope; no
histogram or new dependency is needed.
##########
core/benches/ann_bench.rs:
##########
@@ -1096,7 +1139,9 @@ fn exact_ground_truth(dataset: &Dataset, k: usize) ->
Vec<Vec<i64>> {
.then_with(|| left.1.cmp(&right.1))
});
}
- distances.into_iter().map(|(_, row)| row).collect()
+ // Borrow the retained top-k slice: consuming the Vec can reuse its
+ // original N-vector allocation for every ground-truth row.
Review Comment:
The intent is to avoid allocation reuse, not enable it. Verified on Linux
with rustc 1.98.0: after truncating Vec<(f32, i64)> from 10,000 to 10 elements,
consuming collect produced Vec<i64> len=10/capacity=20,000, whereas borrowed
collect produced len=10/capacity=10. In 2a76386 the comment now explicitly says
borrowing creates a fresh top-k Vec and avoids the consuming iterator's
possible in-place allocation reuse.
--
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]