Skip to content

Commit cb4bf48

Browse files
authored
Merge pull request #288 from AdaWorldAPI/claude/medcare-rs-continue-6nhbxn
ogar-r2il: CallMask stores its words inline, and gains set-bit iteration
2 parents d1116c1 + e754208 commit cb4bf48

1 file changed

Lines changed: 176 additions & 31 deletions

File tree

‎crates/ogar-r2il/src/lib.rs‎

Lines changed: 176 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -63,10 +63,11 @@
6363
//!
6464
//! Reads like a filtered query, executes as word-tests over a bitmask; no
6565
//! `Vec<Call>` is built and no call outside the mask is ever decoded. The
66-
//! mask is [`CallMask`] — `Box<[u64]>` over call indices, with the same
67-
//! algebra and the same single named materializer as every other mask plane
68-
//! in this workspace, because "a `long[]` of selected ids is still a
69-
//! materialised population" applies here identically.
66+
//! mask is [`CallMask`] — an inline `[u64; 3]` over call indices (three
67+
//! words, no allocation, cover every [`LaneShape`]'s call population), with
68+
//! the same algebra and the same single named materializer as every other
69+
//! mask plane in this workspace, because "a `long[]` of selected ids is
70+
//! still a materialised population" applies here identically.
7071
//!
7172
//! **Reshuffling is a re-READ, never a re-WRITE.** The same slab under two
7273
//! shapes yields two different call streams from the same unchanged bytes —
@@ -357,17 +358,37 @@ impl R2ILVocabulary {
357358
}
358359
}
359360

