mirror of
https://github.com/Gitlawb/openclaude.git
synced 2026-08-24 02:34:15 -05:00
fix(cache): use a monotonic clock for conversation-cache LRU recency (#1965)
evictLRU picks the entry with the smallest recorded access order, but that order was set to accessOrder.size — which plateaus at ~maxSize once the cache is full and drops on delete, so it is not monotonic. After the cache saturates, every set/get stamps roughly the same value, and a freshly-inserted key can end up ranked below entries a prior get already bumped. The next insert then evicts the most-recently-touched key instead of the least-recently-used one, churning the 50-entry session-history cache (sessionHistory.ts) into needless refetches. Stamp recency from a monotonic counter that only ever increases.
This commit is contained in:
@@ -45,10 +45,30 @@ describe('conversationCache', () => {
|
||||
cache.get('a') // access 'a' to update order
|
||||
cache.set('c', [{ role: 'user', content: 'c' }])
|
||||
cache.set('d', [{ role: 'user', content: 'd' }]) // should evict 'b'
|
||||
|
||||
|
||||
expect(cache.get('b')).toBeUndefined()
|
||||
expect(cache.get('a')).toBeDefined()
|
||||
})
|
||||
|
||||
it('keeps a freshly-set entry once the cache is saturated', () => {
|
||||
// Fill to capacity, then bump the two older keys with gets. Once the cache
|
||||
// is full the recency clock must keep advancing; deriving it from map size
|
||||
// makes every touch tie at ~maxSize, so the just-inserted 'd' looks older
|
||||
// than the bumped 'b'/'c' and gets wrongly evicted on the next insert.
|
||||
cache.set('a', [{ role: 'user', content: 'a' }])
|
||||
cache.set('b', [{ role: 'user', content: 'b' }])
|
||||
cache.set('c', [{ role: 'user', content: 'c' }])
|
||||
cache.get('b')
|
||||
cache.get('c')
|
||||
cache.set('d', [{ role: 'user', content: 'd' }]) // evicts 'a' (true LRU)
|
||||
cache.set('e', [{ role: 'user', content: 'e' }]) // must evict 'b', not 'd'
|
||||
|
||||
expect(cache.get('a')).toBeUndefined()
|
||||
expect(cache.get('b')).toBeUndefined()
|
||||
expect(cache.get('d')).toBeDefined()
|
||||
expect(cache.get('c')).toBeDefined()
|
||||
expect(cache.get('e')).toBeDefined()
|
||||
})
|
||||
})
|
||||
|
||||
describe('TTL expiration', () => {
|
||||
|
||||
@@ -22,6 +22,12 @@ export class ConversationCache {
|
||||
private cache = new Map<string, CacheEntry<CacheMessage[]>>()
|
||||
private accessOrder = new Map<string, number>()
|
||||
private evictions = 0
|
||||
// Monotonic logical clock for recency. Must never decrease, so it can't be
|
||||
// derived from `accessOrder.size` — that plateaus at ~maxSize once the cache
|
||||
// is full (and drops on delete), which makes a freshly-set entry tie or even
|
||||
// rank older than entries a prior get already bumped, so evictLRU discards the
|
||||
// most-recently-touched key instead of the least.
|
||||
private accessClock = 0
|
||||
|
||||
private readonly maxSize: number
|
||||
private readonly ttlMs: number
|
||||
@@ -49,7 +55,7 @@ export class ConversationCache {
|
||||
timestamp: Date.now(),
|
||||
hits: 0,
|
||||
})
|
||||
this.accessOrder.set(key, this.accessOrder.size)
|
||||
this.accessOrder.set(key, ++this.accessClock)
|
||||
}
|
||||
|
||||
get(key: string): CacheMessage[] | undefined {
|
||||
@@ -62,7 +68,7 @@ export class ConversationCache {
|
||||
}
|
||||
|
||||
entry.hits++
|
||||
this.accessOrder.set(key, this.accessOrder.size)
|
||||
this.accessOrder.set(key, ++this.accessClock)
|
||||
return entry.value
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user