Skip to content

Commit acbf808

Browse files
committed
rename to indivisible and remove memo thing
Signed-off-by: Adam Gutglick <adam@spiraldb.com>
1 parent 7abbe13 commit acbf808

6 files changed

Lines changed: 35 additions & 96 deletions

File tree

vortex-layout/src/children.rs

Lines changed: 16 additions & 76 deletions
Original file line numberDiff line numberDiff line change
@@ -38,13 +38,14 @@ pub trait LayoutChildren: 'static + Send + Sync {
3838

3939
fn nchildren(&self) -> usize;
4040

41-
/// Returns `true` if the child at `idx` is divisible: it may register split boundaries
42-
/// strictly inside its row range (see [`VTable::is_divisible`](crate::VTable::is_divisible)).
41+
/// Returns `true` if the child at `idx` is known — without materializing it — to be
42+
/// indivisible: it registers no split boundaries strictly inside its row range (see
43+
/// [`VTable::is_indivisible`](crate::VTable::is_indivisible)).
4344
///
44-
/// Implementations must conservatively return `true` when answering would require
45+
/// Implementations must conservatively return `false` when answering would require
4546
/// materializing the child.
46-
fn child_is_divisible(&self, _idx: usize) -> bool {
47-
true
47+
fn child_is_indivisible(&self, _idx: usize) -> bool {
48+
false
4849
}
4950
}
5051

@@ -73,8 +74,8 @@ impl LayoutChildren for Arc<dyn LayoutChildren> {
7374
self.as_ref().nchildren()
7475
}
7576

76-
fn child_is_divisible(&self, idx: usize) -> bool {
77-
self.as_ref().child_is_divisible(idx)
77+
fn child_is_indivisible(&self, idx: usize) -> bool {
78+
self.as_ref().child_is_indivisible(idx)
7879
}
7980
}
8081

@@ -118,8 +119,8 @@ impl LayoutChildren for OwnedLayoutChildren {
118119
self.0.len()
119120
}
120121

121-
fn child_is_divisible(&self, idx: usize) -> bool {
122-
self.0[idx].dyn_is_divisible()
122+
fn child_is_indivisible(&self, idx: usize) -> bool {
123+
self.0[idx].dyn_is_indivisible()
123124
}
124125
}
125126

@@ -133,7 +134,6 @@ pub(crate) struct ViewedLayoutChildren {
133134
allow_unknown: bool,
134135
session: VortexSession,
135136
cache: Arc<[OnceCell<LayoutRef>]>,
136-
divisibility_memo: DivisibilityMemo,
137137
}
138138

139139
impl ViewedLayoutChildren {
@@ -166,7 +166,6 @@ impl ViewedLayoutChildren {
166166
allow_unknown,
167167
session,
168168
cache,
169-
divisibility_memo: DivisibilityMemo::empty(),
170169
}
171170
}
172171

@@ -292,85 +291,26 @@ impl LayoutChildren for ViewedLayoutChildren {
292291
self.cache.len()
293292
}
294293

295-
fn child_is_divisible(&self, idx: usize) -> bool {
294+
fn child_is_indivisible(&self, idx: usize) -> bool {
296295
if idx >= self.nchildren() {
297-
return true;
296+
return false;
298297
}
299298
// Resolve the child's layout encoding from the flatbuffer tag alone, without
300-
// deserializing the child layout. Unknown encodings are conservatively divisible so
301-
// callers fall back to materializing the child.
299+
// deserializing the child layout. Unknown encodings are conservatively not indivisible
300+
// so callers fall back to materializing the child.
302301
let encoding = self
303302
.flatbuffer()
304303
.children()
305304
.unwrap_or_default()
306305
.get(idx)
307306
.encoding();
308307

309-
if let Some(answer) = self.divisibility_memo.get(encoding) {
310-
return answer;
311-
}
312-
313308
let answer = self
314309
.layout_read_ctx
315310
.resolve(encoding)
316311
.and_then(|encoding_id| self.layouts.get(&encoding_id))
317-
.is_none_or(|encoding| encoding.is_divisible());
318-
self.divisibility_memo.set(encoding, answer);
319-
answer
320-
}
321-
}
312+
.is_some_and(|encoding| encoding.is_indivisible());
322313

323-
/// Single-slot cache for [`ViewedLayoutChildren::child_is_divisible`] answers, keyed by the
324-
/// child's flatbuffer encoding tag and shared across clones of the owning
325-
/// [`ViewedLayoutChildren`].
326-
///
327-
/// Divisibility depends only on the child's layout encoding, and children of one layout node
328-
/// almost always share an encoding — so caching the answer for the last-seen tag turns the
329-
/// per-child read-context resolve and registry lookup into a single atomic load for all but the
330-
/// first child.
331-
///
332-
/// The tag, the answer, and a validity flag are packed into one `AtomicU32` so lookups and
333-
/// updates are each a single atomic operation, with no locking and no torn state between the key
334-
/// and its answer:
335-
///
336-
/// ```text
337-
/// bit: 17 16 15..0
338-
/// valid | answer | encoding tag
339-
/// ```
340-
///
341-
/// `Relaxed` ordering suffices throughout: this is purely a performance cache, and each packed
342-
/// word is internally consistent on its own. The worst a racing `get`/`set` can cause is a
343-
/// redundant recomputation.
344-
#[derive(Clone)]
345-
struct DivisibilityMemo(Arc<AtomicU32>);
346-
347-
impl DivisibilityMemo {
348-
/// Low 16 bits: the flatbuffer encoding tag the cached answer belongs to.
349-
const TAG: u32 = 0xFFFF;
350-
/// Bit 16: the cached answer for the stored tag.
351-
const ANSWER: u32 = 1 << 16;
352-
/// Bit 17: set once the memo holds an entry. Needed because the atomic starts at zero, which
353-
/// would otherwise be indistinguishable from a cached `(tag 0, answer false)` entry.
354-
const VALID: u32 = 1 << 17;
355-
356-
/// Create a memo holding no entry.
357-
fn empty() -> Self {
358-
Self(Arc::new(AtomicU32::new(0)))
359-
}
360-
361-
/// Return the cached answer for `tag`, or `None` if the memo is empty or holds a different
362-
/// tag.
363-
fn get(&self, tag: u16) -> Option<bool> {
364-
let memo = self.0.load(Ordering::Relaxed);
365-
(memo & Self::VALID != 0 && memo & Self::TAG == u32::from(tag))
366-
.then_some(memo & Self::ANSWER != 0)
367-
}
368-
369-
/// Cache `answer` for `tag`, replacing any previous entry.
370-
fn set(&self, tag: u16, answer: bool) {
371-
self.0.store(
372-
u32::from(tag) | Self::VALID | if answer { Self::ANSWER } else { 0 },
373-
Ordering::Relaxed,
374-
);
314+
answer
375315
}
376316
}

