Optimize binary search methods#3110
Merged
Merged
Conversation
Contributor
ArcticDB Code Review SummaryNo items requiring attention. The optimization is correct, well-scoped, and the benchmark deltas in the PR description validate it. Verified:
|
alexowens90
approved these changes
May 18, 2026
vasil-pashov
approved these changes
May 18, 2026
Comment on lines
+468
to
+472
| auto record_probe_in_first_block = [&](size_t next_offset, RawType probe_value) { | ||
| prev_offset = cur_offset; | ||
| cur_offset = next_offset; | ||
| return is_before(probe_value, value); | ||
| }; |
Collaborator
There was a problem hiding this comment.
Do these two extra assignments that are omitted really make a difference?
Collaborator
Author
There was a problem hiding this comment.
Most probably they do not.
Most of the benefit is from reusing the already calculated first_block_row_count and first_block_data in make_iter_in_first_block.
It made sense to also add a first block variant of record_probe as well, to make the invariant clearer
Additional micro optimizations on binary search methods: - Don't keep `TypedBlockData` in `ColumnDataIterator` - Don't recalculate block pointer and size when we already know them during gallop
0c2d98c to
6120021
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Reference Issues/PRs
Optimizations on top of #3091
Used in #3062
What does this implement or fix?
Some micro optimizations on binary search methods:
TypedBlockDatainColumnDataIterator. Instead only keepblock_data_andblock_size_Any other comments?
Benchmarks for all search and iteration methods:
Checklist
Checklist for code changes...