From 19f9954050bf0dec6d8237a2a375616f66d470b1 Mon Sep 17 00:00:00 2001 From: Eric Coissac Date: Thu, 20 Aug 2026 05:44:33 +0200 Subject: [PATCH] refactor: centralize nonzero slot iteration logic across views Extract duplicated traversal logic into a shared `NonzeroSlotsView` trait and `nonzero_triples` helper. Update slice view iterators to consistently yield `(position, value)` tuples and use a temporary buffer with explicit permutation mapping for sorted access. Delegate manual slot sorting and column iteration to the new shared helper, eliminating per-format duplication and eager collection while preserving existing public API signatures. --- src/obicompactvec/src/bitmatrix/persistent.rs | 24 +---- src/obicompactvec/src/intmatrix.rs | 21 +--- src/obicompactvec/src/views.rs | 97 +++++++++++++------ 3 files changed, 77 insertions(+), 65 deletions(-) diff --git a/src/obicompactvec/src/bitmatrix/persistent.rs b/src/obicompactvec/src/bitmatrix/persistent.rs index 08956f5d..b6cc09c5 100644 --- a/src/obicompactvec/src/bitmatrix/persistent.rs +++ b/src/obicompactvec/src/bitmatrix/persistent.rs @@ -7,7 +7,7 @@ use crate::bitvec::{PersistentBitVec, PersistentBitVecBuilder}; use crate::layer_meta::LayerMeta; use crate::meta::MatrixMeta; use crate::traits::{BitPartials, ColumnWeights}; -use crate::views::BitSliceView; +use crate::views::{nonzero_triples, BitSliceView}; use super::columnar::ColumnarBitMatrix; use super::packed::PackedBitMatrix; @@ -199,10 +199,9 @@ impl PersistentBitMatrix { /// `Sparse` delegates to its native row-major decode (no `n_cols`-wide /// buffer, ever — the whole point, see /// `PersistentSparseBitMatrix::nonzero_iter`); `Columnar`/`Packed` - /// reuse the same sorted-slot batching `fill_sub_matrix` always used - /// (`BitSliceView::enumerate_nonzero_slots`), just eagerly collected here - /// rather than filled into a dense buffer — same `get` calls, same - /// mmap locality, no per-format duplication of that logic elsewhere. + /// reuse the shared sorted-slot batching in `views::nonzero_triples` + /// (same as `IntMatrix::nonzero_iter`) — same `get` calls, same mmap + /// locality, no per-format duplication of that logic elsewhere. /// `Implicit` is trivial (one column, always present). /// /// Boxed (not `impl Iterator`) because the match arms are genuinely @@ -219,20 +218,7 @@ impl PersistentBitMatrix { Box::new(slots.iter().enumerate().map(|(i, _)| (i, 0usize, 1u32))) } Self::Columnar(_) | Self::Packed(_) => { - let n = slots.len(); - let mut perm: Vec = (0..n).collect(); - perm.sort_by_key(|&i| slots[i]); - let sorted_slots: Vec = perm.iter().map(|&i| slots[i]).collect(); - let n_cols = self.n_cols(); - let mut hits: Vec<(usize, usize, u32)> = Vec::new(); - for c in 0..n_cols { - let view = self.col_view(c); - hits.extend( - view.enumerate_nonzero_slots(&sorted_slots) - .map(|pos| (perm[pos], c, 1u32)), - ); - } - Box::new(hits.into_iter()) + Box::new(nonzero_triples(slots, self.n_cols(), |c| self.col_view(c))) } } } diff --git a/src/obicompactvec/src/intmatrix.rs b/src/obicompactvec/src/intmatrix.rs index 1d4b3246..f26f3228 100644 --- a/src/obicompactvec/src/intmatrix.rs +++ b/src/obicompactvec/src/intmatrix.rs @@ -14,7 +14,7 @@ use crate::meta::MatrixMeta; use crate::reader::PersistentCompactIntVec; use crate::tempbitvec::{TempBitVec, TempBitVecBuilder}; use crate::tempintvec::{TempCompactIntVec, TempCompactIntVecBuilder}; -use crate::views::IntSliceView; +use crate::views::{nonzero_triples, IntSliceView}; fn col_path(dir: &Path, col: usize) -> PathBuf { dir.join(format!("col_{col:06}.pciv")) @@ -403,23 +403,10 @@ impl PersistentCompactIntMatrix { /// sparse row-major access" — counts explicitly not excluded from that /// design even though no sparse count format exists yet). No native /// low-effort case here the way `PersistentSparseBitMatrix` has one — - /// both variants reuse the same sorted-slot batching `fill_sub_matrix` - /// already used, via `IntSliceView::enumerate_nonzero_slots`. + /// both variants reuse the shared sorted-slot batching in + /// `views::nonzero_triples`. pub fn nonzero_iter<'a>(&'a self, slots: &'a [usize]) -> impl Iterator + 'a { - let n = slots.len(); - let mut perm: Vec = (0..n).collect(); - perm.sort_by_key(|&i| slots[i]); - let sorted_slots: Vec = perm.iter().map(|&i| slots[i]).collect(); - let n_cols = self.n_cols(); - let mut hits: Vec<(usize, usize, u32)> = Vec::new(); - for c in 0..n_cols { - hits.extend( - self.col_view(c) - .enumerate_nonzero_slots(&sorted_slots) - .map(|(pos, v)| (perm[pos], c, v)), - ); - } - hits.into_iter() + nonzero_triples(slots, self.n_cols(), |c| self.col_view(c)) } #[inline] diff --git a/src/obicompactvec/src/views.rs b/src/obicompactvec/src/views.rs index 013f74c1..dc80dc74 100644 --- a/src/obicompactvec/src/views.rs +++ b/src/obicompactvec/src/views.rs @@ -43,10 +43,8 @@ impl<'a> BitSliceView<'a> { let mut perm: Vec = (0..n).collect(); perm.sort_by_key(|&i| slots[i]); let sorted: Vec = perm.iter().map(|&i| slots[i]).collect(); - let mut tmp = vec![false; n]; - self.fill_slots_values_sorted(&sorted, &mut tmp); - for (i, &orig_idx) in perm.iter().enumerate() { - out[orig_idx] = tmp[i]; + for (i, v) in self.enumerate_slots_values(&sorted) { + out[perm[i]] = v; } } @@ -62,25 +60,20 @@ impl<'a> BitSliceView<'a> { slots.iter().enumerate().map(move |(pos, &slot)| (pos, self.get(slot))) } - /// Fill `out` assuming `sorted_slots` is already in ascending order. - /// Results are written in `sorted_slots` order (no reordering). - pub(crate) fn fill_slots_values_sorted(&self, sorted_slots: &[usize], out: &mut [bool]) { - assert_eq!(sorted_slots.len(), out.len()); - for (pos, v) in self.enumerate_slots_values(sorted_slots) { - out[pos] = v; - } - } - - /// Positions in `slots` (not the slots themselves) whose bit is set. + /// `(position in slots, 1)` for every slot whose bit is set — the value + /// is always `1u32` (a set bit has no other value to carry), kept in + /// the tuple only so this has the same shape as + /// [`IntSliceView::enumerate_nonzero_slots`] and both can feed the same + /// generic triple-collecting code (see `views::NonzeroSlotsView`). /// Order of `slots` never matters for correctness; callers batching /// several columns over the same slot set sort it once beforehand /// purely for cache-friendlier `get` access (see /// `PersistentBitMatrix::nonzero_iter`). - pub fn enumerate_nonzero_slots<'b>(self, slots: &'b [usize]) -> impl Iterator + 'b + pub fn enumerate_nonzero_slots<'b>(self, slots: &'b [usize]) -> impl Iterator + 'b where Self: 'b, { - self.enumerate_slots_values(slots).filter_map(|(pos, v)| v.then_some(pos)) + self.enumerate_slots_values(slots).filter_map(|(pos, v)| v.then_some((pos, 1u32))) } pub fn count_ones(&self) -> u64 { @@ -209,10 +202,8 @@ impl<'a> IntSliceView<'a> { let mut perm: Vec = (0..n).collect(); perm.sort_by_key(|&i| slots[i]); let sorted: Vec = perm.iter().map(|&i| slots[i]).collect(); - let mut tmp = vec![0u32; n]; - self.fill_slots_values_sorted(&sorted, &mut tmp); - for (i, &orig_idx) in perm.iter().enumerate() { - out[orig_idx] = tmp[i]; + for (i, v) in self.enumerate_slots_values(&sorted) { + out[perm[i]] = v; } } @@ -227,15 +218,6 @@ impl<'a> IntSliceView<'a> { slots.iter().enumerate().map(move |(pos, &slot)| (pos, self.get(slot))) } - /// Fill `out` assuming `sorted_slots` is already in ascending order. - /// Results are written in `sorted_slots` order (no reordering). - pub(crate) fn fill_slots_values_sorted(&self, sorted_slots: &[usize], out: &mut [u32]) { - assert_eq!(sorted_slots.len(), out.len()); - for (pos, v) in self.enumerate_slots_values(sorted_slots) { - out[pos] = v; - } - } - /// Like [`BitSliceView::enumerate_nonzero_slots`]: `(position in slots, /// value)` for every nonzero value — a `filter_map` over `get`, not a /// new traversal. @@ -370,6 +352,63 @@ impl<'a> IntSliceView<'a> { } } +// ── NonzeroSlotsView / nonzero_triples ────────────────────────────────────── + +/// Column views (`BitSliceView`, `IntSliceView`) that can enumerate their +/// nonzero slots as `(position in slots, value)`. Mirrors each type's own +/// inherent `enumerate_nonzero_slots` — exists only so [`nonzero_triples`] +/// can be written once and reused by every matrix type instead of +/// reimplementing the same sort-then-scan-columns loop per format. +pub(crate) trait NonzeroSlotsView: Sized { + fn enumerate_nonzero_slots<'b>(self, slots: &'b [usize]) -> impl Iterator + 'b + where + Self: 'b; +} + +impl<'a> NonzeroSlotsView for BitSliceView<'a> { + #[inline] + fn enumerate_nonzero_slots<'b>(self, slots: &'b [usize]) -> impl Iterator + 'b + where + Self: 'b, + { + BitSliceView::enumerate_nonzero_slots(self, slots) + } +} + +impl<'a> NonzeroSlotsView for IntSliceView<'a> { + #[inline] + fn enumerate_nonzero_slots<'b>(self, slots: &'b [usize]) -> impl Iterator + 'b + where + Self: 'b, + { + IntSliceView::enumerate_nonzero_slots(self, slots) + } +} + +/// Shared body of every matrix `nonzero_iter`: sort `slots` once, scan each +/// column with the sorted slice for mmap locality, remap positions back to +/// the caller's original order. Generic over the column view type so +/// `IntMatrix` and `PersistentBitMatrix` (Columnar/Packed) don't each carry +/// their own copy of this traversal. +pub(crate) fn nonzero_triples( + slots: &[usize], + n_cols: usize, + col_view: impl Fn(usize) -> V, +) -> impl Iterator { + let n = slots.len(); + let mut perm: Vec = (0..n).collect(); + perm.sort_by_key(|&i| slots[i]); + let sorted_slots: Vec = perm.iter().map(|&i| slots[i]).collect(); + let mut hits: Vec<(usize, usize, u32)> = Vec::new(); + for c in 0..n_cols { + hits.extend( + col_view(c).enumerate_nonzero_slots(&sorted_slots) + .map(|(pos, v)| (perm[pos], c, v)), + ); + } + hits.into_iter() +} + // ── IntSliceViewIter ────────────────────────────────────────────────────────── pub struct IntSliceViewIter<'a> {