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

Perf regression: Iteration #176

Closed
Tracked by #178
saik0 opened this issue Feb 5, 2022 · 0 comments · Fixed by #179
Closed
Tracked by #178

Perf regression: Iteration #176

saik0 opened this issue Feb 5, 2022 · 0 comments · Fixed by #179

Comments

@saik0
Copy link
Contributor

saik0 commented Feb 5, 2022

#141 Tracks cardinality remaining in the iterator.

The code it replaced would unconditionally panic. To guess the original author's intent: it was a runtime check that the count is tracked at the outer iterator. Panicking broke some new proptests, so we implemented the correct behavior. However, it is still true that the size hint is unreachable from the exposed API. The iterator is doing extra work the end user can never observe.

I suspect the regression went unnoticed since #125 plus this issue is still a net perf gain.

Proposing we just remove the size hints altogether and return the default (0, None). It will satisfy the validity requirements of tests without needlessly slowing down iteration.

@bors bors bot closed this as completed in 16afed3 Feb 7, 2022
@saik0 saik0 mentioned this issue Feb 7, 2022
not-jan pushed a commit to not-jan/roaring-rs that referenced this issue Aug 31, 2022
179: Remove size hint and exact size from bitmap inner iterators r=Kerollmops a=saik0

Closes RoaringBitmap#176 

Co-authored-by: saik0 <github@saik0.net>
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 a pull request may close this issue.

1 participant