Skip to content

Commit ffaec9f

Browse files
robert3005claude
robert3005
and
claude
authored
Build arrays through non-generic constructors (#10737)
The generic constructors (`ConstantArray::new<S>`, `PrimitiveArray::{new, try_new, new_unchecked, empty}<T>`, `DecimalArray::{try_new, new_unchecked}<T>`, and the `impl IntoIterator` constructors of `StructArray` and `ChunkedArray`) called the generic `Array::from_parts_unchecked` and `Array::try_from_parts` directly. Each crate that called them monomorphized its own copy of those functions and of the full `DynArrayData` vtable for the encoding. The generic constructors now call a private, non-generic builder. `vortex-array` compiles each vtable once, and other crates call it. `ScalarFn` had the same problem because `new_from_validated_slots` was `#[inline]`, and it is not inlined now. The cost is one function call when an array is constructed. | `vortex-ffi` release cdylib (macOS arm64) | before | after | |---|---|---| | `__text` | 37,223,740 B | 36,613,308 B (-610 KB) | | Copies of the vtable: Constant / Primitive / Struct / Chunked / Decimal / ScalarFn | 22 / 17 / 7 / 5 / 5 / 4 | 1 / 1 / 1 / 1 / 1 / 1 | Signed-off-by: Robert Kruszewski <robert@spiraldb.com> Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
1 parent 8e84e21 commit ffaec9f

6 files changed

Lines changed: 110 additions & 13 deletions

File tree

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

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -219,7 +219,7 @@ impl Array<Chunked> {
219219

220220
/// Constructs a new `ChunkedArray`.
221221
pub fn try_new(chunks: impl IntoIterator<Item = ArrayRef>, dtype: DType) -> VortexResult<Self> {
222-
Array::try_from_parts(Self::parts_from_chunks::<true>(chunks, dtype)?)
222+
Self::try_from_chunked_parts(Self::parts_from_chunks::<true>(chunks, dtype)?)
223223
}
224224

225225
pub fn rechunk(
@@ -282,6 +282,26 @@ impl Array<Chunked> {
282282
pub unsafe fn new_unchecked(chunks: impl IntoIterator<Item = ArrayRef>, dtype: DType) -> Self {
283283
let parts = Self::parts_from_chunks::<false>(chunks, dtype)
284284
.vortex_expect("unchecked chunked construction cannot fail");
285+
unsafe { Self::from_chunked_parts_unchecked(parts) }
286+
}
287+
288+
/// Creates the array from parts after validation.
289+
///
290+
/// The generic constructors call this function. It is not generic, so crates that call those
291+
/// constructors do not each compile the `Chunked` array vtable.
292+
fn try_from_chunked_parts(parts: ArrayParts<Chunked>) -> VortexResult<Self> {
293+
Array::try_from_parts(parts)
294+
}
295+
296+
/// Creates the array from parts without validation.
297+
///
298+
/// The generic constructors call this function. It is not generic, so crates that call those
299+
/// constructors do not each compile the `Chunked` array vtable.
300+
///
301+
/// # Safety
302+
///
303+
/// The parts must be logically consistent.
304+
unsafe fn from_chunked_parts_unchecked(parts: ArrayParts<Chunked>) -> Self {
285305
unsafe { Array::from_parts_unchecked(parts) }
286306
}
287307
}

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

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,14 @@ impl Array<Constant> {
4545
where
4646
S: Into<Scalar>,
4747
{
48-
let scalar = scalar.into();
48+
Self::from_scalar(scalar.into(), len)
49+
}
50+
51+
/// Creates the array from a [`Scalar`].
52+
///
53+
/// The generic constructors call this function. It is not generic, so crates that call those
54+
/// constructors do not each compile the `Constant` array vtable.
55+
fn from_scalar(scalar: Scalar, len: usize) -> Self {
4956
let dtype = scalar.dtype().clone();
5057
let data = ConstantData::new(scalar);
5158
unsafe {

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

Lines changed: 24 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -426,7 +426,9 @@ impl Array<Decimal> {
426426
let len = buffer.len();
427427
let slots = DecimalData::make_slots(&validity, len);
428428
let data = unsafe { DecimalData::new_unchecked(buffer, decimal_dtype) };
429-
unsafe { Array::from_parts_unchecked(ArrayParts::new(Decimal, dtype, len, data, slots)) }
429+
unsafe {
430+
Self::from_decimal_parts_unchecked(ArrayParts::new(Decimal, dtype, len, data, slots))
431+
}
430432
}
431433

432434
/// Creates a new [`DecimalArray`] from a host-native buffer with validation.
@@ -439,7 +441,27 @@ impl Array<Decimal> {
439441
let len = buffer.len();
440442
let slots = DecimalData::make_slots(&validity, len);
441443
let data = DecimalData::try_new(buffer, decimal_dtype)?;
442-
Array::try_from_parts(ArrayParts::new(Decimal, dtype, len, data, slots))
444+
Self::try_from_decimal_parts(ArrayParts::new(Decimal, dtype, len, data, slots))
445+
}
446+
447+
/// Creates the array from parts without validation.
448+
///
449+
/// The generic constructors call this function. It is not generic, so crates that call those
450+
/// constructors do not each compile the `Decimal` array vtable.
451+
///
452+
/// # Safety
453+
///
454+
/// The parts must be logically consistent.
455+
unsafe fn from_decimal_parts_unchecked(parts: ArrayParts<Decimal>) -> Self {
456+
unsafe { Array::from_parts_unchecked(parts) }
457+
}
458+
459+
/// Creates the array from parts after validation.
460+
///
461+
/// The generic constructors call this function. It is not generic, so crates that call those
462+
/// constructors do not each compile the `Decimal` array vtable.
463+
fn try_from_decimal_parts(parts: ArrayParts<Decimal>) -> VortexResult<Self> {
464+
Array::try_from_parts(parts)
443465
}
444466

445467
/// Creates a new [`DecimalArray`] from an iterator of values.

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

Lines changed: 30 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -346,7 +346,11 @@ impl Array<Primitive> {
346346
let len = 0;
347347
let data = PrimitiveData::empty::<T>(nullability);
348348
let slots = PrimitiveData::make_slots(&Validity::from(nullability), len);
349-
unsafe { Array::from_parts_unchecked(ArrayParts::new(Primitive, dtype, len, data, slots)) }
349+
unsafe {
350+
Self::from_primitive_parts_unchecked(ArrayParts::new(
351+
Primitive, dtype, len, data, slots,
352+
))
353+
}
350354
}
351355

352356
/// Creates a new `PrimitiveArray`.
@@ -360,7 +364,11 @@ impl Array<Primitive> {
360364
let len = buffer.len();
361365
let slots = PrimitiveData::make_slots(&validity, len);
362366
let data = PrimitiveData::new(buffer, validity);
363-
unsafe { Array::from_parts_unchecked(ArrayParts::new(Primitive, dtype, len, data, slots)) }
367+
unsafe {
368+
Self::from_primitive_parts_unchecked(ArrayParts::new(
369+
Primitive, dtype, len, data, slots,
370+
))
371+
}
364372
}
365373

366374
/// Constructs a new `PrimitiveArray`.
@@ -370,7 +378,9 @@ impl Array<Primitive> {
370378
let slots = PrimitiveData::make_slots(&validity, len);
371379
let data = PrimitiveData::try_new(buffer, validity)?;
372380
Ok(unsafe {
373-
Array::from_parts_unchecked(ArrayParts::new(Primitive, dtype, len, data, slots))
381+
Self::from_primitive_parts_unchecked(ArrayParts::new(
382+
Primitive, dtype, len, data, slots,
383+
))
374384
})
375385
}
376386

@@ -384,7 +394,23 @@ impl Array<Primitive> {
384394
let len = buffer.len();
385395
let slots = PrimitiveData::make_slots(&validity, len);
386396
let data = unsafe { PrimitiveData::new_unchecked(buffer, validity) };
387-
unsafe { Array::from_parts_unchecked(ArrayParts::new(Primitive, dtype, len, data, slots)) }
397+
unsafe {
398+
Self::from_primitive_parts_unchecked(ArrayParts::new(
399+
Primitive, dtype, len, data, slots,
400+
))
401+
}
402+
}
403+
404+
/// Creates the array from parts without validation.
405+
///
406+
/// The generic constructors call this function. It is not generic, so crates that call those
407+
/// constructors do not each compile the `Primitive` array vtable.
408+
///
409+
/// # Safety
410+
///
411+
/// The parts must be logically consistent.
412+
unsafe fn from_primitive_parts_unchecked(parts: ArrayParts<Primitive>) -> Self {
413+
unsafe { Array::from_parts_unchecked(parts) }
388414
}
389415

390416
/// Create a new array from a buffer handle.

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

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -119,7 +119,9 @@ impl Array<ScalarFn> {
119119
/// # Safety
120120
/// The caller must ensure arity and child lengths were validated and
121121
/// "dtype" is scalar function's return dtype.
122-
#[inline]
122+
///
123+
/// This function is not inlined, so crates that construct scalar function arrays do not each
124+
/// compile the `ScalarFn` array vtable.
123125
unsafe fn new_from_validated_slots(
124126
scalar_fn: ScalarFnRef,
125127
dtype: DType,

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

Lines changed: 24 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -294,7 +294,7 @@ impl Array<Struct> {
294294
slots.push(Some(field));
295295
}
296296
let dtype = StructFields::new(names, field_dtypes);
297-
Array::try_from_parts(ArrayParts::new(
297+
Self::try_from_struct_parts(ArrayParts::new(
298298
Struct,
299299
DType::Struct(dtype, validity.nullability()),
300300
length,
@@ -317,7 +317,7 @@ impl Array<Struct> {
317317
let outer_dtype = DType::Struct(dtype, validity.nullability());
318318
let slots = make_struct_slots(fields, &validity, length);
319319
unsafe {
320-
Array::from_parts_unchecked(ArrayParts::new(
320+
Self::from_struct_parts_unchecked(ArrayParts::new(
321321
Struct,
322322
outer_dtype,
323323
length,
@@ -336,7 +336,7 @@ impl Array<Struct> {
336336
) -> VortexResult<Self> {
337337
let outer_dtype = DType::Struct(dtype, validity.nullability());
338338
let slots = make_struct_slots(fields, &validity, length);
339-
Array::try_from_parts(ArrayParts::new(
339+
Self::try_from_struct_parts(ArrayParts::new(
340340
Struct,
341341
outer_dtype,
342342
length,
@@ -345,6 +345,26 @@ impl Array<Struct> {
345345
))
346346
}
347347

348+
/// Creates the array from parts after validation.
349+
///
350+
/// The generic constructors call this function. It is not generic, so crates that call those
351+
/// constructors do not each compile the `Struct` array vtable.
352+
fn try_from_struct_parts(parts: ArrayParts<Struct>) -> VortexResult<Self> {
353+
Array::try_from_parts(parts)
354+
}
355+
356+
/// Creates the array from parts without validation.
357+
///
358+
/// The generic constructors call this function. It is not generic, so crates that call those
359+
/// constructors do not each compile the `Struct` array vtable.
360+
///
361+
/// # Safety
362+
///
363+
/// The parts must be logically consistent.
364+
unsafe fn from_struct_parts_unchecked(parts: ArrayParts<Struct>) -> Self {
365+
unsafe { Array::from_parts_unchecked(parts) }
366+
}
367+
348368
/// Construct a `StructArray` from named fields.
349369
pub fn from_fields<N: AsRef<str>>(items: &[(N, ArrayRef)]) -> VortexResult<Self> {
350370
Self::try_from_iter(items.iter().map(|(a, b)| (a, b.clone())))
@@ -462,7 +482,7 @@ impl Array<Struct> {
462482

463483
let new_dtype = struct_dtype.without_field(position).ok()?;
464484
let new_array = unsafe {
465-
Array::from_parts_unchecked(ArrayParts::new(
485+
Self::from_struct_parts_unchecked(ArrayParts::new(
466486
Struct,
467487
DType::Struct(new_dtype, self.dtype().nullability()),
468488
len,

0 commit comments

Comments
 (0)