-
Notifications
You must be signed in to change notification settings - Fork 356
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
collect_block in QuickwitCollector #4753
Conversation
collect_block + using `first_vals` to batch fetch sort values
let mut len = 0; | ||
for &doc in docs { | ||
filtered_docs_buffer[len] = doc; | ||
len += if timestamp_filter.is_within_range(doc) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we try to vectorize this too? (not in this PR of course)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fetching the values as block, yes definitely.
Filtering via SIMD maybe, the values are u64
, so that would be just 4 integers per block on AVX2.
The fastest variant would probably be a pushdown to the codec where we could filter on the compressed data.
Although I'm not sure if the overhead is not too high with a block size of just 64 elements.
I'm also thinking that maybe we should use a branch here. Since the data we have is mostly ordered by timestamp the branch predictor should have a pretty easy job.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed on all points and doubts.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
a few minor comments.
Co-authored-by: Paul Masurel <[email protected]>
collect_block + using
first_vals
to batch fetch sort values