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 <[email protected]> Co-Authored-By: Claude Fable 5.1 <[email protected]> Claude-Session: https://claude.ai/code/session_01TAbnsC5szeE4SiAmR3FciD Signed-off-by: Robert Kruszewski <[email protected]>
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 <[email protected]> Co-Authored-By: Claude Opus 5.5 <[email protected]> Signed-off-by: Robert Kruszewski <[email protected]>
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 <[email protected]> Co-Authored-By: Claude Opus 5.5 <[email protected]> Signed-off-by: Robert Kruszewski <[email protected]>
Signed-off-by: Robert Kruszewski <[email protected]>
Signed-off-by: Robert Kruszewski <[email protected]>
Signed-off-by: Robert Kruszewski <[email protected]>
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 <[email protected]> Signed-off-by: Robert Kruszewski <[email protected]>
`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 <[email protected]> Signed-off-by: Robert Kruszewski <[email protected]>
e86b215 to
4bb2b50
Compare
Reverts faad45c. That kernel decompressed a Zstd needle once and probed a hash set of a constant list, because the generic path compared every list element against the needle and decompressed it each time. Upstream vortex-data#10052 now reduces a constant list to a PreparedSet, which canonicalizes the needle once and probes its own hash set, so the kernel only adds a decompression step before the same lookup. encodings/zstd/benches/list_contains.rs (8k Zstd strings, median), list_contains over the Zstd values: list length with kernel without 4 147.5 µs 134.7 µs 256 160.6 µs 146.9 µs 2,048 195.8 µs 183.3 µs 24,999 792.3 µs 725.1 µs Before vortex-data#10052 the same call took 29 ms at 2,048 elements when the values were decompressed first, and 252 ms when they were not. Co-Authored-By: Claude Opus 5.5 <[email protected]>
Reverts faad45c. That kernel decompressed a Zstd needle once and probed a hash set of a constant list, because the generic path compared every list element against the needle and decompressed it each time. Upstream vortex-data#10052 now reduces a constant list to a PreparedSet, which canonicalizes the needle once and probes its own hash set, so the kernel only adds a decompression step before the same lookup. encodings/zstd/benches/list_contains.rs (8k Zstd strings, median), list_contains over the Zstd values: list length with kernel without 4 147.5 µs 134.7 µs 256 160.6 µs 146.9 µs 2,048 195.8 µs 183.3 µs 24,999 792.3 µs 725.1 µs Before vortex-data#10052 the same call took 29 ms at 2,048 elements when the values were decompressed first, and 252 ms when they were not. Co-Authored-By: Claude Opus 5.5 <[email protected]>
Since upstream vortex-data#10052, optimizing `list_contains(literal_list, needle)` replaces the literal list with a `PreparedSet`. `list_contains_falsify` read the elements only from a literal, so once the scan had optimized its filter, stats pruning gave up on every IN list: no whole-file pruning from footer statistics and no zone-map pruning. On SmithDB dev, a 50-trace-ID lookup read 12.8-15.9 MB of zone-map and Bloom segments where release-13 reads ~50 KB. Read the elements from the prepared set as well. Co-Authored-By: Claude Opus 5.5 <[email protected]>
Reverts faad45c. That kernel decompressed a Zstd needle once and probed a hash set of a constant list, because the generic path compared every list element against the needle and decompressed it each time. Upstream vortex-data#10052 now reduces a constant list to a PreparedSet, which canonicalizes the needle once and probes its own hash set, so the kernel only adds a decompression step before the same lookup. encodings/zstd/benches/list_contains.rs (8k Zstd strings, median), list_contains over the Zstd values: list length with kernel without 4 147.5 µs 134.7 µs 256 160.6 µs 146.9 µs 2,048 195.8 µs 183.3 µs 24,999 792.3 µs 725.1 µs Before vortex-data#10052 the same call took 29 ms at 2,048 elements when the values were decompressed first, and 252 ms when they were not. Co-Authored-By: Claude Opus 5.5 <[email protected]>
Replace constant-set membership comparison/OR chains with prepared, single-pass probes