fix(hll): preserve set coupon order when serializing#128
Conversation
|
Do we have a test to capture this issue? I can't simply understand the situation by barely reading this patch. |
I am working on a test but you were too fast 🏎️ ! Will mark as draft and work on it :) |
Great! I wonder if you'd suggest to cancel 0.3.0-rc.2 for this fix, or we may later release a (quick) 0.3.1 with this fix. |
|
It's very low priority fix which does not break compatibility. IMO we can release it in > 0.3.0. Merging this patch would just save an allocation and make it bit-perfect with other libraries when the insertion order is identical. |
Compact HLL Set serialization used to sort retained coupons before writing bytes. Java and C++ write compact Set coupons in hash table iteration order. Write the existing iterator output directly. This keeps the compact format and removes the extra allocation.
notfilippo
force-pushed
the
filippo.rossi/push-yoruxxvzvuqm
branch
from
0810c99 to
93e4152
Compare
|
Added the test. Not amazingly happy with it but I think it's ok. I might add some comments later. Will mark ready for review after I'm done thinking about it :) |
tisonkun
left a comment
tisonkun
left a comment
There was a problem hiding this comment.
Verified locally that the test case failed before the impl change.
Generally LGTM.
2 participants
Compact HLL Set serialization used to sort retained coupons before writing bytes. Java and C++ write compact Set coupons in hash table iteration order.
Write the existing iterator output directly. This keeps the compact format and removes the extra allocation.