Skip to content

[Parquet] perf: Create StructArrays directly rather than via ArrayData (1% improvement) #9120

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
alamb merged 4 commits into from
Jan 13, 2026
Merged

[Parquet] perf: Create StructArrays directly rather than via ArrayData (1% improvement) #9120

Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
75ea40b
[Parquet] perf: Create StructArrays directly rather than use ArrayData
alamb Jan 9, 2026
d8426fa
Avoid calling bitmap_builder.finish()
alamb Jan 11, 2026
6c3f6e8
Merge remote-tracking branch 'apache/main' into alamb/less_parquet_al…
alamb Jan 11, 2026
caf0f2c
fmt
alamb Jan 11, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 21 additions & 15 deletions parquet/src/arrow/array_reader/struct_array.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,8 @@
use crate::arrow::array_reader::ArrayReader;
use crate::errors::{ParquetError, Result};
use arrow_array::{Array, ArrayRef, StructArray, builder::BooleanBufferBuilder};
use arrow_data::{ArrayData, ArrayDataBuilder};
use arrow_schema::DataType as ArrowType;
use arrow_buffer::NullBuffer;
use arrow_schema::{DataType as ArrowType, DataType};
use std::any::Any;
use std::sync::Arc;

Expand Down Expand Up @@ -124,16 +124,15 @@ impl ArrayReader for StructArrayReader {
return Err(general_err!("Not all children array length are the same!"));
}

// Now we can build array data
let mut array_data_builder = ArrayDataBuilder::new(self.data_type.clone())
.len(children_array_len)
.child_data(
children_array
.into_iter()
.map(|x| x.into_data())
Copy link
Contributor Author

@alamb alamb Jan 9, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

converting the child arrays into ArrayData is wasteful for at least 2 reasons:

  1. They are just converted back to ArrayRefs below
  2. Each child ArrayData has at least one new allocation (the Vec of buffers)

.collect::<Vec<ArrayData>>(),
);
let DataType::Struct(fields) = &self.data_type else {
return Err(general_err!(
"Internal: StructArrayReader must have struct data type, got {:?}",
self.data_type
));
};
let fields = fields.clone(); // cloning Fields is cheap (Arc internally)

let mut nulls = None;
if self.nullable {
// calculate struct def level data

Expand Down Expand Up @@ -168,12 +167,19 @@ impl ArrayReader for StructArrayReader {
if bitmap_builder.len() != children_array_len {
return Err(general_err!("Failed to decode level data for struct array"));
}

array_data_builder = array_data_builder.null_bit_buffer(Some(bitmap_builder.into()));
nulls = Some(NullBuffer::new(bitmap_builder.finish()));
Copy link
Contributor Author

@alamb alamb Jan 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

NullBuffer::new counts the set bits, but so do the existing code paths

}

let array_data = unsafe { array_data_builder.build_unchecked() };
Ok(Arc::new(StructArray::from(array_data)))
// Safety: checked above that all children array data have same
// length and correct type
unsafe {
Ok(Arc::new(StructArray::new_unchecked_with_length(
fields,
children_array,
nulls,
children_array_len,
)))
}
}

fn skip_records(&mut self, num_records: usize) -> Result<usize> {
Expand Down
Loading