Skip to content

perf: only encode actual list values in RowConverter (16-26 times faster for small sliced list) #7996

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 5 commits into from
Jul 28, 2025
Merged

perf: only encode actual list values in RowConverter (16-26 times faster for small sliced list) #7996

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
daf8f4c
perf: only encode actual list values in `RowConverter`
rluvaton Jul 24, 2025
2e034fe
avoid map_or as there must always be at least 1 offset
rluvaton Jul 24, 2025
4270ee7
added comment
rluvaton Jul 27, 2025
27faadd
Merge branch 'main' into perf-only-encode-actual-list-values
rluvaton Jul 27, 2025
7b2df90
Merge remote-tracking branch 'apache/main' into perf-only-encode-actu…
alamb Jul 28, 2025
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
33 changes: 29 additions & 4 deletions arrow-row/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -518,12 +518,37 @@ impl Codec {
}
Codec::List(converter) => {
let values = match array.data_type() {
DataType::List(_) => as_list_array(array).values(),
DataType::LargeList(_) => as_large_list_array(array).values(),
DataType::FixedSizeList(_, _) => as_fixed_size_list_array(array).values(),
DataType::List(_) => {
let list_array = as_list_array(array);
let first_offset = list_array.offsets()[0] as usize;
let last_offset =
list_array.offsets()[list_array.offsets().len() - 1] as usize;

// values can include more data than referenced in the ListArray, only encode
// the referenced values.
list_array
.values()
.slice(first_offset, last_offset - first_offset)
Comment on lines +523 to +531

@rluvaton rluvaton Jul 24, 2025

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

this can be changed to get_values_sliced() or something after we resolve:

@alamb alamb Jul 26, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Could you also please add some comments explaining the context of this code (that as you have said, is non obvious) for future readers

For example, something like

// values can include more data than referenced in the ListArray, only encode
// the referenced values.

@rluvaton rluvaton Jul 27, 2025

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

done

}
DataType::LargeList(_) => {
let list_array = as_large_list_array(array);

let first_offset = list_array.offsets()[0] as usize;
let last_offset =
list_array.offsets()[list_array.offsets().len() - 1] as usize;

// values can include more data than referenced in the LargeListArray, only encode
// the referenced values.
list_array
.values()
.slice(first_offset, last_offset - first_offset)
}
DataType::FixedSizeList(_, _) => {
as_fixed_size_list_array(array).values().clone()
}
_ => unreachable!(),
};
let rows = converter.convert_columns(&[values.clone()])?;
let rows = converter.convert_columns(&[values])?;
Ok(Encoder::List(rows))
}
Codec::RunEndEncoded(converter) => {
Expand Down
12 changes: 8 additions & 4 deletions arrow-row/src/list.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,14 +27,16 @@ pub fn compute_lengths<O: OffsetSizeTrait>(
rows: &Rows,
array: &GenericListArray<O>,
) {
let shift = array.value_offsets()[0].as_usize();

let offsets = array.value_offsets().windows(2);
lengths
.iter_mut()
.zip(offsets)
.enumerate()
.for_each(|(idx, (length, offsets))| {
let start = offsets[0].as_usize();
let end = offsets[1].as_usize();
let start = offsets[0].as_usize() - shift;
let end = offsets[1].as_usize() - shift;
let range = array.is_valid(idx).then_some(start..end);
*length += encoded_len(rows, range);
});
Expand All @@ -61,14 +63,16 @@ pub fn encode<O: OffsetSizeTrait>(
opts: SortOptions,
array: &GenericListArray<O>,
) {
let shift = array.value_offsets()[0].as_usize();

offsets
.iter_mut()
.skip(1)
.zip(array.value_offsets().windows(2))
.enumerate()
.for_each(|(idx, (offset, offsets))| {
let start = offsets[0].as_usize();
let end = offsets[1].as_usize();
let start = offsets[0].as_usize() - shift;
let end = offsets[1].as_usize() - shift;
let range = array.is_valid(idx).then_some(start..end);
let out = &mut data[*offset..];
*offset += encode_one(out, rows, range, opts)
Expand Down
Loading