From 5ce9bf7fda7a4bc7f52ea990f58b31dfbc7e2cd6 Mon Sep 17 00:00:00 2001 From: Pascal Seitz Date: Mon, 8 Jun 2026 20:11:47 +0200 Subject: [PATCH] use get_range when possible --- columnar/src/block_accessor.rs | 71 ++++++++++++++++++- columnar/src/column_values/mod.rs | 12 +++- columnar/src/column_values/u64_based/tests.rs | 16 +++++ src/aggregation/buffered_sub_aggs.rs | 2 + 4 files changed, 99 insertions(+), 2 deletions(-) diff --git a/columnar/src/block_accessor.rs b/columnar/src/block_accessor.rs index 2793f3a1b..ccd9f0e2b 100644 --- a/columnar/src/block_accessor.rs +++ b/columnar/src/block_accessor.rs @@ -17,7 +17,18 @@ impl pub fn fetch_block<'a>(&'a mut self, docs: &'a [u32], accessor: &Column) { if accessor.index.get_cardinality().is_full() { self.val_cache.resize(docs.len(), T::default()); - accessor.values.get_vals(docs, &mut self.val_cache); + // When the docs form a contiguous ascending run we can fetch the values + // as a single range. This lets codecs (e.g. bitpacked) bulk-decode the + // slice instead of gathering value-by-value, and avoids per-value dynamic + // dispatch. `docs` is always sorted ascending and free of duplicates here, + // so comparing the endpoints is enough to detect contiguity. + if is_contiguous(docs) { + accessor + .values + .get_range(docs[0] as u64, &mut self.val_cache); + } else { + accessor.values.get_vals(docs, &mut self.val_cache); + } } else { self.docid_cache.clear(); self.row_id_cache.clear(); @@ -158,6 +169,22 @@ impl } } +/// Returns true if `docs` is a contiguous ascending run `[d, d + 1, ..., d + n - 1]`. +/// +/// Assumes `docs` is sorted ascending and free of duplicates (the invariant for the +/// doc blocks passed to `fetch_block`), so comparing the endpoints is sufficient. +#[inline] +fn is_contiguous(docs: &[u32]) -> bool { + let (Some(&first), Some(&last)) = (docs.first(), docs.last()) else { + return false; + }; + debug_assert!( + docs.windows(2).all(|w| w[0] < w[1]), + "fetch_block requires docs sorted ascending without duplicates" + ); + (last - first) as usize + 1 == docs.len() +} + /// Given two sorted lists of docids `docs` and `hits`, hits is a subset of `docs`. /// Return all docs that are not in `hits`. fn find_missing_docs(docs: &[u32], hits: &[u32], mut callback: F) @@ -288,4 +315,46 @@ mod tests { assert_eq!(accessor.docid_cache, vec![0]); assert_eq!(accessor.val_cache, vec![1]); } + + #[test] + fn test_is_contiguous() { + assert!(!is_contiguous(&[])); + assert!(is_contiguous(&[5])); + assert!(is_contiguous(&[5, 6, 7, 8])); + assert!(is_contiguous(&[0, 1, 2])); + assert!(!is_contiguous(&[5, 7, 8])); + assert!(!is_contiguous(&[0, 1, 3])); + } + + #[test] + fn test_fetch_block_contiguous_and_gather_match() { + use crate::column_index::ColumnIndex; + use crate::column_values::{ + ALL_U64_CODEC_TYPES, serialize_and_load_u64_based_column_values, + }; + + let vals: Vec = (0..200u64).map(|i| i * 7 + 3).collect(); + let values = + serialize_and_load_u64_based_column_values::(&&vals[..], &ALL_U64_CODEC_TYPES); + let column = Column { + index: ColumnIndex::Full, + values, + }; + + let check = |accessor: &mut ColumnBlockAccessor, docs: &[u32]| { + accessor.fetch_block(docs, &column); + let got: Vec<(u32, u64)> = accessor.iter_docid_vals(docs, &column).collect(); + let expected: Vec<(u32, u64)> = docs.iter().map(|&d| (d, vals[d as usize])).collect(); + assert_eq!(got, expected); + }; + + let mut accessor = ColumnBlockAccessor::::default(); + // Contiguous block -> get_range fast path. + check(&mut accessor, &(10..74).collect::>()); + // Non-contiguous block -> get_vals gather path. + check(&mut accessor, &[0, 5, 9, 100, 199]); + // Single doc and full span. + check(&mut accessor, &[42]); + check(&mut accessor, &(0..200).collect::>()); + } } diff --git a/columnar/src/column_values/mod.rs b/columnar/src/column_values/mod.rs index cdab3be3e..e73d3ca36 100644 --- a/columnar/src/column_values/mod.rs +++ b/columnar/src/column_values/mod.rs @@ -119,8 +119,18 @@ pub trait ColumnValues: Send + Sync + DowncastSync { /// the segment's `maxdoc`. #[inline(always)] fn get_range(&self, start: u64, output: &mut [T]) { - for (out, idx) in output.iter_mut().zip(start..) { + let mut out_chunks = output.chunks_exact_mut(4); + let mut idx = start; + for out_x4 in out_chunks.by_ref() { + out_x4[0] = self.get_val(idx as u32); + out_x4[1] = self.get_val((idx + 1) as u32); + out_x4[2] = self.get_val((idx + 2) as u32); + out_x4[3] = self.get_val((idx + 3) as u32); + idx += 4; + } + for out in out_chunks.into_remainder() { *out = self.get_val(idx as u32); + idx += 1; } } diff --git a/columnar/src/column_values/u64_based/tests.rs b/columnar/src/column_values/u64_based/tests.rs index ff5b7051a..be27552ba 100644 --- a/columnar/src/column_values/u64_based/tests.rs +++ b/columnar/src/column_values/u64_based/tests.rs @@ -121,6 +121,22 @@ pub(crate) fn create_and_validate( reader.get_vals(&all_docs, &mut buffer); assert_eq!(vals, buffer); + // Validate `get_range` over the full column and a sub-range. The sub-range starts + // at a non-zero offset to exercise the entrance-ramp alignment of the batch decode. + buffer.resize(all_docs.len(), 0); + reader.get_range(0, &mut buffer); + assert_eq!(vals, buffer, "get_range (full) mismatch in data set {name}"); + if vals.len() >= 2 { + let start = 1usize; + buffer.resize(vals.len() - start, 0); + reader.get_range(start as u64, &mut buffer); + assert_eq!( + &vals[start..], + &buffer[..], + "get_range (sub-range) mismatch in data set {name}" + ); + } + if !vals.is_empty() { let test_rand_idx = rand::rng().random_range(0..=vals.len() - 1); let expected_positions: Vec = vals diff --git a/src/aggregation/buffered_sub_aggs.rs b/src/aggregation/buffered_sub_aggs.rs index 87ce47bb5..0e8c76706 100644 --- a/src/aggregation/buffered_sub_aggs.rs +++ b/src/aggregation/buffered_sub_aggs.rs @@ -138,6 +138,7 @@ impl SubAggBuffer for HighCardSubAggBuffer { } } + #[inline] fn push(&mut self, bucket_id: BucketId, doc_id: DocId) { let idx = bucket_id % NUM_PARTITIONS as u32; let slot = &mut self.partitions[idx as usize]; @@ -196,6 +197,7 @@ impl SubAggBuffer for LowCardSubAggBuffer { } } + #[inline] fn push(&mut self, bucket_id: BucketId, doc_id: DocId) { let idx = bucket_id as usize; if self.per_bucket_docs.len() <= idx {