feat: add semaphore to limit concurrency in capability requests #23
Conversation
There was a problem hiding this comment.
Pull request overview
This pull request adds semaphores to limit concurrency in language server capability operations. The changes aim to prevent overwhelming the language server when processing large numbers of concurrent requests by limiting parallel execution to 32 concurrent operations per capability.
Changes:
- Added
anyio.Semaphorefields (defaulting to 32 concurrent operations) toOutlineCapability,ReferenceCapability,DefinitionCapability,RenamePreviewCapability, andRenameExecuteCapability - Wrapped concurrent operations with semaphore locks to control resource usage
- Updated imports to include
anyioand thefieldfunction fromattrs
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| src/lsap/capability/rename.py | Added file_sem semaphore to both RenamePreviewCapability and RenameExecuteCapability classes to limit concurrent file operations |
| src/lsap/capability/reference.py | Added process_sem semaphore to ReferenceCapability to limit concurrent reference processing |
| src/lsap/capability/outline.py | Added hover_sem semaphore to OutlineCapability to limit concurrent hover requests |
| src/lsap/capability/definition.py | Added resolve_sem semaphore to DefinitionCapability to limit concurrent symbol resolution |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| async with self.file_sem: | ||
| if reader is None: | ||
| content = await self.client.read_file( | ||
| self.client.from_uri(uri, relative=False) | ||
| ) | ||
| reader = DocumentReader(content) |
There was a problem hiding this comment.
The semaphore only protects the file reading operation but releases before processing the edits (lines 317-340). This means that all the subsequent work (creating diffs, processing edits) happens outside the semaphore's protection, which defeats the purpose of limiting concurrency. The semaphore should wrap the entire function body to properly limit concurrent execution of this method.
| async with self.process_sem: | ||
| file_path = self.client.from_uri(loc.uri) |
There was a problem hiding this comment.
The semaphore only protects a synchronous URI-to-path conversion (line 98), while the actual I/O operation (reading the file on line 99) and all subsequent processing (lines 100-149) happen outside the semaphore's protection. This defeats the purpose of limiting concurrency. The semaphore should wrap the entire function body to properly limit concurrent execution of this method.
| async with self.resolve_sem: | ||
| target_file_path = self.client.from_uri(loc.uri) |
There was a problem hiding this comment.
The semaphore only protects a synchronous URI-to-path conversion (line 76), while the actual async work (resolving symbol information on lines 77-80) happens outside the semaphore's protection. This defeats the purpose of limiting concurrency. The semaphore should wrap the entire function body to properly limit concurrent execution of this method.
| async with self.file_sem: | ||
| if reader is None: | ||
| content = await self.client.read_file( | ||
| self.client.from_uri(uri, relative=False) | ||
| ) | ||
| reader = DocumentReader(content) |
There was a problem hiding this comment.
The semaphore only protects the file reading operation but releases before processing the edits (lines 221-244). This means that all the subsequent work (creating diffs, processing edits) happens outside the semaphore's protection, which defeats the purpose of limiting concurrency. The semaphore should wrap the entire function body to properly limit concurrent execution of this method.
|
Addresses feedback from @copilot-pull-request-reviewer: expanded the semaphore scope to cover the entire function body/processing block in , , and to ensure all async and CPU-intensive work is properly limited. |
|
@observerw I've opened a new pull request, #24, to work on those changes. Once the pull request is ready, I'll request review from you. |
Summary
anyio.SemaphoretoOutlineCapability,ReferenceCapability,DefinitionCapability,RenamePreviewCapability, andRenameExecuteCapability.