-
Notifications
You must be signed in to change notification settings - Fork 269
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
Misc Missing Functionality for Roaring64 bitmaps #549
Comments
If I may add a few to the list:
|
The implementation was actually already written using an inclusive range already. See #549
The implementation was actually already written using an inclusive range already. See #549
The implementation was actually already written using an inclusive range already. See #549
The implementation was actually already written using an inclusive range already. See #549
The implementation was actually already written using an inclusive range already. See #549
I noticed that the 64-bit implementation is also missing _init_with_capacity() and _init_cleared() APIs for use cases where the caller prefers to handle storing the bitmap structure. Maybe these can be added to the list as well? |
I believe the lack of
|
@Dr-Emann is correct, we did not expose these init methods as they don't make sense for the 64-bit implementation and also because we don't expose the definition of |
Thank you for the explanation on why _with_capacity() makes no sense for the 64-bit implementation. I realize that To add some context, I am working on a codebase where it is common to have data structures wrapped in other structs, to use our implementation of software transactional memory. The additional allocation can be significant for performance, but maybe even more importantly, this means that accessing such a structure leads to an additional pointer dereference and therefore can incur additional cache misses. These are costly in a system where we are mostly bottlenecked on memory. |
@mux Hiding the implementation allows us to easily make changes that change the structure of the implementation. I am not sure why this was not done for the 32-bit implementation, but it was written much earlier than the 64-bit version and it's hard to change now. |
@SLieve I understand - like I was saying I was curious about how things came to be like this but I totally get why you'd want to keep things this way. Nearly every single time I create a new API acting on data, I have a forward struct declaration, and provide pointer-based functions. Thanks for your answer. |
_overwrite() is also missing, correct? |
@nolaforensix It is now on the list. |
This is just a list of functionality which I noticed was missing in comparison to
roaring_bitmap_t
as I was adding bindings to croaring-rs. I may add more if I find anything else, rather than making more issues.roaring64_bitmap_clear
, to remove all data, and start from an empty bitmap(can this be done more efficiently thanroaring64_bitmap_remove_range_closed(0, UINT64_MAX)
). It seems likeroaring64_bitmap_remove_range_closed
could be optimized, it appears to loop through all high 48 bit combos, rather than skipping around using the ART.roaring64_bitmap_remove_range_closed(0, UINT64_MAX)
is now efficient.roaring64_bitmap_remove_run_compression
. This is probably only really useful after Provide serialization/deserialization functionality to the ART-based 64-bit roaring #542, to avoid run containers in the serialization when the reader won't understand themroaring64_bitmap_internal_validate
, mostly useful for tests, to ensure our assumptions are never broken (max art depth, things which should be sorted are, etc).roaring64_bitmap_flip
roaring64_bitmap_range_cardinality
is exclusive only, it's unfortunate thatroaring64_bitmap_cardinality(r)
can't be emulated byroaring64_bitmap_range_cardinality(r, 0, UINT64_MAX)
roaring64_bitmap_steal_roaring32(roaring_bitmap_t *)
which steals the containers from the passed 32 bit bitmap. (naming?) Addroaring64_bitmap_move_from_roaring32
function #649roaring64_bitmap_add_offset
roaring64_bitmap_overwrite
The text was updated successfully, but these errors were encountered: