fix(lru): actually evict stale entries from TtlLruCache on lookup#2974
Open
Tatlatat wants to merge 1 commit into
Open
fix(lru): actually evict stale entries from TtlLruCache on lookup#2974Tatlatat wants to merge 1 commit into
Tatlatat wants to merge 1 commit into
Conversation
TtlLruCache is documented as "a stale hit is treated as a miss and evicted", but get() returned undefined for an expired entry without removing it. Worse, the preceding this.inner.get(key) had already re-inserted (promoted) the entry to most-recently-used, so a stale value lingered in memory and wrongly outlived fresher entries during eviction — a slow leak in the gitignore and directory-listing caches that call this on hot file-operation paths. Add a public delete(key) to LruCache and call it from TtlLruCache.get when an entry is expired, so the stale entry is evicted immediately as the comment promises. Add a vitest case (fake timers) asserting the expired key is gone from the underlying store and no longer survives eviction.
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
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.
What
TtlLruCacheis documented as "a stale hit is treated as a miss andevicted", but
get()didn't evict:Two problems for an expired key:
this.inner.get(key)has already re-inserted it asmost-recently-used, so the stale entry outlives fresher ones during
eviction.
TtlLruCachebacks the gitignore cache and the directory-listing cache (both5s TTL), which are hit on hot file-operation paths — so stale entries
accumulate and skew eviction under load.
Fix
delete(key)toLruCache.TtlLruCache.get, callthis.inner.delete(key)when the entry is expiredbefore returning
undefined, so it is evicted immediately as documented.Test
Appended a
TtlLruCachecase (fake timers): after the TTL elapses, a lookupreturns
undefined, the key is gone from the underlying store, and it no longersurvives subsequent evictions. Verified it fails on the old code and passes on
the fix.
Verification
npx vitest run tests/lru.test.ts(11 passed);biome checkclean on thechanged files.