Conversation
Merging this PR will degrade performance by 31.73%
|
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
Signed-off-by: Baris Palaska <barispalaska@gmail.com>
Signed-off-by: Baris Palaska <barispalaska@gmail.com>
|
Stats lookups (rwlock + hashmap + scalar conversion) were too expensive to be in the hot path (search_index is called in tight loops) so I still went with a OnceLock approach, combined with stats. |
Signed-off-by: Baris Palaska <barispalaska@gmail.com>
|
I think RwLock -> ArcSwap change made setting stats more expensive.. |
|
It does look that way. I wonder if it made reading them faster? |
|
Github is failing me - |
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