make caching/lru a real LRU#491
Open
omar-polo wants to merge 12 commits into
Open
Conversation
* add a test for more than one eviction * test Stats() as we operate on the cache
This makes it easier to implement a real LRU, since now the behaviour is that of a FIFO.
while here, also change the way it's managed: we push items to the head and evict from the tail.
Get now "bumps" the retrieved element at the top of the list, which makes eviction at the tail actually remove the last accessed item.
silently bump non-sensical values for size to a minimum of two, and while here add some doc for the New method.
Contributor
Author
|
open point: should i "bump" the value when Put overrides it? tempted to say yes. |
Contributor
Author
|
ah, also, (using this as a mental todo) i'd like to improve the |
in both cases, the breakage comes from the fact that the new LRU has an implicit bigger size, so tests that were relying on tiny capacities needed a fix. * cache_internal_test: add a few more nodes to fill the cache * iter_internal_test: flush the cache to restore previous behaviour
omar-polo
force-pushed
the
op/lru-really-lru
branch
from
July 12, 2026 17:08
43de094 to
dfe7598
Compare
Contributor
Yes, even though I don't think we have much code that uses this codepath. |
mathieu-plak
approved these changes
Jul 13, 2026
mathieu-plak
left a comment
mathieu-plak
left a comment
Contributor
There was a problem hiding this comment.
Looks good to me, I'm a tiny bit worried about the contention on Get but that shouldn't be too too bad.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
context: i've added originally this package to amortize the BTree operations, but I got a bit lazy. The improvements with the FIFO were already good enough that I didn't spend the last 20% of effort for making a real LRU.
Let's revisit that.
This is a preparatory step for further BTree work that I'd like to do.
While here, augment a bit the coverage, and add a benchmark, even if I'm not sure how much useful it is and might drop it.