perf: optimize array_empty udf - #23923
Conversation
|
|
||
| fn general_array_empty<O: OffsetSizeTrait>(array: &ArrayRef) -> Result<ArrayRef> { | ||
| let result = as_generic_list_array::<O>(array)? | ||
| .iter() |
There was a problem hiding this comment.
calling iter on ListArray is slow since it slice each list values which every slice recompute the null count and more
| fn general_array_empty<O: OffsetSizeTrait>(array: &ArrayRef) -> Result<ArrayRef> { | ||
| let result = as_generic_list_array::<O>(array)? | ||
| .iter() | ||
| .map(|arr| arr.map(|arr| arr.is_empty())) |
There was a problem hiding this comment.
iter also return None for nulls which means that it need to read the null info while it is not needed
| let result = as_generic_list_array::<O>(array)? | ||
| .iter() | ||
| .map(|arr| arr.map(|arr| arr.is_empty())) | ||
| .collect::<BooleanArray>(); |
There was a problem hiding this comment.
collect not using the from_trusted_len_iter helper which is faster
| .collect::<BooleanArray>(); | ||
| let result = as_generic_list_array::<O>(array)?; | ||
| // No nulls, just look at the offsets | ||
| let is_empty_iter = result.offsets().lengths().map(|n| n == 0); |
There was a problem hiding this comment.
this is faster since we only need to look at one buffer and can make it very fast
the values of the list are irrelevent
| let (values, _) = output_buffer.into_parts(); | ||
|
|
||
| // Add the nulls | ||
| let result = BooleanArray::new(values, result.nulls().filter(|n| n.null_count() > 0).cloned()); |
There was a problem hiding this comment.
We don't need to evaluate the null of specific items since they are not relevant, we just need to move them
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #23923 +/- ##
==========================================
+ Coverage 80.67% 80.69% +0.02%
==========================================
Files 1095 1095
Lines 372376 372626 +250
Branches 372376 372626 +250
==========================================
+ Hits 300397 300687 +290
+ Misses 54052 53986 -66
- Partials 17927 17953 +26 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
3 participants
Which issue does this PR close?
N/A
Rationale for this change
I saw that
emptyimplementation is inefficient.I wrote review comments for more why inefficient.
there are some optimizations that do not require benchmark since they are obvious once you understand, this is one of them
What changes are included in this PR?
rewrote the function to be fast
Are these changes tested?
existing tests
Are there any user-facing changes?
no