You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@datafusion.apache.org by gi...@apache.org on 2026/04/10 16:01:50 UTC
(datafusion) branch main updated: perf: Optimize NULL handling in `array_remove` (#21532)
This is an automated email from the ASF dual-hosted git repository.
github-bot pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/datafusion.git
The following commit(s) were added to refs/heads/main by this push:
new 42d9835346 perf: Optimize NULL handling in `array_remove` (#21532)
42d9835346 is described below
commit 42d9835346126e2ef7ef0acdbffe2cd1adcbbfc4
Author: Neil Conway <ne...@gmail.com>
AuthorDate: Fri Apr 10 11:59:50 2026 -0400
perf: Optimize NULL handling in `array_remove` (#21532)
## Which issue does this PR close?
- Closes #21531.
## Rationale for this change
`array_remove` computes the result NULL bitmap incrementally
(row-by-row). This is inefficient; it is faster to compute the result
NULL bitmap via the bitwise AND of the input NULL bitmaps.
Benchmarks (Arm64):
```
- array_remove_binary/remove/10: 6.8ms → 6.6ms (-2.9%)
- array_remove_binary/remove/100: 13.0ms → 12.7ms (-2.3%)
- array_remove_binary/remove/500: 75.2ms → 76.1ms (+1.2%)
- array_remove_boolean/remove/10: 4.5ms → 4.5ms (-0.6%)
- array_remove_boolean/remove/100: 8.0ms → 7.8ms (-2.5%)
- array_remove_boolean/remove/500: 21.0ms → 21.3ms (+1.4%)
- array_remove_decimal64/remove/10: 5.6ms → 5.7ms (+1.8%)
- array_remove_decimal64/remove/100: 10.0ms → 10.2ms (+2.0%)
- array_remove_decimal64/remove/500: 58.6ms → 56.2ms (-4.1%)
- array_remove_f64/remove/10: 5.4ms → 5.4ms (0.0%)
- array_remove_f64/remove/100: 7.7ms → 7.8ms (+1.3%)
- array_remove_f64/remove/500: 36.5ms → 36.8ms (+0.8%)
- array_remove_fixed_size_binary/remove/10: 6.1ms → 5.4ms (-11.5%)
- array_remove_fixed_size_binary/remove/100: 12.4ms → 11.6ms (-6.5%)
- array_remove_fixed_size_binary/remove/500: 66.7ms → 64.4ms (-3.4%)
- array_remove_int64/remove/10: 5.6ms → 5.7ms (+1.8%)
- array_remove_int64/remove/100: 7.8ms → 7.8ms (-1.0%)
- array_remove_int64/remove/500: 35.5ms → 36.3ms (+2.3%)
- array_remove_strings/remove/10: 6.6ms → 6.8ms (+3.0%)
- array_remove_strings/remove/100: 18.0ms → 18.3ms (+1.7%)
- array_remove_strings/remove/500: 78.0ms → 79.7ms (+2.2%)
```
Not a massive win, but I think still worth making the change; the
resulting code is also more idiomatic.
## What changes are included in this PR?
* Implement optimization
* Tweak `array_remove` benchmark for f64: the previous code resulted in
never actually removing anything from the array, which was inconsistent
with the other benchmarks and probably unintentional.
## Are these changes tested?
Yes.
## Are there any user-facing changes?
No.
---
datafusion/functions-nested/benches/array_remove.rs | 2 +-
datafusion/functions-nested/src/remove.rs | 17 ++++++++---------
2 files changed, 9 insertions(+), 10 deletions(-)
diff --git a/datafusion/functions-nested/benches/array_remove.rs b/datafusion/functions-nested/benches/array_remove.rs
index a494d32239..6fd153548a 100644
--- a/datafusion/functions-nested/benches/array_remove.rs
+++ b/datafusion/functions-nested/benches/array_remove.rs
@@ -389,7 +389,7 @@ fn create_f64_list_array(
if rng.random::<f64>() < null_density {
None
} else {
- Some(rng.random_range(0.0..array_size as f64))
+ Some(rng.random_range(0..array_size as i64) as f64)
}
})
.collect::<Float64Array>();
diff --git a/datafusion/functions-nested/src/remove.rs b/datafusion/functions-nested/src/remove.rs
index 730b4b39ac..c428009425 100644
--- a/datafusion/functions-nested/src/remove.rs
+++ b/datafusion/functions-nested/src/remove.rs
@@ -20,10 +20,10 @@
use crate::utils;
use crate::utils::make_scalar_function;
use arrow::array::{
- Array, ArrayRef, Capacities, GenericListArray, MutableArrayData, NullBufferBuilder,
- OffsetSizeTrait, cast::AsArray, make_array,
+ Array, ArrayRef, Capacities, GenericListArray, MutableArrayData, OffsetSizeTrait,
+ cast::AsArray, make_array,
};
-use arrow::buffer::OffsetBuffer;
+use arrow::buffer::{NullBuffer, OffsetBuffer};
use arrow::datatypes::{DataType, FieldRef};
use datafusion_common::cast::as_int64_array;
use datafusion_common::utils::ListCoercion;
@@ -386,12 +386,13 @@ fn general_remove<OffsetSize: OffsetSizeTrait>(
false,
Capacities::Array(original_data.len()),
);
- let mut valid = NullBufferBuilder::new(list_array.len());
+
+ // Pre-compute combined null bitmap
+ let nulls = NullBuffer::union(list_array.nulls(), element_array.nulls());
for (row_index, offset_window) in list_array.offsets().windows(2).enumerate() {
- if list_array.is_null(row_index) || element_array.is_null(row_index) {
+ if nulls.as_ref().is_some_and(|nulls| nulls.is_null(row_index)) {
offsets.push(offsets[row_index]);
- valid.append_null();
continue;
}
@@ -414,7 +415,6 @@ fn general_remove<OffsetSize: OffsetSizeTrait>(
if num_to_remove == 0 {
mutable.extend(0, start, end);
offsets.push(offsets[row_index] + OffsetSize::usize_as(end - start));
- valid.append_non_null();
continue;
}
@@ -445,7 +445,6 @@ fn general_remove<OffsetSize: OffsetSizeTrait>(
}
offsets.push(offsets[row_index] + OffsetSize::usize_as(copied));
- valid.append_non_null();
}
let new_values = make_array(mutable.freeze());
@@ -453,7 +452,7 @@ fn general_remove<OffsetSize: OffsetSizeTrait>(
Arc::clone(list_field),
OffsetBuffer::new(offsets.into()),
new_values,
- valid.finish(),
+ nulls,
)?))
}
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@datafusion.apache.org
For additional commands, e-mail: commits-help@datafusion.apache.org