361+
/// Inline word count for [`CallMask`]'s bitset.
362+
///
363+
/// Derived, not chosen: the widest [`LaneShape`] is `Pairs`, at
364+
/// `180 = LaneShape::Pairs.calls_per_lane() * ogar_loco::CONTENT_SLOTS` call
365+
/// slots (pinned by the
366+
/// `masks_of_different_shapes_range_over_different_populations` test), and
367+
/// `180.div_ceil(64) == 3`. `Triples` (120) and `Quads` (90) both need fewer
368+
/// words and simply leave the high word(s) unused — always zero, since
369+
/// [`CallMask::set`] only ever touches a word index `< len.div_ceil(64)`.
370+
/// Three inline `u64` words therefore cover every shape with no allocation.
371+
const MASK_WORDS: usize = 3;
372+
360373
/// A population of CALL INDICES within one body, as words.
361374
///
362375
/// The same law as every other mask plane here: reads like a selection,
363376
/// executes as word ops, and leaves mask form only through the one named
364-
/// materializer. Call indices are shape-relative — a mask built under
365-
/// `Pairs` indexes a different population than the same bits under `Quads` —
366-
/// which is why [`project`] takes both and why [`CallMask::shape`] is
367-
/// carried rather than inferred.
368-
#[derive(Clone, Debug, PartialEq, Eq)]
377+
/// materializer ([`CallMask::materialize_indices`]) — the lazy
378+
/// [`CallMask::set_indices`] iterator is not a second exit, it stays inside
379+
/// the same word-scan discipline and only turns into data when a caller
380+
/// chooses to collect it. Call indices are shape-relative — a mask built
381+
/// under `Pairs` indexes a different population than the same bits under
382+
/// `Quads` — which is why [`project`] takes both and why [`CallMask::shape`]
383+
/// is carried rather than inferred.
384+
///
385+
/// Backed by [`MASK_WORDS`] inline `u64` words rather than a `Box<[u64]>`:
386+
/// every [`LaneShape`]'s call population fits in three words with room to
387+
/// spare, so a boxed, heap-allocated slice would only be paying for an
388+
/// allocation the largest shape never needs.
389+
#[derive(Clone, Copy, Debug, PartialEq, Eq)]
369390
pub struct CallMask {
370-
words: Box<[u64]>,
391+
words: [u64; MASK_WORDS],
371392
len: u32,
372393
shape: LaneShape,
373394
}
@@ -377,8 +398,12 @@ impl CallMask {
377398
#[must_use]
378399
pub fn empty(shape: LaneShape) -> Self {
379400
let len = shape.calls_per_lane() * ogar_loco::CONTENT_SLOTS;
401+
debug_assert!(
402+
len.div_ceil(64) <= MASK_WORDS,
403+
"shape needs more than MASK_WORDS={MASK_WORDS} inline words"
404+
);
380405
Self {
381-
words: vec![0u64; len.div_ceil(64)].into_boxed_slice(),
406+
words: [0u64; MASK_WORDS],
382407
len: u32::try_from(len).unwrap_or(u32::MAX),
383408
shape,
384409
}
@@ -434,12 +459,7 @@ impl CallMask {
434459
fn zip(&self, other: &Self, f: impl Fn(u64, u64) -> u64) -> Self {
435460
debug_assert_eq!(self.shape, other.shape, "masks of different shapes");
436461
Self {
437-
words: self
438-
.words
439-
.iter()
440-
.zip(other.words.iter())
441-
.map(|(&a, &b)| f(a, b))
442-
.collect(),
462+
words: std::array::from_fn(|i| f(self.words[i], other.words[i])),
443463
len: self.len,
444464
shape: self.shape,
445465
}
@@ -468,30 +488,78 @@ impl CallMask {
468488

469489
/// Complement WITHIN the shape's call count.
470490
///
471-
/// The tail word's bits past `len` are cleared: a complement that keeps
472-
/// them invents call slots the body does not have, and every downstream
473-
/// count and projection silently inflates.
491+
/// Every bit at or past `len` is cleared, word by word — not just in the
492+
/// physically-last of [`MASK_WORDS`] inline words. With fixed inline
493+
/// storage, a shape narrower than `Pairs` (e.g. `Triples`, `Quads`) never
494+
/// touches the high word(s) at all, so a plain `!w` over every word would
495+
/// flip those always-zero, never-meant-to-be-addressed words to all-ones
496+
/// — phantom call slots the body does not have, inflating every
497+
/// downstream count and projection. Clearing per-word by each word's own
498+
/// bit range (fully out of range → zeroed; straddling `len` → masked to
499+
/// its in-range bits; fully in range → left untouched) is what keeps this
500+
/// correct for every shape, not only the widest one.
474501
#[must_use]
475502
pub fn not(&self) -> Self {
476-
let mut out = Self {
477-
words: self.words.iter().map(|&w| !w).collect(),
503+
let mut words: [u64; MASK_WORDS] = std::array::from_fn(|i| !self.words[i]);
504+
for (word_idx, w) in words.iter_mut().enumerate() {
505+
let word_start = (word_idx as u32) * 64;
506+
if word_start >= self.len {
507+
*w = 0;
508+
} else if word_start + 64 > self.len {
509+
let bits_in_word = self.len - word_start;
510+
*w &= (1u64 << bits_in_word) - 1;
511+
}
512+
}
513+
Self {
514+
words,
478515
len: self.len,
479516
shape: self.shape,
480-
};
481-
let tail = u64::from(self.len % 64);
482-
if tail != 0
483-
&& let Some(last) = out.words.last_mut()
484-
{
485-
*last &= (1u64 << tail) - 1;
486517
}
487-
out
518+
}
519+
520+
/// Set bit indices, ascending, as a lazy iterator.
521+
///
522+
/// One `trailing_zeros` + `w &= w - 1` per set bit, word by word — O(3 +
523+
/// popcount) rather than [`materialize_indices`](Self::materialize_indices)'s
524+
/// O(len) per-index `contains` scan. This is the lazy sibling, not a
525+
/// second exit from mask form: nothing is collected until a caller
526+
/// chooses to. `materialize_indices` is built on top of it.
527+
///
528+
/// The trailing `take_while` is a defensive bound, not a hot path: by
529+
/// construction no bit is ever set at an index `>= len` (`set` guards it,
530+
/// and `not` now clears per-word instead of assuming the tail lives in
531+
/// the last of [`MASK_WORDS`] words), so this only ever matters if that
532+
/// invariant is ever broken elsewhere.
533+
//
534+
// No `#[must_use]`: `impl Iterator` already carries it, and stacking a
535+
// second bare one is `clippy::double_must_use` (CI runs `-D warnings`).
536+
pub fn set_indices(&self) -> impl Iterator<Item = u32> + '_ {
537+
let len = self.len;
538+
self.words
539+
.iter()
540+
.enumerate()
541+
.flat_map(|(word_idx, &word)| {
542+
let base = (word_idx as u32) * 64;
543+
let mut w = word;
544+
std::iter::from_fn(move || {
545+
if w == 0 {
546+
None
547+
} else {
548+
let bit = w.trailing_zeros();
549+
w &= w - 1;
550+
Some(base + bit)
551+
}
552+
})
553+
})
554+
.take_while(move |&i| i < len)
488555
}
489556

490557
/// **The named materializer** — call indices out, ascending. O(n), and
491-
/// the only exit from mask form.
558+
/// the only *eager* exit from mask form (see [`Self::set_indices`] for
559+
/// the lazy view this is built on).
492560
#[must_use]
493561
pub fn materialize_indices(&self) -> Vec<u32> {
494-
(0..self.len).filter(|&i| self.contains(i)).collect()
562+
self.set_indices().collect()
495563
}
496564
}
497565

@@ -790,4 +858,81 @@ mod tests {
790858
);
791859
assert_eq!(r2il_mask(&mixed, LaneShape::Pairs).count(), 1);
792860
}
861+
862+
/// `CallMask` is exactly its three inline fields, padded to its own
863+
/// alignment — no `Box<[u64]>` (pointer + length) hiding inside it. The
864+
/// expected size is computed from the field layout itself rather than a
865+
/// hardcoded byte count, so this stays correct however `LaneShape` is
866+
/// represented.
867+
#[test]
868+
fn call_mask_stores_its_words_inline_not_boxed() {
869+
let words_bytes = std::mem::size_of::<[u64; MASK_WORDS]>();
870+
let len_bytes = std::mem::size_of::<u32>();
871+
let shape_bytes = std::mem::size_of::<LaneShape>();
872+
let raw_sum = words_bytes + len_bytes + shape_bytes;
873+
let align = std::mem::align_of::<CallMask>();
874+
let expected = raw_sum.div_ceil(align) * align;
875+
assert_eq!(
876+
std::mem::size_of::<CallMask>(),
877+
expected,
878+
"CallMask must be exactly its inline fields (padded to its own \
879+
alignment) — a Box<[u64]> backing would additionally cost a \
880+
pointer plus a length"
881+
);
882+
// A CallMask is trivially Copy-able data now — no field left behind
883+
// that would make Copy unsound.
884+
let m = CallMask::empty(LaneShape::Pairs);
885+
let copy = m;
886+
assert_eq!(m, copy, "CallMask must remain usable after being Copy'd");
887+
}
888+
889+
/// [`CallMask::set_indices`] yields exactly the set-bit indices,
890+
/// ascending — for an empty mask (silence), a single-bit mask, and a
891+
/// full mask (every index the shape admits), across every `LaneShape`.
892+
#[test]
893+
fn set_indices_yields_exactly_the_set_bit_indices() {
894+
for shape in LaneShape::ALL {
895+
// Empty: yields nothing.
896+
let empty = CallMask::empty(shape);
897+
assert_eq!(
898+
empty.set_indices().collect::<Vec<u32>>(),
899+
Vec::<u32>::new(),
900+
"{shape:?}: empty mask must iterate to nothing"
901+
);
902+
903+
// A handful of scattered bits, including one that straddles the
904+
// word boundary at index 64 and (for Pairs) one near the tail.
905+
let mut m = CallMask::empty(shape);
906+
let candidates = [0u32, 1, 63, 64, 65, m.len().saturating_sub(1)];
907+
let mut expected: Vec<u32> = candidates
908+
.into_iter()
909+
.filter(|&i| i < m.len())
910+
.collect::<std::collections::BTreeSet<u32>>()
911+
.into_iter()
912+
.collect();
913+
expected.sort_unstable();
914+
for &i in &expected {
915+
m.set(i);
916+
}
917+
assert_eq!(
918+
m.set_indices().collect::<Vec<u32>>(),
919+
expected,
920+
"{shape:?}: scattered bits must iterate ascending, exactly the set ones"
921+
);
922+
assert_eq!(
923+
m.materialize_indices(),
924+
expected,
925+
"{shape:?}: materialize_indices must agree with set_indices"
926+
);
927+
928+
// Full: every index the shape admits, ascending, none skipped.
929+
let full = CallMask::all(shape);
930+
let got_full: Vec<u32> = full.set_indices().collect();
931+
let want_full: Vec<u32> = (0..full.len()).collect();
932+
assert_eq!(
933+
got_full, want_full,
934+
"{shape:?}: full mask must yield 0..len"
935+
);
936+
}
937+
}
793938
}

0 commit comments

Comments
 (0)