fix: add LRU eviction to LSP client file and diagnostics tracking #7050
Conversation
|
The following comment was made by an LLM, it may be inaccurate: No duplicate PRs found |
There was a problem hiding this comment.
Pull request overview
This PR addresses memory leaks in the LSP client by implementing LRU (Least Recently Used) eviction for file and diagnostics tracking. It introduces a reusable LRUMap utility class and applies it to prevent unbounded memory growth during extended LSP sessions.
Key Changes:
- Created a generic
LRUMap<K, V>class with automatic eviction when capacity is exceeded - Applied LRU eviction to both
filesanddiagnosticstracking with a limit of 1000 entries per LSP client - Added comprehensive test coverage for the LRUMap implementation
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
packages/opencode/src/util/lru-map.ts |
New LRUMap utility class implementing LRU eviction policy using JavaScript Map's insertion order guarantee |
packages/opencode/test/util/lru-map.test.ts |
Comprehensive test suite with 10 test cases covering basic operations, eviction behavior, and edge cases |
packages/opencode/src/lsp/client.ts |
Migrated files object and diagnostics Map to use LRUMap with MAX_TRACKED_FILES capacity |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Thanks for your contribution! This PR doesn't have a linked issue. All PRs must reference an existing issue. Please:
See CONTRIBUTING.md for details. |
The LSP client's files object and diagnostics Map grew unbounded as files were opened during a session. Every unique file path got an entry that was never removed, causing memory to accumulate. Changes: - Add LRUMap utility class with configurable capacity - Use LRUMap for both files and diagnostics tracking in LSP client - Set MAX_TRACKED_FILES to 1000 per client - Add comprehensive tests for LRUMap behavior When capacity is exceeded, least recently used entries are evicted. This bounds memory usage while keeping recently accessed files tracked.
…validation, add tests
fwang
force-pushed
the
fix/lsp-client-file-tracking-leak
branch
from
6ff29c1 to
1fcd656
Compare
2 participants
Fixes #3013
Summary
LRUMaputility class with configurable max sizeProblem
The LSP client tracks files and diagnostics in unbounded Maps. In large codebases or long sessions, these maps grow without limit, contributing to memory pressure.
Solution
Replace plain
MapwithLRUMapthat automatically evicts least-recently-used entries when a maximum size is exceeded. Default limits:Testing
Added tests for LRUMap utility. LSP functionality preserved.