Skip to content
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

#10: Feat - LFU cache implementation with approximate counting. #476

Merged
merged 18 commits into from
Sep 17, 2024

Conversation

Rits1272
Copy link
Contributor

@Rits1272 Rits1272 commented Sep 7, 2024

Fixes #10

  • LFU cache eviction policy support in Dice

How LFU cache is implemented?

  • The implementation is similar to Redis - details
  • The access counter of a key is piggybacked to the LastAccessedAt field in Dice Obj defined here. The first 8 bits is allocated to maintaining the access counter and the last 24 bits are allocated to maintaining the last access time of a key
  • The access counter is an approximate counter which is incremented logarithmically
  • This PR does not include support for the decay mechanism as of yet.
  • This LFU implementation do not add any additional memory footprint

Issue: #10

@Rits1272
Copy link
Contributor Author

Rits1272 commented Sep 7, 2024

@JyotinderSingh please have a look. TIA!

Copy link
Collaborator

@JyotinderSingh JyotinderSingh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for introducing a new eviction algorithm to Dice @Rits1272! Please add comprehensive tests for these changes.

internal/store/evictionpool.go Show resolved Hide resolved
@arpitbbhayani
Copy link
Contributor

@Rits1272 Please squash your commits before you push. You can do this once all changes are done.

@JyotinderSingh
Copy link
Collaborator

Hey @Rits1272, could you please address the comments and fix any conflicts? This PR has been open a while, let's try to get it merged.

@Rits1272
Copy link
Contributor Author

@JyotinderSingh didn't get time this week, but will work on adding test cases, resolving conflicts and comments by this weekend

@JyotinderSingh JyotinderSingh self-requested a review September 15, 2024 18:24
@Rits1272
Copy link
Contributor Author

Rits1272 commented Sep 16, 2024

@JyotinderSingh apologies for the delay. I have added test cases, please review
Since the PR has gotten stale, there are many conflicts from previous commits. Can we use Squash and merge for this PR during merge to master?

Copy link
Collaborator

@JyotinderSingh JyotinderSingh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for adding support for LFU eviction algorithm, @Rits1272! The changes look good.
Approved

@JyotinderSingh JyotinderSingh changed the title Feat - LFU cache implementation #10: Feat - LFU cache implementation Sep 17, 2024
@JyotinderSingh JyotinderSingh changed the title #10: Feat - LFU cache implementation #10: Feat - LFU cache implementation with approximate counting. Sep 17, 2024
@JyotinderSingh JyotinderSingh merged commit 46699d2 into DiceDB:master Sep 17, 2024
2 checks passed
@Rits1272 Rits1272 deleted the feat-implement_LFU_cache branch September 18, 2024 05:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add support for LFU eviction using Approximate Counting
4 participants