perf: upgrade analysis cache from FIFO to LRU eviction#246
Conversation
When a cache entry is hit, delete and re-insert it so it moves to the back of the Map's insertion-order. This means the entry deleted on eviction is always the least-recently-used rather than the first inserted, which better retains hot files (e.g. the file a developer is currently editing) across larger workspaces. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
askpt
changed the title
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #246 +/- ##
==========================================
+ Coverage 68.80% 68.83% +0.03%
==========================================
Files 8 8
Lines 2936 2939 +3
Branches 283 283
==========================================
+ Hits 2020 2023 +3
Misses 914 914
Partials 2 2 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
askpt
deleted the
repo-assist/perf-lru-cache-2026-04-07-75018df41e67a7cc
branch
There was a problem hiding this comment.
Pull request overview
This PR updates the in-memory analysis result cache used by MetricsAnalyzerFactory.analyzeFile to use LRU (least-recently-used) eviction semantics, improving cache hit rates in typical VS Code “switch between a few files” workflows.
Changes:
- Promotes cache entries on hit by
delete+setto move them to the most-recently-used position inMapinsertion order. - Keeps eviction logic (when full) but makes it effectively remove the least-recently-used entry due to the new promotion behavior.
- Updates the cache JSDoc to describe LRU eviction.
| const CACHE_MAX_SIZE = 20; | ||
|
|
||
| /** Cache of analysis results keyed by language + content hash. Evicts oldest entry when full. */ | ||
| /** Cache of analysis results keyed by language + content hash. Evicts least-recently-used entry when full. */ |
There was a problem hiding this comment.
Cache key generation includes sourceText.length in addition to language + hash, but the cache JSDoc says it’s keyed only by language + content hash. Please update the comment to match the actual key components (or remove the length component if it’s not needed).
| /** Cache of analysis results keyed by language + content hash. Evicts least-recently-used entry when full. */ | |
| /** Cache of analysis results keyed by language + source text length + content hash. Evicts least-recently-used entry when full. */ |
| if (cached) { | ||
| // Move to end to maintain LRU order (most recently used stays at back) | ||
| analysisCache.delete(cacheKey); | ||
| analysisCache.set(cacheKey, cached); | ||
| return cached; |
There was a problem hiding this comment.
LRU behavior is implemented by delete+set on cache hit, but there isn’t a test that verifies LRU eviction semantics (i.e., a recently-accessed entry should survive an eviction while the true least-recently-used entry is evicted). Please add a unit test that fills the cache, re-accesses one key to promote it, then inserts one more entry and asserts the promoted key is still cached (e.g., via reference equality) while the oldest non-promoted key is recomputed.
🤖 This PR was created by Repo Assist, an automated AI assistant.
Summary
The analysis result cache in
MetricsAnalyzerFactory.analyzeFilepreviously used FIFO eviction: when the cache was full, the first-inserted entry was deleted, regardless of whether it had been accessed recently.This changes it to LRU (Least Recently Used) eviction by deleting and re-inserting a cache entry on every hit, which moves it to the end of the
Map's insertion order. The entry at the front is then always the least recently used, so eviction removes the entry least likely to be needed again.Why this matters
In a typical VS Code workflow a developer cycles between a small number of open files. With LRU eviction those files stay warm in the 20-entry cache even in a large workspace, whereas with FIFO they could be evicted by infrequently-opened files that happened to be visited first.
Changes
src/metricsAnalyzer/metricsAnalyzerFactory.ts: on cache hit,delete+setto promote to MRU position; updated JSDoc comment.Test Status
npm run compile✅ no errorsnpm run lint✅ no errorsnpm run test:unit✅ all 35 existing tests passNo new tests in this PR (cache-behaviour tests are covered in the companion PR
repo-assist/test-coverage-improvements-2026-04-07).