fix: add LRU eviction to LSP client file and diagnostics tracking - #7050
fix: add LRU eviction to LSP client file and diagnostics tracking#7050hendem wants to merge 6 commits into
Conversation
|
The following comment was made by an LLM, it may be inaccurate: No duplicate PRs found |
Sorry, something went wrong.
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 files and diagnostics tracking 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.
Sorry, something went wrong.
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.
Sorry, something went wrong.
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.
Sorry, something went wrong.
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.
Sorry, something went wrong.
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.
Sorry, something went wrong.
|
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. |
Sorry, something went wrong.
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
|
Closing this pull request because it has had no updates for more than 60 days. If you plan to continue working on it, feel free to reopen or open a new PR. |
Sorry, something went wrong.
Fixes #3013
Summary
Problem
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 Map with LRUMap that automatically evicts least-recently-used entries when a maximum size is exceeded. Default limits:
Testing
Added tests for LRUMap utility. LSP functionality preserved.