perf: Optimize array_has() for array needle#23337
Open
freakyzoidberg wants to merge 3 commits into
Open
Conversation
This was referenced Jul 6, 2026
Contributor
|
run benchmarks array_has |
|
🤖 Benchmark running (GKE) | trigger CPU Details (lscpu)Comparing perf/array-has-array (ab0504c) to a0e9887 (merge-base) diff using: array_has File an issue against this benchmark runner |
|
🤖 Benchmark completed (GKE) | trigger Instance: CPU Details (lscpu)Details
Resource Usagearray_has — base (merge-base)
array_has — branch
File an issue against this benchmark runner |
freakyzoidberg
force-pushed
the
perf/array-has-array
branch
from
ab0504c to
7ed6f01
Compare
Contributor
|
run benchmarks array_has |
|
🤖 Benchmark running (GKE) | trigger CPU Details (lscpu)Comparing perf/array-has-array (7ed6f01) to 0365d3c (merge-base) diff using: array_has File an issue against this benchmark runner |
|
🤖 Benchmark completed (GKE) | trigger Instance: CPU Details (lscpu)Details
Resource Usagearray_has — base (merge-base)
array_has — branch
File an issue against this benchmark runner |
Contributor
|
Those are some pretty good results! |
`array_has(array, element)` returns, for each row, whether the array contains the element. When the `element` (needle) is an array rather than a scalar -- the needle argument is a column with one value per row, e.g. `array_has(t1.tags, t2.key)` in a join filter -- execution goes through `array_has_dispatch_for_array` (the `ColumnarValue::Array` needle branch), which compared each row by invoking the Arrow `eq` kernel once per row. That kernel allocates a `BooleanArray` and pays downcast and dispatch overhead on every row. (The scalar-needle branch was optimized separately in apache#20374.) Add a fast path for primitive and string element types, preserving the Arrow `eq` kernel semantics (total-order float equality; null elements never match). With all-valid elements each row is a single branchless OR-reduction over the native values. When primitive elements contain nulls -- whose backing values are arbitrary -- the per-element equality bitmap is ANDed with the validity bitmap (one word-parallel op, no per-element branch) before the per-row reduction, so null slots never match regardless of their value. Past a moderate average list length (`NULL_FAST_PATH_MAX_LEN`) this bitmap's extra passes lose to the per-row kernel, so the element-null branch bails to it there; that length is measured over the visible (sliced) region, so a sliced array's hidden child elements don't route a small window to the slow path. The all-valid fold has no such crossover. For string elements each row is a single pass over the row's values; for `Utf8View` this is view-aware -- the byte length and 4-byte prefix packed in the 128-bit view reject non-matches before touching the data buffer (what the `eq` kernel does, but without its per-row allocation), and an inline needle (<= 12 bytes) is matched by full-view equality with no materialization at all. Nested and other element types keep using the per-row `eq` kernel. What this removes is the fixed per-row kernel overhead, not the element comparison itself, so the gain is largest for short lists and shrinks as lists grow. Criterion microbenchmark, array needle on a 100k-row batch vs the per-row `eq` kernel (NOT end-to-end query time): i64, all-valid elements: ~15x (64-elem lists), ~1.9x at 1024-elem i64, ~30% null elements: ~9x (8-elem) ... ~1x (512-elem); falls back beyond Utf8 / LargeUtf8: ~2.5x Utf8View (view-aware): ~5x on short/inline strings Every shape improves with no regression. End-to-end impact depends on how much of a query `array_has` accounts for. For a query dominated by an array-needle `array_has` join filter (a `NestedLoopJoinExec` with `filter=array_has(tags, key)` over 3000x3000 rows of 8-element lists) total time drops from 0.95s to 0.059s (~16x, identical results). For a workload where `array_has` is a smaller fraction -- e.g. the ~6% of profile that motivated this (see apache#18070 / apache#18161, which fixed the join's deep-copy but left the per-row `array_has` cost) -- the overall speedup is single-digit percent. The array-needle criterion benchmark used for these numbers is added separately (so it can be run against `main` to capture the before baseline). Part of apache#18727. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
freakyzoidberg
force-pushed
the
perf/array-has-array
branch
from
7ed6f01 to
7598605
Compare
Move the behavioral coverage for the array (column) needle path from Rust unit tests into datafusion/sqllogictest/test_files/array/array_has.slt (element and needle nulls, null/empty rows, the null-fill collision, found/not-found, over i64, Utf8, Utf8View, LargeList and LargeUtf8, plus a >512-row case that exercises the chunked element-null path), following the maintainer preference for covering UDFs with SLT. Keep a single unit test for sliced List / FixedSizeList offset handling, whose sliced-array shape SQL can't produce. Drops the reference oracle and the string-list test helper, no longer needed. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Jefffrey
reviewed
Jul 10, 2026
Jefffrey
left a comment
Jefffrey
left a comment
Contributor
There was a problem hiding this comment.
nice improvements on the benchmarks
| // so the all-valid win path never pays for `null_count`. Strings | ||
| // (single-pass) and nested types have no such crossover and never | ||
| // reach this arm. | ||
| let num_rows = offsets.len() - 1; |
Contributor
There was a problem hiding this comment.
we can omit detail about null_count and string/nested types here. null_count check is cheap anyway (its always precomputed on array creation), and string/other types are apparent in the below arms
| let end = offsets[i + 1] - first_offset; | ||
| value_slice[start..end] | ||
| .iter() | ||
| .fold(false, |acc, &v| acc | v.is_eq(needle_val)) |
Contributor
There was a problem hiding this comment.
is it better to use any() here or does LLVM generate equivalent code anyway
Contributor
There was a problem hiding this comment.
I think the point of this fold is to be branchless: there is no early exit on first match found as any() would introduce (acc || v.is_eq(needle_val) would be equivalent to any(), but it's not branchless.)
Comment on lines
+492
to
+493
| needle_expanded | ||
| .resize(offsets[i + 1] - first_offset - elem_start, needle_slice[i]); |
Contributor
There was a problem hiding this comment.
Suggested change
| needle_expanded | |
| .resize(offsets[i + 1] - first_offset - elem_start, needle_slice[i]); | |
| needle_expanded.extend(std::iter::repeat_n( | |
| needle_slice[i], | |
| offsets[i + 1] - offsets[i], | |
| )); |
thoughts on if this is easier to read?
Comment on lines
+381
to
+386
| let offsets: Vec<usize> = haystack.offsets().collect(); | ||
| let first_offset = offsets[0]; | ||
| let visible_values = haystack | ||
| .values() | ||
| .slice(first_offset, offsets[offsets.len() - 1] - first_offset); | ||
| let visible_values = visible_values.as_ref(); |
Contributor
There was a problem hiding this comment.
might consider using the new OffsetBuffer::subtract to normalize the offsets instead of needing to handle the offset from the first offset everywhere downstream, see
gabotechs
reviewed
Jul 10, 2026
Comment on lines
+567
to
+569
| // Low 32 bits are the byte length; the next 32 are the inline prefix. | ||
| let needle_inline = (needle_view as u32) <= 12; | ||
| let needle_lo = needle_view as u64; |
Contributor
There was a problem hiding this comment.
I spent a fair amount of time looking at this not knowing what's happening here.
It might be better to not hardcode the 12 and just use the exported constant from arrow:
let needle_inline = (needle_view as u32) <= arrow::array::MAX_INLINE_VIEW_LEN;| } | ||
| } | ||
|
|
||
| /// Evaluate `array_has` with an array (per-row) needle. |
Contributor
There was a problem hiding this comment.
After reading this, my first instinct is that this should not be contributed to this repo.
I see that this PR is pretty much implement an array_has arrow kernel from scratch, and the code contributed here is not even related to DataFusion, it's just plain Arrow.
However, looking at the rest of this file... I think that ship sailed some time ago. This file was already filled with code that looks like arrow-rs could have been a better fit.
So nothing to do here I guess.
| let combined_nulls = NullBuffer::union(haystack.nulls(), needle.nulls()); | ||
|
|
||
| if let Some(values) = | ||
| array_has_array_fast_path(&haystack, needle, combined_nulls.as_ref()) |
Contributor
There was a problem hiding this comment.
_fast_path is probably not the best name for this.
array_has_array_fast_path describes the current motivation/performance role, but not what the function actually does.
I'd even go beyond this and avoid some callstack nesting by restructuring this function to something that requires less jumps from readers to understand.
Contributor
There was a problem hiding this comment.
I'd probably even remove the array_has_array_fast_path extra function jump at all, and integrate its body inside this array_has_dispatch_for_array function.
In the end, it's not possible to understand what array_has_dispatch_for_array without navigating to the body of array_has_array_fast_path, as the array_has_array_fast_path name and signature are not informative enough to know what they do without reading its contents.
Comment on lines
+396
to
+405
| // The element-null branch of `array_has_array_primitive` makes | ||
| // several passes over the values; past a moderate average list | ||
| // length the per-row `eq` kernel wins, so bail to it there. Measured | ||
| // over the *visible* region -- the offset span and `visible_values` | ||
| // nulls, not the full backing child -- so a sliced array's hidden | ||
| // elements can't skew the decision. The average check short-circuits, | ||
| // so the all-valid win path never pays for `null_count`. Strings | ||
| // (single-pass) and nested types have no such crossover and never | ||
| // reach this arm. | ||
| let num_rows = offsets.len() - 1; |
Contributor
There was a problem hiding this comment.
I've been starting for longer that I'd like to admit at this comment and I have to say I don't understand it. For example, I struggle to understand what "backing child" means, what's a "hidden element", or what is "The average check", or what's the "all-valid win path".
Any chance of maybe rewording this comment? I think it's trying to make the justification of what the (offsets[num_rows] - first_offset) / num_rows > NULL_FAST_PATH_MAX_LEN, which is actually a question that I had that I was not able to understand with this comment.
- Inline array_has_array_fast_path into array_has_dispatch_for_array so the dispatch reads in one pass. - Rebase offsets to 0 once and drop the first_offset parameter. - Use arrow's MAX_INLINE_VIEW_LEN instead of a hardcoded 12. - Build the expanded needle with std::iter::repeat_n instead of a cumulative Vec::resize. - Trim doc comments to the essentials.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Which issue does this PR close?
array_haswhen the search element is a column #23334.Full disclosure - this was heavily assisted by AI, and I did my best to understand and justify every change here before submitting.
Rationale for this change
array_has(array, element)returns, for each row, whether the array contains the element.When the
element(needle) is an array rather than a scalar, the needle argument is a column with one value per row, e.g.array_has(t1.tags, t2.key)in a join filter, execution goes througharray_has_dispatch_for_array(theColumnarValue::Arrayneedle branch), which compared each row by invoking the Arroweqkernel once per row.That kernel allocates a
BooleanArrayand pays downcast and dispatch overhead on every row. (The scalar-needle branch was optimized separately in #20374.)What this removes is the fixed per-row kernel overhead, not the element comparison itself, so the gain is largest for short lists and shrinks as lists grow.
All numbers below are from the committed criterion benchmark (
cargo bench --bench array_has, groupsarray_has_array_null_patterns/array_has_array_by_size/array_has_array_by_rows): thearray_hasUDF evaluated in isolation with an array needle, origin (the per-roweqkernel) vs now. "list length" is the number of elements in each row's array (not the row count). Not end-to-end query time.By data type and null pattern (list length 64, 10K rows)
The i64 null cases are uniform (~4x) whether the match is present, absent, the whole list is null, or the needle collides with a null slot's backing fill value — validity is folded in with one word-parallel op, so there is no per-row rescan and no null slot can match.
Strings win ~2.1–2.5x mainly by dropping the per-row
BooleanArrayallocation.Utf8Viewadditionally uses a view-aware compare: the byte length and 4-byte prefix packed into the 128-bit view reject non-matches before touching the data buffer, and an inline value (≤ 12 bytes) is matched by whole-view equality with no materialization at all — hence ~5x on short/inline strings. When long strings share a prefix (e.g. ARNs) the prefix can't reject, soUtf8Viewfalls in line with the other string types (~2.1–2.4x). No string case regresses.By list length (i64, 30% element nulls, not found, 10K rows)
The element-null branch makes a few passes over the values; past a moderate average list length (
NULL_FAST_PATH_MAX_LEN) the per-row kernel wins, so it bails to it there — no meaningful regression. That average is measured over the visible (sliced) region, so a sliced array's hidden child elements can't route a small window to the slow path. The all-valid fold has no such crossover.By row count (i64, 8 elems/row, 30% nulls, not found)
Invariant to the number of rows — the per-row overhead removed is a fixed cost, so absolute savings scale linearly with the column height.
The remaining benchmarks in the suite (scalar
array_has,array_has_all,array_has_any— paths this PR does not touch) are unchanged (median 0.99x, within measurement noise), confirming no regression outside the array-needle path.End-to-end (context)
For a query dominated by an array-needle
array_hasjoin filter (aNestedLoopJoinExecwithfilter=array_has(tags, key)over 3000x3000 rows of 8-element lists) total time drops from 0.95s to 0.059s (~16x, identical results). For a workload wherearray_hasis a smaller fraction, e.g. the ~6% of profile that motivated this (see #18070 / #18161, which fixed the join's deep-copy but left the per-rowarray_hascost), the overall speedup is single-digit percent.What changes are included in this PR?
A fast path for primitive and string element types in
array_has_dispatch_for_array, preserving the Arroweqkernel semantics (total-order float equality; null elements never match):NULL_FAST_PATH_MAX_LENaverage elements/row a length check over the visible (sliced) region bails to the per-row kernel (see the list-length table).Utf8Viewcompares the packed 128-bit views directly — length + 4-byte prefix reject non-matches before any data-buffer access, and an inline value (≤ 12 bytes) matches by whole-view equality with no materialization.eqkernel.The array-needle benchmarks used for the numbers above are added in #3 (null patterns, list length, and row count).
Are these changes tested?
Yes:
NaN/-0.0), sliced arrays (including a small visible window over a large backing child),LargeListoffsets, empty rows, a multi-chunk input, and a long-list input that exercises the per-row fallback, each cross-checked against the original per-roweqkernel as an oracle.array_has/array_contains/join_listssqllogictest suites pass.Are there any user-facing changes?
No.