Skip to content

Conversation

@johnwonder
Copy link

No description provided.

Copy link
Member

@enjoy-binbin enjoy-binbin left a comment

Choose a reason for hiding this comment

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

are you sure? i forgot the details but i see the function is named LFUGetTimeInMinutes

@johnwonder
Copy link
Author

are you sure? i forgot the details but i see the function is named LFUGetTimeInMinutes

yes,because "seconds resolution" points to the lruclock,and the "or alternatively the LFU counter" sentence that follows is specifically for the LFUGetTimeInMinutes method

@enjoy-binbin
Copy link
Member

ok, i see, however we have this #2857 change at the same time, i am guessing #2857 will make this PR become a dead code...

@johnwonder
Copy link
Author

ok, i see, however we have this #2857 change at the same time, i am guessing #2857 will make this PR become a dead code...

It's okay. I just noticed this issue in Redis,so I dropped by valkey to take a look

@codecov
Copy link

codecov bot commented Nov 28, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 72.46%. Comparing base (d16788e) to head (c4f4059).
⚠️ Report is 1 commits behind head on unstable.

Additional details and impacted files
@@             Coverage Diff              @@
##           unstable    #2882      +/-   ##
============================================
+ Coverage     72.45%   72.46%   +0.01%     
============================================
  Files           128      128              
  Lines         70485    70485              
============================================
+ Hits          51068    51078      +10     
+ Misses        19417    19407      -10     
Files with missing lines Coverage Δ
src/object.c 82.04% <ø> (ø)

... and 15 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@JimB123
Copy link
Member

JimB123 commented Dec 1, 2025

Yes, there were comments in 5 different files indicating the mechanism for LRU/LFU. With #2857 it localizes that knowledge and eliminates this comment issue.

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.

3 participants