-
Notifications
You must be signed in to change notification settings - Fork 32
Add serialize deserialize methods to XorBinaryFuse16 #47
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
base: master
Are you sure you want to change the base?
Add serialize deserialize methods to XorBinaryFuse16 #47
Conversation
…thods' into sis/add-serialize-deserialize-methods
lemire
left a comment
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 PR makes sense but you are making unrelated changes like making a class final, commenting on the choice of the random generator.
Fixing our typos is fine, but please minimize changes.
|
This is nice and invited. |
|
hi @lemire thank you your quick response!
done. please take a look.
do you want me to extend the serialization/deserialization to all filters? I can do that. I wasn't just sure if you have concerns about the taken approach for the serialization (there could be other options and preferences starting from the hopefully deprecated some day java vanilla serialization, serialization through Input/OutputStream and others). I have choosed to work with ByteBuffers because that opens up for me a chance to read from the direct buffers without the need to copy to heap. |
|
Looks good !!! Could you extend it to Xor8, X16, XorBinaryFuse8, XorBinaryFuse16, XorBinaryFuse32? |
hi @lemire
nice to e-meet you. First of all I would like to say thank you for such great algorithms and implementation. This is literelly a treasure for my currrent work!
I want to transfer the
XorBinaryFuse16over the wires. Therefore I want to wrap the serialized filter data with an envolpe to support different underlying filters / algorithms (e.g. use RoaringBitmap serialized data if I really need in some edge cases exact filtering). So to avoid additional memory copying I thought do the logic around the givenByteBuffer. So the idea is the followingorg.fastfilter.Filter#getSerializedSizeand allocates aByteBufferof enough size, then it writes the headerorg.fastfilter.Filter#serialize(ByteBuffer buffer)to write the serialized filterorg.fastfilter.xor.XorBinaryFuse16#deserialize(ByteBuffer buffer)to create the filter.If this looks good to you I can expose the same approach to other filters implemented in the library.
Looking forward to hearing a feedback from you!