-
Notifications
You must be signed in to change notification settings - Fork 0
feat: add semaphore to limit concurrency in capability requests #23
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,7 +1,8 @@ | ||
| from functools import cached_property | ||
|
|
||
| import anyio | ||
| import asyncer | ||
| from attrs import Factory, define | ||
| from attrs import Factory, define, field | ||
| from lsp_client.capability.request import ( | ||
| WithRequestDocumentSymbol, | ||
| WithRequestHover, | ||
|
|
@@ -29,6 +30,7 @@ | |
| @define | ||
| class ReferenceCapability(Capability[ReferenceRequest, ReferenceResponse]): | ||
| _cache: PaginationCache[ReferenceItem] = Factory(PaginationCache) | ||
| process_sem: anyio.Semaphore = field(default=anyio.Semaphore(32)) | ||
|
|
||
| @cached_property | ||
| def locate(self) -> LocateCapability: | ||
|
|
@@ -92,7 +94,8 @@ async def _process_reference( | |
| context_lines: int, | ||
| items: list[ReferenceItem], | ||
| ) -> None: | ||
| file_path = self.client.from_uri(loc.uri) | ||
| async with self.process_sem: | ||
| file_path = self.client.from_uri(loc.uri) | ||
|
Comment on lines
+97
to
+98
|
||
| content = await self.client.read_file(file_path) | ||
| reader = DocumentReader(content) | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -6,8 +6,9 @@ | |
| from pathlib import Path | ||
| from typing import override | ||
|
|
||
| import anyio | ||
| import asyncer | ||
| from attrs import define | ||
| from attrs import define, field | ||
| from lsp_client.capability.request import WithRequestRename | ||
| from lsp_client.protocol import CapabilityClientProtocol | ||
| from lsp_client.utils.types import lsp_type | ||
|
|
@@ -141,6 +142,8 @@ def should_exclude(uri: str) -> bool: | |
|
|
||
| @define | ||
| class RenamePreviewCapability(Capability[RenamePreviewRequest, RenamePreviewResponse]): | ||
| file_sem: anyio.Semaphore = field(default=anyio.Semaphore(32)) | ||
|
|
||
| @cached_property | ||
| def locate(self) -> LocateCapability: | ||
| return LocateCapability(self.client) | ||
|
|
@@ -208,11 +211,12 @@ async def _to_file_change( | |
| *, | ||
| reader: DocumentReader | None = None, | ||
| ) -> RenameFileChange | None: | ||
| if reader is None: | ||
| content = await self.client.read_file( | ||
| self.client.from_uri(uri, relative=False) | ||
| ) | ||
| reader = DocumentReader(content) | ||
| 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) | ||
|
Comment on lines
+214
to
+219
|
||
|
|
||
| diffs: list[RenameDiff] = [] | ||
| for edit in edits: | ||
|
|
@@ -246,6 +250,8 @@ async def _to_file_change( | |
|
|
||
| @define | ||
| class RenameExecuteCapability(Capability[RenameExecuteRequest, RenameExecuteResponse]): | ||
| file_sem: anyio.Semaphore = field(default=anyio.Semaphore(32)) | ||
|
|
||
| @override | ||
| async def __call__(self, req: RenameExecuteRequest) -> RenameExecuteResponse | None: | ||
| cached = _preview_cache.get(req.rename_id) | ||
|
|
@@ -301,11 +307,12 @@ async def _to_file_change( | |
| *, | ||
| reader: DocumentReader | None = None, | ||
| ) -> RenameFileChange | None: | ||
| if reader is None: | ||
| content = await self.client.read_file( | ||
| self.client.from_uri(uri, relative=False) | ||
| ) | ||
| reader = DocumentReader(content) | ||
| 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) | ||
|
Comment on lines
+314
to
+319
|
||
|
|
||
| diffs: list[RenameDiff] = [] | ||
| for edit in edits: | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.