Skip to content

Commit d65a9da

Browse files
committed
final touches
Signed-off-by: Connor Tsui <connor.tsui20@gmail.com>
1 parent 6f16c60 commit d65a9da

4 files changed

Lines changed: 69 additions & 33 deletions

File tree

‎vortex-array/src/scalar_fn/unstable/row/execute/owned.rs‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ use crate::scalar_fn::ExecutionArgs;
1919
use crate::scalar_fn::unstable::row::FailureEvidence;
2020
use crate::scalar_fn::unstable::row::IndexedElementTuple;
2121
use crate::scalar_fn::unstable::row::OutputElement;
22+
use crate::scalar_fn::unstable::row::ViewLen;
2223
use crate::scalar_fn::unstable::row::visitor::assert_owned_output_needs_no_drop;
2324

2425
/// Zero-sized failure accumulator for infallible owned visits.
@@ -77,19 +78,19 @@ where
7778
let failure = if let Some(views) = Args::views_if_no_consts(&columns) {
7879
// Keep this validation beside the views so LLVM sees their common length here.
7980
vortex_ensure!(
80-
Args::view_lens_match(&views, row_count),
81+
Args::ARITY == 0 || views.len() == row_count,
8182
"a decoded row input does not address exactly {row_count} rows",
8283
);
8384

84-
// SAFETY: `view_lens_match` proved every view addresses exactly `row_count` rows
85-
// immediately above.
85+
// SAFETY: the tuple length check proved every non-nullary view addresses exactly
86+
// `row_count` rows immediately above. Nullary tuples do not access an input view.
8687
let source = unsafe { Args::indexed_source(views, row_count) };
8788

8889
source.map_checked_into(output, |elements| apply(&prepared, elements))
8990
} else {
9091
// Keep this proof branch-local. Shared validation prevents LLVM from specializing this
9192
// loop for each batch-constant arrangement, leaving it scalar under multiple CGUs without
92-
// LTO. The exact pass interaction is unknown.
93+
// LTO.
9394
vortex_ensure!(
9495
Args::decoded_lens_match(&columns, row_count),
9596
"a decoded row input does not address exactly {row_count} rows",

‎vortex-array/src/scalar_fn/unstable/row/execute/sink.rs‎

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,6 @@ where
3939
ApplyResult: SinkResult<WriteToken = <Sink as OutputSink<Options>>::WriteToken>,
4040
{
4141
let columns = Args::decode(args, ctx)?;
42-
let views = Args::views_if_no_consts(&columns);
4342

4443
let row_count = args.row_count();
4544
let const_values = Args::const_values(&columns);
@@ -59,13 +58,15 @@ where
5958
"the output sink must address exactly {row_count} rows, got {sink_row_count}",
6059
);
6160

61+
let views = Args::views_if_no_consts(&columns);
6262
if let Some(views) = views {
63-
if !Args::view_lens_match(&views, row_count) {
63+
if Args::ARITY != 0 && views.len() != row_count {
6464
decoded_length_error(row_count)?;
6565
}
6666

6767
for index in 0..row_count {
68-
// SAFETY: `view_lens_match` proved every view has `row_count` rows before the loop.
68+
// SAFETY: the tuple length check proved every non-nullary view has `row_count`
69+
// rows before the loop. Nullary tuples do not access an input view.
6970
let elements = unsafe { Args::get_from_views_unchecked(&views, index) };
7071
// SAFETY: the sink row-count check above proved every loop index is in bounds.
7172
let output =
@@ -143,7 +144,7 @@ where
143144
);
144145

145146
if let Some(views) = views {
146-
if !Args::view_lens_match(&views, row_count) {
147+
if Args::ARITY != 0 && views.len() != row_count {
147148
decoded_length_error(row_count)?;
148149
}
149150

@@ -153,8 +154,9 @@ where
153154
let output =
154155
unsafe { <Sink as OutputSink<Options>>::row_unchecked(&mut rows, index) };
155156

156-
// SAFETY: `view_lens_match` proved every view has `row_count` rows, and mask
157-
// indices are below `row_count`.
157+
// SAFETY: the tuple length check proved every non-nullary view has `row_count`
158+
// rows, and mask indices are below `row_count`. Nullary tuples do not access an
159+
// input view.
158160
let elements = unsafe { Args::get_from_views_unchecked(&views, index) };
159161

160162
apply(&prepared, elements, output).into_result()

‎vortex-array/src/scalar_fn/unstable/row/types/element/tuple/element_tuple.rs‎

Lines changed: 8 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -148,7 +148,7 @@ pub trait ElementTuple: 'static + private::Sealed {
148148
type Columns;
149149

150150
/// Borrowed views of decoded columns with no batch constants.
151-
type Views<'a>;
151+
type Views<'a>: ViewLen;
152152

153153
/// The borrowed row of element values.
154154
type Elems<'a>;
@@ -206,19 +206,15 @@ pub trait ElementTuple: 'static + private::Sealed {
206206
/// returned tuple removes batch-constant checks from the row loop.
207207
fn views_if_no_consts(columns: &Self::Columns) -> Option<Self::Views<'_>>;
208208

209-
/// Whether every view contains exactly `row_count` rows.
210-
///
211-
/// The executor calls this once before the loop used when no input is batch-constant. A
212-
/// successful check gives LLVM a dominating equality between the loop bound and every source
213-
/// length, which lets it optimize the tuple access as one fixed-length traversal.
214-
fn view_lens_match(views: &Self::Views<'_>, row_count: usize) -> bool;
215-
216209
/// Whether every argument decoded at full batch length contains exactly `row_count` rows.
217210
///
218-
/// This is the equivalent of [`view_lens_match`](Self::view_lens_match) when the columns include
219-
/// batch constants. It runs once before the hot loop for the same LLVM optimization. A batch
220-
/// constant is exempt because its `ArgColumn` constructor already validated the one-row
221-
/// representation produced by decoding.
211+
/// NB: `Columns` cannot implement [`ViewLen`] because a batch-constant [`ArgColumn`] physically
212+
/// contains one decoded row while logically addressing every row in the batch. By contrast,
213+
/// [`views_if_no_consts`](Self::views_if_no_consts) constructs `Views` only when every argument
214+
/// is non-constant, so those views have one common physical length.
215+
///
216+
/// This check runs once before the hot loop. A batch constant is exempt because its
217+
/// [`ArgColumn`] constructor already validated the one-row representation produced by decoding.
222218
fn decoded_lens_match(columns: &Self::Columns, row_count: usize) -> bool;
223219

224220
/// Read one row from borrowed views.
@@ -285,10 +281,6 @@ impl ElementTuple for () {
285281
Some(())
286282
}
287283

288-
fn view_lens_match(_views: &Self::Views<'_>, _row_count: usize) -> bool {
289-
true
290-
}
291-
292284
fn decoded_lens_match(_columns: &Self::Columns, _row_count: usize) -> bool {
293285
true
294286
}
@@ -369,13 +361,6 @@ macro_rules! element_tuple {
369361
Some(($($t::view(columns.$idx.as_column()?),)+))
370362
}
371363

372-
fn view_lens_match(
373-
views: &Self::Views<'_>,
374-
row_count: usize,
375-
) -> bool {
376-
$(views.$idx.len() == row_count &&)+ true
377-
}
378-
379364
fn decoded_lens_match(columns: &Self::Columns, row_count: usize) -> bool {
380365
$(columns.$idx.addresses_rows(row_count) &&)+ true
381366
}

‎vortex-array/src/scalar_fn/unstable/row/types/view.rs‎

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,3 +54,51 @@ impl<T: ViewLen + ?Sized> ViewLen for &mut T {
5454
T::len(self)
5555
}
5656
}
57+
58+
macro_rules! impl_tuple_view_len {
59+
($first:ident; $($rest:ident : $idx:tt),*) => {
60+
impl<$first: ViewLen, $($rest: ViewLen),*> ViewLen for ($first, $($rest,)*) {
61+
fn len(&self) -> usize {
62+
let len = self.0.len();
63+
$(assert_eq!(self.$idx.len(), len, "tuple views must have equal lengths");)*
64+
65+
len
66+
}
67+
}
68+
};
69+
}
70+
71+
impl_tuple_view_len!(A;);
72+
impl_tuple_view_len!(A; B: 1);
73+
impl_tuple_view_len!(A; B: 1, C: 2);
74+
impl_tuple_view_len!(A; B: 1, C: 2, D: 3);
75+
impl_tuple_view_len!(A; B: 1, C: 2, D: 3, E: 4);
76+
impl_tuple_view_len!(A; B: 1, C: 2, D: 3, E: 4, F: 5);
77+
impl_tuple_view_len!(A; B: 1, C: 2, D: 3, E: 4, F: 5, G: 6);
78+
impl_tuple_view_len!(A; B: 1, C: 2, D: 3, E: 4, F: 5, G: 6, H: 7);
79+
impl_tuple_view_len!(A; B: 1, C: 2, D: 3, E: 4, F: 5, G: 6, H: 7, I: 8);
80+
impl_tuple_view_len!(A; B: 1, C: 2, D: 3, E: 4, F: 5, G: 6, H: 7, I: 8, J: 9);
81+
impl_tuple_view_len!(A; B: 1, C: 2, D: 3, E: 4, F: 5, G: 6, H: 7, I: 8, J: 9, K: 10);
82+
impl_tuple_view_len!(A; B: 1, C: 2, D: 3, E: 4, F: 5, G: 6, H: 7, I: 8, J: 9, K: 10, L: 11);
83+
84+
#[cfg(test)]
85+
mod tests {
86+
use super::ViewLen;
87+
88+
#[test]
89+
fn tuple_len_returns_common_len() {
90+
let first: &[i64] = &[1, 2];
91+
let second: &[i64] = &[3, 4];
92+
93+
assert_eq!((first, second).len(), 2);
94+
}
95+
96+
#[test]
97+
#[should_panic(expected = "tuple views must have equal lengths")]
98+
fn tuple_len_rejects_mismatch() {
99+
let first: &[i64] = &[1];
100+
let second: &[i64] = &[2, 3];
101+
102+
let _ = (first, second).len();
103+
}
104+
}

0 commit comments

Comments
 (0)