Skip to content
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

Merged
merged 4 commits into from
Mar 18, 2024
Merged

collect_block in QuickwitCollector #4753

merged 4 commits into from
Mar 18, 2024

Conversation

PSeitz
Copy link
Contributor

@PSeitz PSeitz commented Mar 17, 2024

collect_block + using first_vals to batch fetch sort values

qw_collect_block_bench

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) {
Copy link
Contributor

@fulmicoton fulmicoton Mar 18, 2024

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)

Copy link
Contributor Author

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.

Copy link
Contributor

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.

Copy link
Contributor

@fulmicoton fulmicoton left a 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.

@PSeitz PSeitz requested a review from fulmicoton March 18, 2024 06:43
@PSeitz PSeitz merged commit 3b43f71 into main Mar 18, 2024
4 checks passed
@PSeitz PSeitz deleted the faster_collect branch March 18, 2024 07:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants