Conversation
Merging this PR will degrade performance by 64.19%
|
| Mode | Benchmark | BASE |
HEAD |
Efficiency | |
|---|---|---|---|---|---|
| ❌ | Simulation | search_index_above_max |
266 µs | 499.5 µs | -46.75% |
| ⚡ | Simulation | search_index_above_max_chunked |
1,869.9 µs | 499.5 µs | ×3.7 |
| ❌ | Simulation | search_index_below_min |
262 µs | 496.9 µs | -47.27% |
| ⚡ | Simulation | search_index_below_min_chunked |
1,550.4 µs | 496.9 µs | ×3.1 |
| ❌ | Simulation | search_index_full_range_random |
264 µs | 737.3 µs | -64.19% |
| ❌ | Simulation | search_index_full_range_random_chunked |
1.6 ms | 2.1 ms | -22.02% |
| ❌ | Simulation | search_index_in_range |
264.3 µs | 737.2 µs | -64.14% |
| ❌ | Simulation | search_index_in_range_chunked |
2.1 ms | 2.5 ms | -17.84% |
| ❌ | Simulation | search_index_mixed_out_of_range |
264 µs | 498.2 µs | -47% |
| ⚡ | Simulation | search_index_mixed_out_of_range_chunked |
1,568.5 µs | 498.2 µs | ×3.1 |
| ❌ | Simulation | patched_take_10k_dispersed |
286.5 µs | 344.9 µs | -16.93% |
| ❌ | Simulation | patched_take_10k_first_chunk_only |
273 µs | 331.3 µs | -17.6% |
| ❌ | Simulation | patched_take_10k_adversarial |
229.6 µs | 288.1 µs | -20.32% |
| ❌ | Simulation | take_10k_dispersed |
240.8 µs | 298.5 µs | -19.33% |
| ❌ | Simulation | take_10k_first_chunk_only |
226.9 µs | 284.2 µs | -20.18% |
| ⚡ | Simulation | decompress_rd[f64, (100000, 0.0)] |
980.2 µs | 843.6 µs | +16.18% |
Comparing bp/minmax-index (6efcd3e) with develop (d7c22ba)
joseph-isaacs
left a comment
There was a problem hiding this comment.
Indices arrays have min/max stats these should be used instead.
We might not currently save or use this, but by these benchmarks we should.
If these benchmarks are cleaned up I am happy to merge those first.
Isolating benchmarks before this optimization: #7753 --------- Signed-off-by: Baris Palaska <barispalaska@gmail.com>
robert3005
left a comment
There was a problem hiding this comment.
Meta point, this is why you want to have special logic that's shared across all arrays, if we have Patched array then that could be one place where all of this logic could live without hacks. Alas we need Patched array to get over the line
Cache
min/maxpatch indices inPatchesand short-circuitsearch_indexwhen the query falls outside that range. Speeds up ALP/BitPackedscalar_at. This should help with slicing any patched array into a region that doesn't overlap the patch range