Skip to content

Buffered mutable bool index #6977

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 3 commits into from
Aug 6, 2025
Merged

Buffered mutable bool index #6977

merged 3 commits into from
Aug 6, 2025

Conversation

coszio
Copy link
Contributor

@coszio coszio commented Aug 5, 2025

Builds on top of #6954

Same approach implemented in #6954, but for bool index. Since bool index also used unbuffered updates, this PR switches their flags component to leverage BufferedDynamicFlags.

@coszio coszio requested review from timvisee and generall August 5, 2025 00:49
@timvisee timvisee added the next patch Include this in the next patch release label Aug 5, 2025
@timvisee timvisee force-pushed the use-mutable-null-index branch 2 times, most recently from eaf5712 to 1ba3a6a Compare August 5, 2025 14:41
Base automatically changed from use-mutable-null-index to dev August 5, 2025 15:04
@coszio coszio force-pushed the mutable-bool-index branch from d77e511 to 3b79a7c Compare August 5, 2025 15:30
@coderabbitai coderabbitai

This comment was marked as resolved.

Sign in to view
timvisee
timvisee approved these changes Aug 6, 2025
View reviewed changes
Copy link
Member

@timvisee timvisee left a comment

Choose a reason for hiding this comment

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

✔️

coderabbitai[bot]

This comment was marked as resolved.

Sign in to view

agourlay
agourlay reviewed Aug 6, 2025
View reviewed changes
@agourlay
Copy link
Member

agourlay commented Aug 6, 2025

Did we benchmark this change with bfb for a sanity check?

@coszio
Copy link
Contributor Author

coszio commented Aug 6, 2025

@agourlay @timvisee

Just benchmarked it on boolean_filtering bench, and I'd say result is same within noise. This is vs dev

boolean-query-points/binary-index
                        time:   [1.4627 ms 1.4669 ms 1.4729 ms]
                        change: [−4.7812% −3.7910% −2.9575%] (p = 0.00 < 0.05)
                        Performance has improved.
Found 5 outliers among 100 measurements (5.00%)
  3 (3.00%) high mild
  2 (2.00%) high severe

@coszio coszio merged commit 6ca5068 into dev Aug 6, 2025
16 checks passed
@coszio coszio deleted the mutable-bool-index branch August 6, 2025 14:10
timvisee added a commit that referenced this pull request Aug 11, 2025
@coszio @timvisee
Buffered mutable bool index (#6977)
356cdc8 
* use buffered dynamic flags in bool index

* put base_dir out of Storage

* Adjust populate comment

---------

Co-authored-by: timvisee <tim@visee.me>
@timvisee timvisee mentioned this pull request Aug 11, 2025
Merged
@coderabbitai coderabbitai bot mentioned this pull request Aug 13, 2025
Merged
9 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@timvisee timvisee timvisee approved these changes

@coderabbitai coderabbitai[bot] coderabbitai[bot] left review comments

@agourlay agourlay agourlay approved these changes

@generall generall Awaiting requested review from generall

Assignees
No one assigned
Labels
next patch Include this in the next patch release
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@coszio @agourlay @timvisee