Repository navigation
Optimize list_contains with prepared constant sets - #10052
Conversation
8e24c7e to
f1eea0e
Compare
Merging this PR will regress 12 benchmarks
|
| Mode | Benchmark | BASE |
HEAD |
Efficiency | |
|---|---|---|---|---|---|
| ❌ | Simulation | optimize_lookup_predicate[ids=192, shape=in_list] |
51 µs | 76.7 µs | -33.44% |
| ❌ | Simulation | optimize_lookup_predicate[ids=128, shape=in_list] |
51.6 µs | 74 µs | -30.21% |
| ❌ | Simulation | optimize_lookup_predicate[ids=64, shape=in_list] |
51.5 µs | 71.4 µs | -27.83% |
| ❌ | Simulation | optimize_lookup_predicate[ids=16, shape=in_list] |
52.4 µs | 70.2 µs | -25.41% |
| ❌ | Simulation | optimize_lookup_predicate[ids=256, shape=in_list] |
79.6 µs | 106.1 µs | -24.97% |
| ❌ | Simulation | optimize_lookup_predicate[ids=1, shape=in_list] |
75.3 µs | 90.7 µs | -17.07% |
| ❌ | Simulation | sum_v2_i64 |
194.2 µs | 224.9 µs | -13.63% |
| ❌ | Simulation | sum_i64 |
194.4 µs | 224.7 µs | -13.48% |
| ❌ | Simulation | random_i8[0.01] |
108.4 µs | 123.9 µs | -12.5% |
| ❌ | WallTime | filter_avx2[64] |
2.1 µs | 2.4 µs | -12.23% |
| ❌ | WallTime | filter_avx2[80] |
2.2 µs | 2.5 µs | -10.93% |
| ❌ |
Simulation | random_i128[0.01] |
7.2 µs | 8 µs | -10.53% |
| ⚡ | Simulation | i64_random_chunked[256] |
13,907.4 µs | 226.4 µs | ×61 |
| ⚡ | Simulation | utf8_random[256] |
12,973.6 µs | 416.1 µs | ×31 |
| ⚡ | Simulation | i64_random[256] |
3,928.8 µs | 170.1 µs | ×23 |
| ⚡ | Simulation | nested_list_random[32] |
6.7 ms | 1 ms | ×6.4 |
| ⚡ | WallTime | filter_avx512[80] |
3.2 µs | 2.3 µs | +38.85% |
| ⚡ | WallTime | filter_avx512[64] |
2.9 µs | 2.2 µs | +33.18% |
| ⚡ | WallTime | words_gather_dispatch_neon[65536] |
2.3 µs | 2 µs | +10.42% |
Tip
Investigate this regression by commenting @codspeedbot fix this regression on this PR, or directly use the CodSpeed MCP with your agent.
Comparing claude/friendly-knuth-dj6ed1-list-contains (4bb2b50) with develop (751e0af)
f1eea0e to
ef75b12
Compare
ef75b12 to
f350f7b
Compare
|
How do i extend this to work for new custom kernels? I really think we want to make probe an array to allow reduce/execute rules? |
3e6c3cb to
12f4b8f
Compare
12f4b8f to
4b5ce4a
Compare
4b5ce4a to
74eda6a
Compare
ab3ce3b to
fd2586b
Compare
| ParentRuleSet::lift(&FillNullReduceAdaptor(Constant)), | ||
| ParentRuleSet::lift(&FilterReduceAdaptor(Constant)), | ||
| ParentRuleSet::lift(&NotReduceAdaptor(Constant)), | ||
| ParentRuleSet::lift(&PrepareConstantListRule), |
There was a problem hiding this comment.
Why is this defined here and in scalar_fn?
| } | ||
|
|
||
| /// The elements of a list scalar as an array, one row per element; empty for a null list. | ||
| pub(crate) fn list_scalar_elements(list: &ListScalar, allocator: &BufferAllocatorRef) -> ArrayRef { |
There was a problem hiding this comment.
why is this used in two places
seems like both ConstantArray list_contains and list_contains scalar fn vtable share the constant heystack impl?
|
|
||
| /// Bit-pack the predicate `f(values[i])` over a slice into a [`BitBuffer`]. | ||
| pub(super) fn collect_bits<T: Copy>( | ||
| pub(crate) fn collect_bits<T: Copy>( |
There was a problem hiding this comment.
shared with vortex-compute?
| #[cfg(test)] | ||
| fn is_bitmap(&self) -> bool { | ||
| false | ||
| } |
joseph-isaacs
left a comment
There was a problem hiding this comment.
Comments worth fixing, but otherall good
|
I will have a follow up to convert the existing trait into an Array |
`list_contains(lit([...]), needles)`, the `IN` shape, built one `Eq` per set element and OR-reduced them: a pass over the needles per element. The constant list is now prepared once into a probe structure and every needle is probed in a single pass. The probe adapts to the elements: integers, and floats by their bit patterns, use a bitmap over a dense span or a sorted slice; decimals their unscaled values the same way; UTF-8 and binary a table keyed by the view for short values and by the bytes for long ones; nested rows a sorted index over the row comparator. A literal list becomes a `PreparedSetLiteral` when the expression is optimized, so every batch probes one shared set; a constant list in an array tree becomes a `PreparedSetArray` on optimization. Reduce rules for `Chunked` and `Sequence` push the function into their chunks with slices of that array, so all share the set. The list-column-against-needle-column case, previously unsupported, gathers each list's elements next to its row's needle for one equality. A list scalar's elements are written straight from their values rather than through a scalar per element. The `list_contains_set` benchmark covers integer, string and nested sets over flat and chunked needles. Signed-off-by: Robert Kruszewski <robert@spiraldb.com> Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TAbnsC5szeE4SiAmR3FciD Signed-off-by: Robert Kruszewski <github@robertk.io>
Keep the list, needle and comparison validity of the column-against-column path as masks, so that all-valid and all-invalid inputs do not allocate a bit buffer, and skip the null-element fold when every comparison is valid. Shorten the PrepareConstantListRule docs. Signed-off-by: Robert Kruszewski <robert@spiraldb.com> Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Robert Kruszewski <github@robertk.io>
Execute the gathered comparisons of the column-against-column path to `Columnar`, so that a constant answer decides each row from whether its list is empty instead of folding every list. Drop the constant-list fallback in `compute_list_contains`: the executor's reduce_parent step replaces a constant list with a prepared set before execution. Signed-off-by: Robert Kruszewski <robert@spiraldb.com> Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Robert Kruszewski <github@robertk.io>
The lookup of needles in a constant set is now a crate-private `ArrayHaystack` in `array/haystack`, next to `ArrayProbe`. A probe reads rows by index; a haystack answers whether each needle of a batch is an element. The per-dtype states (integer bitmap or sorted, decimal, bytes, rows) implement a private `HaystackState` trait. `PreparedSetData` still builds the haystack lazily on the first lookup and shares it. Docs and test names say "haystack" or "look up" in place of "probe", so that they do not read as part of the `ArrayProbe` API. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Robert Kruszewski <github@robertk.io>
`collect_bits` names both the module and the function that `lane_kernels` re-exports, so rustdoc rejected the bare link. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Robert Kruszewski <github@robertk.io>
e86b215 to
4bb2b50
Compare
Replace constant-set membership comparison/OR chains with prepared, single-pass probes