Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Added optional serde support, including an example in the README #59
base: master
Are you sure you want to change the base?
Added optional serde support, including an example in the README #59
Changes from all commits
f4ea0fa
34d1155
7490d2c
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, deriving should work.
However, it is possible to manually implemented the serialization and deserialization so that they use only
layer0
. This would make serialized result use less memory and make serialization faster, but possibly slow deserialization down. When deserializing we have to do operations for all layers in both implementations and IO is usually the more expensive part, so the custom implementation would most likely be faster in general.Choosing which to use should be chosen before publishing new version as changing serialization format is a breaking change.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The memory usage difference is that with the derive we use
O(n log_{bits in usize} n)
and with custom derive we useO(n)
memory.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Probably best to benchmark the difference in speed if we want to have the custom implementation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is totally up to your team, no doubt a custom serializer would be more compact and faster. In my use case I have a bitset of 100K bits, the size is not as compact as I'd like, about the same as a test with fixedbitset (12K). If there is a way to get it much smaller I would be interested, and if you can guide me on that I may have time to implement it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So how the data structure works is that the
layer0
is essentiallyfixedbitset
.So if we want to access n:th bit we find which
usize
in theVec
it belongs to and then mask the right bit out of it.How
hibitset
differs is that it has layers above it. The idea is that if there is a1
in on upper layer at some point, then layer below contains1
's in correspondingusize
. This means that ifn
:th bit is1
inlayer0
then inlayer1
there will be1
infloor(n/64)
position and1
infloor(n/(62^2))
position oflayer2
and so on.What all this means is that in serialization we can just serialize
layer0
as is (which means that the serialized form is pretty much the same as infixedbitset
). However, when deserializing, the layers above have to be reconstructed. Most naive way to do this would be just to create emptyhibitset
with correct size withwith_capacity
and then insert bits one by one. Less naive way would be to createhibitset
with deserialized vector onlayer0
and, empty, but right sized, layers above and then go throughlayer1
checking if correspondinglayer0
'susize
is equal to0
or not, repeating until on highest layer.If you want to try your hand on implementing it, I can help you.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@WaDelma Here is a simple implementation of the naive way to do this, now if you can give me some pointers I'll try the layer building.
I'm sure your method will be much faster so I'd like to try it with some guidance, but this gives me a test to start with that functions properly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@WaDelma I ran some tests, and using
serde
andbitpacking
the size is only slightly larger than the custom serialization. It was about 12.7K usingserde
and the custom serialization was just over 12K. It could be because my bitset was not very dense, so maybe it will make a bigger difference if we use your more advanced method. Let me know what you think, I'll wait until you can provide some direction on how to do the layer reconstruction upon deserialization.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@cisaacson, Did you try and see how much
fixedbitset
uses memory? Just to see if that reduction looks good or not.It could be possible to do some tricks to make sparse/dense bitmaps to take less space, but those might be format dependent and will make some bitsets use more memory, so not sure about them.
As for updating upper layers:
Let
b := bits in usize
If your the length of
layer0
isn
then the length oflayer1
has to bem = ceil(n/b)
This means that for
layer1
you can loop from0
to non-inclusivem
.For each iteration
j
we need to go through all bits of the correspondingusize
.For each bit
i
we check iflayer0[(j * b) + i ] != 0
to see if the current bit should be1
or not.This same works with pretty much all layers.
(Was away, so got to answer this only now)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@WaDelma Thanks that makes sense.
I did some more work and research on this, I don't expect there will be much savings for reasonable size BitSets that are sparse over the
serde
method. Justlayer0
for 100K bits is about 12.5K, which is what you would expect, I don't have exact stats any more butserde
was about 12.7K. For my use caseserde
will work for sure.I also did some homework on sparse bitsets and how to compress them. This is a challenging area (Roaring has done a tremendous amount of work on this). My use case is definitely random and sparse, which makes it virtually impossible to get good compression. Your library is better than Roaring in terms of space by about 25%, because once you have a random sparse BitSet things like run-length encoding only slow it down and take more space. I'm very happy with Hibitset, we will definitely be using it. It's ideal for our use case, but serializing to a smaller size does not seem too feasible right now.
Building the layers as you state would be faster using the layer0 serialization, but again I don't know if it is worth it over using the
serde
implementation.Let me know what you think, I am good for now with what you have provided and the enhancements I proposed for
serde
.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, its maybe better to start with serde generated implementation and then maybe optimize it later if necessary.