Skip to content

Cache min/max indices in patches#7753

Open
palaska wants to merge 5 commits intodevelopfrom
bp/minmax-index
Open

Cache min/max indices in patches#7753
palaska wants to merge 5 commits intodevelopfrom
bp/minmax-index

Conversation

@palaska
Copy link
Copy Markdown
Contributor

@palaska palaska commented May 1, 2026

Cache min/max patch indices in Patches and short-circuit search_index when the query falls outside that range. Speeds up ALP/BitPacked scalar_at. This should help with slicing any patched array into a region that doesn't overlap the patch range

Screenshot 2026-05-01 at 16 26 04

Signed-off-by: Baris Palaska <barispalaska@gmail.com>
@palaska palaska added the changelog/performance A performance improvement label May 1, 2026
@codspeed-hq
Copy link
Copy Markdown

codspeed-hq Bot commented May 1, 2026

Merging this PR will degrade performance by 64.19%

⚠️ Unknown Walltime execution environment detected

Using the Walltime instrument on standard Hosted Runners will lead to inconsistent data.

For the most accurate results, we recommend using CodSpeed Macro Runners: bare-metal machines fine-tuned for performance measurement consistency.

⚠️ Different runtime environments detected

Some benchmarks with significant performance changes were compared across different runtime environments,
which may affect the accuracy of the results.

Open the report in CodSpeed to investigate

⚡ 4 improved benchmarks
❌ 12 regressed benchmarks
✅ 1164 untouched benchmarks

⚠️ Please fix the performance issues or acknowledge them on CodSpeed.

Performance Changes

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)

Open in CodSpeed

Signed-off-by: Baris Palaska <barispalaska@gmail.com>
Copy link
Copy Markdown
Contributor

@joseph-isaacs joseph-isaacs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@joseph-isaacs
Copy link
Copy Markdown
Contributor

Signed-off-by: Baris Palaska <barispalaska@gmail.com>
Signed-off-by: Baris Palaska <barispalaska@gmail.com>
palaska added a commit that referenced this pull request May 5, 2026
Isolating benchmarks before this optimization:
#7753

---------

Signed-off-by: Baris Palaska <barispalaska@gmail.com>
Copy link
Copy Markdown
Contributor

@robert3005 robert3005 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

changelog/performance A performance improvement

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants