Repository navigation
Share metadata between lsp server instances in daemon mode - #84713
Conversation
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 8d3437a1-4998-4003-9741-faf9e1fce969
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 8d3437a1-4998-4003-9741-faf9e1fce969
Add a default-on metadata sharing configuration seam and compare two daemon clients with and without the shared backing metadata cache. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 8d3437a1-4998-4003-9741-faf9e1fce969
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 8d3437a1-4998-4003-9741-faf9e1fce969
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 8d3437a1-4998-4003-9741-faf9e1fce969
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 8d3437a1-4998-4003-9741-faf9e1fce969
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 8d3437a1-4998-4003-9741-faf9e1fce969
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 8d3437a1-4998-4003-9741-faf9e1fce969
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 8d3437a1-4998-4003-9741-faf9e1fce969
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 8d3437a1-4998-4003-9741-faf9e1fce969
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 8d3437a1-4998-4003-9741-faf9e1fce969
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 8d3437a1-4998-4003-9741-faf9e1fce969
|
Azure Pipelines: Successfully started running 2 pipeline(s). There may be pipelines that require an authorized user to comment /azp run to run. |
3c92f44 to
245a11d
Compare
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 8d3437a1-4998-4003-9741-faf9e1fce969
245a11d to
8f44cb7
Compare
|
|
||
| internal sealed class DocumentationProviderService : IDocumentationProviderService | ||
| { | ||
| private readonly ConcurrentDictionary<string, DocumentationProvider> _assemblyPathToDocumentationProviderMap = new(); |
There was a problem hiding this comment.
this cache wasn't doing anything really useful. This just caches the documentation provider against the metadata file path. But this documentation provider just used to create the MetadataReference which is cached by the MetadataReferenceCache
|
Azure Pipelines: Successfully started running 2 pipeline(s). There may be pipelines that require an authorized user to comment /azp run to run. |
There was a problem hiding this comment.
Pull request overview
This PR introduces a shared metadata reference cache intended to allow multiple LSP server instances (in daemon mode) to reuse loaded metadata and reduce incremental memory usage when opening additional solutions/servers.
Changes:
- Introduces an
IMetadataReferenceCacheServiceabstraction and refactorsIMetadataServiceto delegate reference caching to it. - Adds a process/host-wide shared cache implementation for Language Server daemon mode (
SharedMetadataReferenceCache) plus unit tests validating sharing, invalidation, and lifetime behavior. - Updates daemon benchmarks and test infrastructure to exercise solution-loading and capture memory deltas.
Show a summary per file
| File | Description |
|---|---|
| src/Workspaces/CoreTest/SolutionTests/MetadataServiceTests.cs | Adds regression tests around metadata reference properties and failure behavior. |
| src/Workspaces/Core/Portable/Workspace/Host/Metadata/MetadataServiceFactory.cs | Refactors metadata service to use IMetadataReferenceCacheService and provide a reference-creation callback. |
| src/Workspaces/Core/Portable/Workspace/Host/Metadata/MetadataReferenceCache.cs | Converts the existing cache into a workspace service implementing IMetadataReferenceCacheService. |
| src/Workspaces/Core/Portable/Workspace/Host/Metadata/IMetadataReferenceCacheService.cs | Introduces the workspace service contract for metadata reference caching. |
| src/Workspaces/Core/Portable/Workspace/Host/Documentation/DocumentationProviderServiceFactory.cs | Removes the old factory-based documentation provider service. |
| src/Workspaces/Core/Portable/Workspace/Host/Documentation/DocumentationProviderService.cs | Adds a simplified MEF-exported documentation provider service implementation. |
| src/Tools/IdeCoreBenchmarks/LanguageServer/LanguageServerDaemonBenchmarks.cs | Updates daemon benchmarks to create daemons per-iteration and log memory deltas. |
| src/Tools/IdeCoreBenchmarks/LanguageServer/LanguageServerBenchmarkHost.cs | Extends benchmark host wrapper to support solution opening. |
| src/LanguageServer/Microsoft.CodeAnalysis.LanguageServer/HostWorkspace/SharedMetadataReferenceCache.cs | Adds the shared weak cache used to share metadata references across daemon workspaces. |
| src/LanguageServer/Microsoft.CodeAnalysis.LanguageServer/HostWorkspace/LanguageServerMetadataReferenceCacheService.cs | Exports a host-layer IMetadataReferenceCacheService backed by the shared cache. |
| src/LanguageServer/Microsoft.CodeAnalysis.LanguageServer.UnitTests/Utilities/AbstractLanguageServerHostTests.cs | Adds OpenSolutionAsync support and factors common “project load completion” notification logic. |
| src/LanguageServer/Microsoft.CodeAnalysis.LanguageServer.UnitTests/HostWorkspace/SharedMetadataReferenceCacheTests.cs | Adds unit tests for concurrency, invalidation, weak-lifetime, and failure caching behavior. |
| src/LanguageServer/Microsoft.CodeAnalysis.LanguageServer.UnitTests/Daemon/LanguageServerProjectLoadingTests.cs | Adds coverage for loading .slnx solutions in both daemon and non-daemon configurations. |
| src/LanguageServer/Microsoft.CodeAnalysis.LanguageServer.UnitTests/Daemon/LanguageServerDaemonTests.cs | Adds a daemon test validating that metadata references unique to a closed solution can be collected while shared ones remain alive. |
Copilot's findings
- Files reviewed: 14/14 changed files
- Comments generated: 1
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 8d3437a1-4998-4003-9741-faf9e1fce969
There was a problem hiding this comment.
Copilot's findings
Suppressed comments (1)
src/Workspaces/Core/Portable/Workspace/Host/Metadata/MetadataServiceFactory.cs:37
- GetReference currently passes the instance method group
CreateReferenceinto the cache on every call. This creates a new delegate instance per invocation (even on cache hits), which can become a steady allocation source in metadata-heavy paths. Consider caching the delegate in a readonly field initialized in the constructor and reusing it for all calls.
public PortableExecutableReference GetReference(string resolvedPath, MetadataReferenceProperties properties)
=> _metadataReferenceCacheService.GetReference(
resolvedPath, properties, CreateReference);
- Files reviewed: 14/14 changed files
- Comments generated: 0 new
There was a problem hiding this comment.
Copilot's findings
Suppressed comments (1)
src/Workspaces/Core/Portable/Workspace/Host/Documentation/DocumentationProviderService.cs:27
DocumentationProviderService.GetDocumentationProviderno longer caches providers (previous implementation cached per assembly path), andXmlDocumentationProvider.CreateFromFilecreates a new provider instance each call and does aFile.Existscheck. IfPortableExecutableReferences are collected (weak metadata cache) and later recreated, this can cause repeated provider creation and potentially repeated XML doc parsing, which may regress perf for documentation-heavy scenarios. Consider reintroducing caching with a weak/evicting strategy (or otherwise documenting/justifying the tradeoff).
public DocumentationProvider GetDocumentationProvider(string assemblyPath)
{
if (assemblyPath == null)
throw new ArgumentNullException(nameof(assemblyPath));
return XmlDocumentationProvider.CreateFromFile(Path.ChangeExtension(assemblyPath, "xml"));
}
- Files reviewed: 14/14 changed files
- Comments generated: 1
Implements a process wide shared metadata cache for daemon mode.
This shows a ~500 MB decrease in incremental memory opening a second LSP server with Roslyn.slnx (with Roslyn.slnx already loaded in the first).
Microsoft Reviewers: Open in CodeFlow