Skip to content

Commit 33eeddc

Browse files
zhuqi-lucaszhuqi-lucas
committed
Address review: report Vec capacity (not len) in size() for memory accounting
Per nathanb9 on #22706: `size_of_val(slice)` returns `len * size_of::<T>()`, not the underlying Vec's allocation, so size() under-reports the heap footprint that the memory pool uses for spill decisions. Switch to the existing `VecAllocExt::allocated_size()` helper used throughout multi_group_by (primitive.rs, bytes.rs, mod.rs): - list.rs: offsets: Vec<O> -> offsets.allocated_size() - struct_.rs: also account for children: Vec<Box<dyn GroupColumn>>'s own capacity (previously only child contents were counted) fixed_size_list.rs has no owned Vec fields beyond outer_nulls (already correct) so it does not need a change.
1 parent af12254 commit 33eeddc

2 files changed

Lines changed: 4 additions & 2 deletions

File tree

‎datafusion/physical-plan/src/aggregates/group_values/multi_group_by/list.rs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ use arrow::array::{
3636
use arrow::buffer::{OffsetBuffer, ScalarBuffer};
3737
use arrow::datatypes::FieldRef;
3838
use datafusion_common::{Result, internal_datafusion_err};
39-
use std::mem::size_of_val;
39+
use datafusion_execution::memory_pool::proxy::VecAllocExt;
4040
use std::sync::Arc;
4141

4242
/// A [`GroupColumn`] for `List<T>` (`O = i32`) and `LargeList<T>` (`O = i64`).
@@ -169,7 +169,7 @@ impl<O: OffsetSizeTrait> GroupColumn for ListGroupValueBuilder<O> {
169169
}
170170

171171
fn size(&self) -> usize {
172-
size_of_val(self.offsets.as_slice())
172+
self.offsets.allocated_size()
173173
+ self.outer_nulls.allocated_size()
174174
+ self.child.size()
175175
}

‎datafusion/physical-plan/src/aggregates/group_values/multi_group_by/struct_.rs‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ use crate::aggregates::group_values::null_builder::MaybeNullBufferBuilder;
2323
use arrow::array::{Array, ArrayRef, BooleanBufferBuilder, StructArray};
2424
use arrow::datatypes::Fields;
2525
use datafusion_common::Result;
26+
use datafusion_execution::memory_pool::proxy::VecAllocExt;
2627
use std::sync::Arc;
2728

2829
/// A [`GroupColumn`] for `Struct<...>` whose children are themselves
@@ -122,6 +123,7 @@ impl GroupColumn for StructGroupValueBuilder {
122123

123124
fn size(&self) -> usize {
124125
self.outer_nulls.allocated_size()
126+
+ self.children.allocated_size()
125127
+ self.children.iter().map(|c| c.size()).sum::<usize>()
126128
}
127129

0 commit comments

Comments
 (0)