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

implement nextSetBit #253

Merged
merged 1 commit into from
Jun 5, 2018
Merged

implement nextSetBit #253

merged 1 commit into from
Jun 5, 2018

Conversation

richardstartin
Copy link
Member

This implements similar functionality to java.util.BitSet.nextSetBit, except the next bit is returned as a long, since RoaringBitmaps can contain all 32 bits.

@coveralls
Copy link

coveralls commented Jun 4, 2018

Coverage Status

Coverage increased (+0.09%) to 91.437% when pulling b17eaa7 on richardstartin:nextsetbit into 3089d7d on RoaringBitmap:master.

@lemire
Copy link
Member

lemire commented Jun 5, 2018

I'll merge and try to throw in some benchmarks.

@lemire
Copy link
Member

lemire commented Jun 5, 2018

(Great work btw.)

@lemire lemire merged commit 72f2c09 into RoaringBitmap:master Jun 5, 2018
@lemire
Copy link
Member

lemire commented Jun 5, 2018

I changed the name "nextValue" because I think that's more in the spirit of the API.

The benchmark is in as of the last commit...

benchmark                          (density)  Mode  Cnt   Score   Error  Units
BitmapNextBenchmark.bitset_count         0.1  avgt    5   2.111 ± 0.022  us/op
BitmapNextBenchmark.bitset_count         0.2  avgt    5   1.881 ± 0.036  us/op
BitmapNextBenchmark.bitset_count         0.3  avgt    5   1.678 ± 0.014  us/op
BitmapNextBenchmark.bitset_count         0.4  avgt    5   1.636 ± 0.012  us/op
BitmapNextBenchmark.bitset_count         0.5  avgt    5   1.648 ± 0.036  us/op
BitmapNextBenchmark.roaring_count        0.1  avgt    5  13.601 ± 0.092  us/op
BitmapNextBenchmark.roaring_count        0.2  avgt    5  14.597 ± 0.104  us/op
BitmapNextBenchmark.roaring_count        0.3  avgt    5  13.999 ± 0.034  us/op
BitmapNextBenchmark.roaring_count        0.4  avgt    5  13.941 ± 0.043  us/op
BitmapNextBenchmark.roaring_count        0.5  avgt    5  13.898 ± 0.807  us/op

@richardstartin
Copy link
Member Author

So it's multiplicatively slower because of container lookup overhead and binary searches. I didn't try to optimise this code at all though. Rather than implementing the bitset API verbatim it might have been interesting to get the next page of values after a given value, like a skip list.

@richardstartin richardstartin deleted the nextsetbit branch June 5, 2018 06:31
@lemire
Copy link
Member

lemire commented Jun 5, 2018

@richardstartin

Right. It is an API designed to work well for BitSet. It is a terrible way to iterate over values in a Roaring bitmap.

@richardstartin
Copy link
Member Author

@lemire agreed that it's a terrible iteration strategy to purposefully forget where you are each time you go back to the data structure, but it potentially has interesting use cases, either to retrieve an iterator starting at an offset, or to fill an array of values after an offset. This would be useful for stateless pagination.

@lemire
Copy link
Member

lemire commented Jun 5, 2018

@richardstartin

If you retrieve more than one value each time, thus amortizing the search cost, then it makes a lot of sense, I agree.

Smallhi pushed a commit to Smallhi/RoaringBitmap that referenced this pull request Jun 14, 2021
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.

None yet

3 participants