-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
#238: PFADD/PFCOUNT implementation #422
#238: PFADD/PFCOUNT implementation #422
Conversation
@JyotinderSingh / @soumya-codes please review once. Few considerations have listed as part of description here. |
Could you resolve the conflicts for this PR? |
@JyotinderSingh conflicts resolved. Please review once. |
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.
Thanks for adding these changes @lucifercr07! The changes look good and well tested, just left one minor command.
Could you please create Issues for these TODOs to ensure we track them? |
Sure, will create. Will try to look into few of them as as part of |
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.
Thanks for adding HLL support to Dice @lucifercr07! The changes look good and well tested.
LGTM.
Sparse
representation and doesn't switch toDense
once threshold is reached as done by Redis. Also it doesn't do caching as part ofPFCOUNT
.PFMERGE, PFDEBUG
etc.:PFCOUNT
.Sparse
andDense
representations.Sparse
andDense
representations based on benchmark testing.