vortex-layout/src/encoding.rs

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -68,10 +68,10 @@ pub trait LayoutVTablePlugin: 'static + Send + Sync + Debug {
6868
build_ctx: &LayoutBuildContext<'_>,
6969
) -> VortexResult<LayoutRef>;
7070

71-
/// Returns `true` if this layout is divisible: its readers may register natural split
72-
/// boundaries strictly inside their row range (see [`VTable::is_divisible`]).
73-
fn is_divisible(&self) -> bool {
74-
true
71+
/// Returns `true` if this layout is indivisible: its readers never register natural split
72+
/// boundaries strictly inside their row range (see [`VTable::is_indivisible`]).
73+
fn is_indivisible(&self) -> bool {
74+
false
7575
}
7676
}
7777

@@ -109,8 +109,8 @@ impl<V: VTable> LayoutVTablePlugin for V {
109109
.into_layout())
110110
}
111111

112-
fn is_divisible(&self) -> bool {
113-
VTable::is_divisible(self)
112+
fn is_indivisible(&self) -> bool {
113+
VTable::is_indivisible(self)
114114
}
115115
}
116116

vortex-layout/src/layout.rs

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -268,10 +268,10 @@ pub trait DynLayout: 'static + Send + Sync + Debug {
268268
ctx: &LayoutReaderContext,
269269
) -> VortexResult<LayoutReaderRef>;
270270

271-
/// Returns `true` if this layout is divisible: its readers may register natural split
272-
/// boundaries strictly inside their row range (see [`crate::VTable::is_divisible`]).
273-
fn dyn_is_divisible(&self) -> bool {
274-
true
271+
/// Returns `true` if this layout is indivisible: its readers never register natural split
272+
/// boundaries strictly inside their row range (see [`crate::VTable::is_indivisible`]).
273+
fn dyn_is_indivisible(&self) -> bool {
274+
false
275275
}
276276
}
277277

@@ -330,8 +330,8 @@ impl<V: VTable> DynLayout for Layout<V> {
330330
Layout::new_reader(self, name, segment_source, session, ctx)
331331
}
332332

333-
fn dyn_is_divisible(&self) -> bool {
334-
self.vtable().is_divisible()
333+
fn dyn_is_indivisible(&self) -> bool {
334+
self.vtable().is_indivisible()
335335
}
336336
}
337337

vortex-layout/src/layouts/chunked/reader.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -118,7 +118,7 @@ impl ChunkedReader {
118118
return ChunkSkips::None;
119119
}
120120
let skips = (0..nchildren)
121-
.map(|idx| !children.child_is_divisible(idx))
121+
.map(|idx| children.child_is_indivisible(idx))
122122
.collect::<Box<[bool]>>();
123123
if skips.iter().all(|&skip| skip) {
124124
ChunkSkips::All

vortex-layout/src/layouts/flat/mod.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -67,8 +67,8 @@ impl VTable for Flat {
6767

6868
/// Flat readers only ever register the end of the requested range, so flat layouts are
6969
/// indivisible and split collection can skip materializing flat children.
70-
fn is_divisible(&self) -> bool {
71-
false
70+
fn is_indivisible(&self) -> bool {
71+
true
7272
}
7373

7474
fn metadata(layout: &Layout<Self>) -> Self::Metadata {

vortex-layout/src/vtable.rs

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -114,13 +114,12 @@ pub trait VTable: 'static + Clone + Send + Sync + Debug {
114114
ctx: &LayoutReaderContext,
115115
) -> VortexResult<LayoutReaderRef>;
116116

117-
/// Returns `true` if this layout is divisible: its readers may register natural split
117+
/// Returns `true` if this layout is indivisible: its readers never register natural split
118118
/// boundaries strictly inside their row range (see [`crate::LayoutReader::register_splits`]).
119119
///
120120
/// Indivisible layouts — like flat, whose readers only ever push the end of the requested
121-
/// range — return `false`, which lets parent layouts skip materializing the child entirely
122-
/// during split collection.
123-
fn is_divisible(&self) -> bool {
124-
true
121+
/// range — let parent layouts skip materializing the child entirely during split collection.
122+
fn is_indivisible(&self) -> bool {
123+
false
125124
}
126125
}

0 commit comments

Comments
 (0)