Skip to content

Commit 5800dc1

Browse files
committed
[arrow] Minimize allocation in GenericViewArray::slice()
Use the suggested Arc<[Buffer]> storage for ViewArray storage instead of an owned Vec<Buffer> so that the slice clone does not allocate.
1 parent 116ae12 commit 5800dc1

File tree

1 file changed

+44
-17
lines changed

1 file changed

+44
-17
lines changed

arrow-array/src/array/byte_view_array.rs

Lines changed: 44 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -165,7 +165,7 @@ use super::ByteArrayType;
165165
pub struct GenericByteViewArray<T: ByteViewType + ?Sized> {
166166
data_type: DataType,
167167
views: ScalarBuffer<u128>,
168-
buffers: Vec<Buffer>,
168+
buffers: Arc<[Buffer]>,
169169
phantom: PhantomData<T>,
170170
nulls: Option<NullBuffer>,
171171
}
@@ -188,7 +188,10 @@ impl<T: ByteViewType + ?Sized> GenericByteViewArray<T> {
188188
/// # Panics
189189
///
190190
/// Panics if [`GenericByteViewArray::try_new`] returns an error
191-
pub fn new(views: ScalarBuffer<u128>, buffers: Vec<Buffer>, nulls: Option<NullBuffer>) -> Self {
191+
pub fn new<U>(views: ScalarBuffer<u128>, buffers: U, nulls: Option<NullBuffer>) -> Self
192+
where
193+
U: Into<Arc<[Buffer]>>,
194+
{
192195
Self::try_new(views, buffers, nulls).unwrap()
193196
}
194197

@@ -198,11 +201,16 @@ impl<T: ByteViewType + ?Sized> GenericByteViewArray<T> {
198201
///
199202
/// * `views.len() != nulls.len()`
200203
/// * [ByteViewType::validate] fails
201-
pub fn try_new(
204+
pub fn try_new<U>(
202205
views: ScalarBuffer<u128>,
203-
buffers: Vec<Buffer>,
206+
buffers: U,
204207
nulls: Option<NullBuffer>,
205-
) -> Result<Self, ArrowError> {
208+
) -> Result<Self, ArrowError>
209+
where
210+
U: Into<Arc<[Buffer]>>,
211+
{
212+
let buffers: Arc<[Buffer]> = buffers.into();
213+
206214
T::validate(&views, &buffers)?;
207215

208216
if let Some(n) = nulls.as_ref() {
@@ -230,11 +238,14 @@ impl<T: ByteViewType + ?Sized> GenericByteViewArray<T> {
230238
/// # Safety
231239
///
232240
/// Safe if [`Self::try_new`] would not error
233-
pub unsafe fn new_unchecked(
241+
pub unsafe fn new_unchecked<U>(
234242
views: ScalarBuffer<u128>,
235-
buffers: Vec<Buffer>,
243+
buffers: U,
236244
nulls: Option<NullBuffer>,
237-
) -> Self {
245+
) -> Self
246+
where
247+
U: Into<Arc<[Buffer]>>,
248+
{
238249
if cfg!(feature = "force_validate") {
239250
return Self::new(views, buffers, nulls);
240251
}
@@ -243,7 +254,7 @@ impl<T: ByteViewType + ?Sized> GenericByteViewArray<T> {
243254
data_type: T::DATA_TYPE,
244255
phantom: Default::default(),
245256
views,
246-
buffers,
257+
buffers: buffers.into(),
247258
nulls,
248259
}
249260
}
@@ -253,7 +264,7 @@ impl<T: ByteViewType + ?Sized> GenericByteViewArray<T> {
253264
Self {
254265
data_type: T::DATA_TYPE,
255266
views: vec![0; len].into(),
256-
buffers: vec![],
267+
buffers: vec![].into(),
257268
nulls: Some(NullBuffer::new_null(len)),
258269
phantom: Default::default(),
259270
}
@@ -279,7 +290,7 @@ impl<T: ByteViewType + ?Sized> GenericByteViewArray<T> {
279290
}
280291

281292
/// Deconstruct this array into its constituent parts
282-
pub fn into_parts(self) -> (ScalarBuffer<u128>, Vec<Buffer>, Option<NullBuffer>) {
293+
pub fn into_parts(self) -> (ScalarBuffer<u128>, Arc<[Buffer]>, Option<NullBuffer>) {
283294
(self.views, self.buffers, self.nulls)
284295
}
285296

@@ -885,8 +896,21 @@ impl<T: ByteViewType + ?Sized> Array for GenericByteViewArray<T> {
885896

886897
fn shrink_to_fit(&mut self) {
887898
self.views.shrink_to_fit();
888-
self.buffers.iter_mut().for_each(|b| b.shrink_to_fit());
889-
self.buffers.shrink_to_fit();
899+
900+
// The goal of `shrink_to_fit` is to minimize the space used by any of
901+
// its allocations. The use of `Arc::get_mut` over `Arc::make_mut` is
902+
// because if the reference count is greater than 1, `Arc::make_mut`
903+
// will first clone its contents. So, any large allocations will first
904+
// be cloned before being shrunk, leaving the pre-cloned allocations
905+
// intact, before adding the extra (used) space of the new clones.
906+
if let Some(buffers) = Arc::get_mut(&mut self.buffers) {
907+
buffers.iter_mut().for_each(|b| b.shrink_to_fit());
908+
}
909+
910+
// With the assumption that this is a best-effort function, no attempt
911+
// is made to shrink `self.buffers`, which it can't because it's type
912+
// does not expose a `shrink_to_fit` method.
913+
890914
if let Some(nulls) = &mut self.nulls {
891915
nulls.shrink_to_fit();
892916
}
@@ -944,7 +968,7 @@ impl<T: ByteViewType + ?Sized> From<ArrayData> for GenericByteViewArray<T> {
944968
fn from(value: ArrayData) -> Self {
945969
let views = value.buffers()[0].clone();
946970
let views = ScalarBuffer::new(views, value.offset(), value.len());
947-
let buffers = value.buffers()[1..].to_vec();
971+
let buffers = value.buffers()[1..].to_vec().into();
948972
Self {
949973
data_type: T::DATA_TYPE,
950974
views,
@@ -1012,12 +1036,15 @@ where
10121036
}
10131037

10141038
impl<T: ByteViewType + ?Sized> From<GenericByteViewArray<T>> for ArrayData {
1015-
fn from(mut array: GenericByteViewArray<T>) -> Self {
1039+
fn from(array: GenericByteViewArray<T>) -> Self {
10161040
let len = array.len();
1017-
array.buffers.insert(0, array.views.into_inner());
1041+
1042+
let mut buffers = array.buffers.to_vec();
1043+
buffers.insert(0, array.views.into_inner());
1044+
10181045
let builder = ArrayDataBuilder::new(T::DATA_TYPE)
10191046
.len(len)
1020-
.buffers(array.buffers)
1047+
.buffers(buffers)
10211048
.nulls(array.nulls);
10221049

10231050
unsafe { builder.build_unchecked() }

0 commit comments

Comments
 (0)