Skip to content

Commit d8fa2b6

Browse files
authored
Faster ScalarFnArray creation, lazy ArrayStats (#10329)
ScalarFnArray creation now uses 1 allocation instead of 3. ArrayStats are now lazy and aren't constructed unless read or written Signed-off-by: Mikhail Kot <mikhail@spiraldb.com>
1 parent 5cfc0b5 commit d8fa2b6

26 files changed

Lines changed: 207 additions & 96 deletions

File tree

‎vortex-array/src/array/mod.rs‎

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -425,7 +425,7 @@ impl<V: VTable> DynArrayData for ArrayData<V> {
425425
slots,
426426
);
427427
let encoding_id = self.vtable.id();
428-
let stats = this.statistics().to_array_stats();
428+
let stats = this.statistics().share_existing();
429429
let uninit = Arc::new_uninit();
430430

431431
// SAFETY: `uninit` is new.
@@ -492,7 +492,7 @@ impl<V: VTable> DynArrayData for ArrayData<V> {
492492
fn execute(&self, this: ArrayRef, ctx: &mut ExecutionCtx) -> VortexResult<ExecutionResult> {
493493
let len = this.len();
494494
let dtype = this.dtype().clone();
495-
let stats = this.statistics().to_array_stats();
495+
let stats = this.statistics().to_owned();
496496
let result = unsafe { self.execute_unchecked(this, ctx)? };
497497

498498
if matches!(result.step(), ExecutionStep::Done) {
@@ -511,10 +511,7 @@ impl<V: VTable> DynArrayData for ArrayData<V> {
511511
);
512512
}
513513

514-
result
515-
.array()
516-
.statistics()
517-
.set_iter(crate::stats::StatsSet::from(stats).into_iter());
514+
result.array().statistics().set_iter(stats.into_iter());
518515
}
519516

520517
Ok(result)

‎vortex-array/src/arrays/scalar_fn/array.rs‎

Lines changed: 59 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
use std::fmt::Display;
55
use std::fmt::Formatter;
66

7+
use smallvec::SmallVec;
78
use vortex_error::VortexExpect;
89
use vortex_error::VortexResult;
910
use vortex_error::vortex_bail;
@@ -15,6 +16,7 @@ use crate::array::Array;
1516
use crate::array::ArrayParts;
1617
use crate::array::TypedArrayRef;
1718
use crate::arrays::ScalarFn;
19+
use crate::dtype::DType;
1820
use crate::scalar_fn::ScalarFnRef;
1921

2022
// ScalarFnArray has a variable number of slots (one per child)
@@ -74,9 +76,13 @@ impl<T: TypedArrayRef<ScalarFn>> ScalarFnArrayExt for T {}
7476

7577
impl Array<ScalarFn> {
7678
/// Create a new ScalarFnArray from a scalar function and its children.
77-
pub fn try_new(scalar_fn: ScalarFnRef, children: Vec<ArrayRef>) -> VortexResult<Self> {
78-
let len = Self::infer_len(&children)?;
79-
Self::try_new_with_len(scalar_fn, children, len)
79+
pub fn try_new(
80+
scalar_fn: ScalarFnRef,
81+
children: impl IntoIterator<Item = ArrayRef>,
82+
) -> VortexResult<Self> {
83+
let slots: ArraySlots = children.into_iter().map(Some).collect();
84+
let len = Self::infer_len(&slots)?;
85+
Self::try_new_from_slots(scalar_fn, slots, len)
8086
}
8187

8288
/// Create a new ScalarFnArray from a scalar function, children, and an explicit length.
@@ -85,37 +91,61 @@ impl Array<ScalarFn> {
8591
/// child array to infer the length from.
8692
pub fn try_new_with_len(
8793
scalar_fn: ScalarFnRef,
88-
children: Vec<ArrayRef>,
94+
children: impl IntoIterator<Item = ArrayRef>,
95+
len: usize,
96+
) -> VortexResult<Self> {
97+
let slots: ArraySlots = children.into_iter().map(Some).collect();
98+
Self::try_new_from_slots(scalar_fn, slots, len)
99+
}
100+
101+
#[inline]
102+
fn try_new_from_slots(
103+
scalar_fn: ScalarFnRef,
104+
slots: ArraySlots,
89105
len: usize,
90106
) -> VortexResult<Self> {
91-
Self::validate_arity(&scalar_fn, children.len())?;
92-
Self::validate_children_len(&children, len)?;
107+
Self::validate_arity(&scalar_fn, slots.len())?;
108+
Self::validate_children_len(&slots, len)?;
93109

94-
let arg_dtypes: Vec<_> = children.iter().map(|c| c.dtype().clone()).collect();
110+
let arg_dtypes = Self::arg_dtypes(&slots);
95111
let dtype = scalar_fn.return_dtype(&arg_dtypes)?;
96-
let data = ScalarFnData {
97-
scalar_fn: scalar_fn.clone(),
98-
};
112+
113+
Ok(unsafe { Self::new_from_validated_slots(scalar_fn, dtype, slots, len) })
114+
}
115+
116+
/// # Safety
117+
/// The caller must ensure arity and child lengths were validated and
118+
/// "dtype" is scalar function's return dtype.
119+
#[inline]
120+
unsafe fn new_from_validated_slots(
121+
scalar_fn: ScalarFnRef,
122+
dtype: DType,
123+
slots: ArraySlots,
124+
len: usize,
125+
) -> Self {
99126
let vtable = ScalarFn { id: scalar_fn.id() };
127+
let data = ScalarFnData { scalar_fn };
128+
unsafe { Array::from_parts_unchecked(ArrayParts::new(vtable, dtype, len, data, slots)) }
129+
}
100130

101-
Ok(unsafe {
102-
Array::from_parts_unchecked(ArrayParts::new(
103-
vtable,
104-
dtype,
105-
len,
106-
data,
107-
children.into_iter().map(Some).collect::<ArraySlots>(),
108-
))
109-
})
131+
#[inline]
132+
fn arg_dtypes(slots: &[Option<ArrayRef>]) -> SmallVec<[DType; 4]> {
133+
let mut arg_dtypes = SmallVec::with_capacity(slots.len());
134+
for child in slots.iter().flatten() {
135+
arg_dtypes.push(child.dtype().clone());
136+
}
137+
arg_dtypes
110138
}
111139

112-
fn infer_len(children: &[ArrayRef]) -> VortexResult<usize> {
113-
let Some(child) = children.first() else {
140+
#[inline]
141+
fn infer_len(slots: &[Option<ArrayRef>]) -> VortexResult<usize> {
142+
let Some(child) = slots.first().and_then(Option::as_ref) else {
114143
vortex_bail!("ScalarFnArray length cannot be inferred without children");
115144
};
116145
Ok(child.len())
117146
}
118147

148+
#[inline]
119149
fn validate_arity(scalar_fn: &ScalarFnRef, child_count: usize) -> VortexResult<()> {
120150
let arity = scalar_fn.signature().arity();
121151
vortex_ensure!(
@@ -125,11 +155,14 @@ impl Array<ScalarFn> {
125155
Ok(())
126156
}
127157

128-
fn validate_children_len(children: &[ArrayRef], len: usize) -> VortexResult<()> {
129-
vortex_ensure!(
130-
children.iter().all(|c| c.len() == len),
131-
"ScalarFnArray must have children equal to the array length"
132-
);
158+
#[inline]
159+
fn validate_children_len(slots: &[Option<ArrayRef>], len: usize) -> VortexResult<()> {
160+
for child in slots.iter().flatten() {
161+
vortex_ensure!(
162+
child.len() == len,
163+
"ScalarFnArray must have children equal to the array length"
164+
);
165+
}
133166
Ok(())
134167
}
135168
}

‎vortex-array/src/arrays/scalar_fn/rules.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -190,7 +190,7 @@ mod tests {
190190
ConstantArray::new(7i32, 100).into_array()
191191
}
192192
})
193-
.collect();
193+
.collect::<Vec<_>>();
194194
let array = ScalarFnArray::try_new(
195195
TypedScalarFnInstance::new(Binary, Operator::Add).erased(),
196196
children,

‎vortex-array/src/executor.rs‎

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,6 @@ use crate::optimizer::ArrayOptimizer;
4545
use crate::optimizer::kernels::ArrayKernelsExt;
4646
use crate::optimizer::kernels::ParentExecutionKernels;
4747
use crate::optimizer::kernels::execute_parent_key;
48-
use crate::stats::ArrayStats;
4948
use crate::stats::StatsSet;
5049
use crate::trace_op;
5150

@@ -268,7 +267,7 @@ impl ArrayRef {
268267

269268
let expected_len = current_array.len();
270269
let expected_dtype = current_array.dtype().clone();
271-
let stats = current_array.statistics().to_array_stats();
270+
let stats = current_array.statistics().to_owned();
272271
let encoding_id = current_array.encoding_id();
273272
trace_op!(record_execute_encoding(&current_array));
274273
let result = current_array.execute_encoding_unchecked(ctx)?;
@@ -592,7 +591,7 @@ fn finalize_done(
592591
mut builder: Option<Box<dyn ArrayBuilder>>,
593592
expected_len: usize,
594593
expected_dtype: DType,
595-
stats: ArrayStats,
594+
stats: StatsSet,
596595
encoding_id: ArrayId,
597596
) -> VortexResult<(ArrayRef, Option<Box<dyn ArrayBuilder>>)> {
598597
let output = if let Some(mut builder) = builder.take() {
@@ -616,9 +615,7 @@ fn finalize_done(
616615
);
617616
}
618617

619-
output
620-
.statistics()
621-
.set_iter(StatsSet::from(stats).into_iter());
618+
output.statistics().set_iter(stats.into_iter());
622619
Ok((output, None))
623620
}
624621

‎vortex-array/src/expression.rs‎

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
// SPDX-License-Identifier: Apache-2.0
22
// SPDX-FileCopyrightText: Copyright the Vortex contributors
33

4-
use itertools::Itertools;
4+
use smallvec::SmallVec;
55
use vortex_error::VortexExpect;
66
use vortex_error::VortexResult;
77

@@ -30,13 +30,13 @@ impl ArrayRef {
3030
return Ok(ConstantArray::new(scalar.clone(), self.len()).into_array());
3131
}
3232

33-
let children: Vec<_> = children
33+
let len = self.len();
34+
let children = children
3435
.iter()
3536
.map(|child| self.clone().apply_bound(child))
36-
.try_collect()?;
37+
.collect::<VortexResult<SmallVec<[ArrayRef; 4]>>>()?;
3738

38-
let array =
39-
ScalarFnArray::try_new_with_len(scalar_fn.clone(), children, self.len())?.into_array();
39+
let array = ScalarFnArray::try_new_with_len(scalar_fn.clone(), children, len)?.into_array();
4040

4141
array.optimize()
4242
}
@@ -54,18 +54,18 @@ impl ArrayRef {
5454
}
5555

5656
// Otherwise, collect the child arrays.
57-
let children: Vec<_> = expr
57+
let len = self.len();
58+
let children = expr
5859
.children()
5960
.iter()
6061
.map(|e| self.clone().apply(e))
61-
.try_collect()?;
62+
.collect::<VortexResult<SmallVec<[ArrayRef; 4]>>>()?;
6263

6364
// And wrap the scalar function up in an array.
6465
let scalar_fn = expr
6566
.as_scalar()
6667
.vortex_expect("root and literal were handled above, so this is a scalar node");
67-
let array =
68-
ScalarFnArray::try_new_with_len(scalar_fn.clone(), children, self.len())?.into_array();
68+
let array = ScalarFnArray::try_new_with_len(scalar_fn.clone(), children, len)?.into_array();
6969

7070
// Optimize the resulting array's root.
7171
array.optimize()

‎vortex-array/src/scalar_fn/fns/between/mod.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -197,7 +197,7 @@ impl Between {
197197
upper: ArrayRef,
198198
options: BetweenOptions,
199199
) -> VortexResult<ScalarFnArray> {
200-
ScalarFnArray::try_new(Between.bind(options), vec![array, lower, upper])
200+
ScalarFnArray::try_new(Between.bind(options), [array, lower, upper])
201201
}
202202
}
203203

‎vortex-array/src/scalar_fn/fns/binary/mod.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,7 @@ impl Binary {
6363
rhs: ArrayRef,
6464
operator: Operator,
6565
) -> VortexResult<ScalarFnArray> {
66-
ScalarFnArray::try_new(Binary.bind(operator), vec![lhs, rhs])
66+
ScalarFnArray::try_new(Binary.bind(operator), [lhs, rhs])
6767
}
6868
}
6969

‎vortex-array/src/scalar_fn/fns/cast/mod.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@ impl Cast {
5858
/// Creates a lazy cast of `input` to `target_dtype`.
5959
#[expect(clippy::new_ret_no_self, reason = "constructs the lazy result array")]
6060
pub fn new(input: ArrayRef, target_dtype: DType) -> ScalarFnArray {
61-
ScalarFnArray::try_new(Cast.bind(target_dtype), vec![input])
61+
ScalarFnArray::try_new(Cast.bind(target_dtype), [input])
6262
.vortex_expect("Cast has one child and an infallible return dtype")
6363
}
6464
}

‎vortex-array/src/scalar_fn/fns/fill_null/mod.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@ impl FillNull {
4646
///
4747
/// Returns an error if the children have different lengths or incompatible dtypes.
4848
pub fn try_new(input: ArrayRef, fill_value: ArrayRef) -> VortexResult<ScalarFnArray> {
49-
ScalarFnArray::try_new(FillNull.bind(EmptyOptions), vec![input, fill_value])
49+
ScalarFnArray::try_new(FillNull.bind(EmptyOptions), [input, fill_value])
5050
}
5151
}
5252

‎vortex-array/src/scalar_fn/fns/get_item.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@ impl GetItem {
5050
input: ArrayRef,
5151
field_name: impl Into<FieldName>,
5252
) -> VortexResult<ScalarFnArray> {
53-
ScalarFnArray::try_new(GetItem.bind(field_name.into()), vec![input])
53+
ScalarFnArray::try_new(GetItem.bind(field_name.into()), [input])
5454
}
5555
}
5656

0 commit comments

Comments
 (0)