Cache the MEF composition in the Roslyn LSP. by JoeRobich · Pull Request #76276 · dotnet/roslyn

JoeRobich · 2024-12-05T08:02:12Z

To improve startup time of the LSP we can cache the MEF composition and load it from file instead of building up a new composition.

The cache is invalidated when:

The location of the Microsoft.CodeAnalysis.LanguageServer changes (such as installing an updated C# extension or dotnet.server.path is updated).
Changing major .NET runtime version.
The set of assembly paths that make up the composition changes.
The last write time of any assembly that is part of the composition changes.

Resolves #68568

src/LanguageServer/Microsoft.CodeAnalysis.LanguageServer/ExportProviderBuilder.cs

jasonmalinowski · 2024-12-05T17:18:43Z

src/LanguageServer/Microsoft.CodeAnalysis.LanguageServer/ExportProviderBuilder.cs

+    {
+        // Include the .NET runtime version in the cache path so that running on a newer
+        // runtime causes the cache to be rebuilt.
+        var cacheSubdirectory = $".NET {Environment.Version.Major}";


Does that Version.Major mean a minor update won't create a new cache? Is that a problem?

I am not 100% sure. I took this bit from DevKit and have updated the comment to match theirs. I know we have changed target runtime from 8.0.9 to 8.0.10 and now 8.0.11 seemingly without incident.

src/LanguageServer/Microsoft.CodeAnalysis.LanguageServer/ExportProviderBuilder.cs

...r/Microsoft.CodeAnalysis.LanguageServer.UnitTests/Utilities/LanguageServerTestComposition.cs

src/LanguageServer/Microsoft.CodeAnalysis.LanguageServer/ExportProviderBuilder.cs

JoeRobich · 2024-12-06T00:19:02Z

@ToddGrun @dibarbet Ready for another review.

src/LanguageServer/Microsoft.CodeAnalysis.LanguageServer/ExportProviderBuilder.cs

dibarbet

generally looks good - assuming the telemetry is fixed/removed and the async call to save the cache is tracked

src/LanguageServer/Microsoft.CodeAnalysis.LanguageServer/ExportProviderBuilder.cs

ToddGrun · 2024-12-07T01:19:54Z

...LanguageServer/Microsoft.CodeAnalysis.LanguageServer.UnitTests/ExportProviderBuilderTests.cs

+        {
+            await using var testServer = await CreateLanguageServerAsync(includeDevKitComponents: false);
+
+            var mefCompositions = Directory.EnumerateFiles(MefCacheDirectory.Path, "*.mef-composition", SearchOption.AllDirectories);


var mefCompositions = Directory.EnumerateFiles(MefCacheDirectory.Path, "*.mef-composition", SearchOption.AllDirectories);

Sorry, I'm probably being dense here. Since the saving is asynchronous, how do you know that it's completed before we check for it here?

ah, thanks. Will fix up.

@ToddGrun Updated.

Out of curiosity, I thought the IAsynchronousOperationListener was the way that we usually exposed async operations to tests. Was that not an option?

I will admit I have not tested a lot of these async operations. Looking through the source it seems the IAsynchronousOperationListenerProvider is typically MEF imported but this code is building the MEF composition, so we couldn't follow that pattern. Nothing is stopping us from creating our own listener instance and making it available through the TestAccessor, but I am not sure that has any benefits over using a Task. If you know of any reasons, I am happy to rework this bit in a follow up.

nickodei · 2025-01-23T19:13:48Z

src/LanguageServer/Microsoft.CodeAnalysis.LanguageServer/Program.cs

    var typeRefResolver = new ExtensionTypeRefResolver(assemblyLoader, loggerFactory);

-    using var exportProvider = await ExportProviderBuilder.CreateExportProviderAsync(extensionManager, assemblyLoader, serverConfiguration.DevKitDependencyPath, loggerFactory);
+    var cacheDirectory = Path.Combine(Path.GetDirectoryName(typeof(Program).Assembly.Location)!, "cache");


Hey, I just wanted to mention that this breaks read-only filesystem systems like NixOS where this is throwing and Exception because it cant create the folder in the nix-store. It's a bit unfortunate that this path could not be configured:

Thanks @nickodei! I opened #76892 to track this.

JoeRobich requested a review from dibarbet December 5, 2024 08:02

JoeRobich requested a review from a team as a code owner December 5, 2024 08:02

ghost added Area-Infrastructure untriaged Issues and PRs which have not yet been triaged by a lead labels Dec 5, 2024

dotnet-policy-service bot added the VSCode label Dec 5, 2024

JoeRobich added Area-IDE LSP issues related to the roslyn language server protocol implementation and removed Area-Infrastructure untriaged Issues and PRs which have not yet been triaged by a lead labels Dec 5, 2024

JoeRobich force-pushed the dev/jorobich/cache-lsp-mef branch from 312cd6b to 6b38161 Compare December 5, 2024 08:27

Cache the MEF composition in the Roslyn LSP.

2e9a0e7

JoeRobich force-pushed the dev/jorobich/cache-lsp-mef branch from 6b38161 to 2e9a0e7 Compare December 5, 2024 08:42

ToddGrun reviewed Dec 5, 2024

View reviewed changes

src/LanguageServer/Microsoft.CodeAnalysis.LanguageServer/ExportProviderBuilder.cs Outdated Show resolved Hide resolved

ToddGrun reviewed Dec 5, 2024

View reviewed changes

src/LanguageServer/Microsoft.CodeAnalysis.LanguageServer/ExportProviderBuilder.cs Show resolved Hide resolved

ToddGrun reviewed Dec 5, 2024

View reviewed changes

src/LanguageServer/Microsoft.CodeAnalysis.LanguageServer/ExportProviderBuilder.cs Outdated Show resolved Hide resolved

ToddGrun reviewed Dec 5, 2024

View reviewed changes

src/LanguageServer/Microsoft.CodeAnalysis.LanguageServer/ExportProviderBuilder.cs Show resolved Hide resolved

jasonmalinowski reviewed Dec 5, 2024

View reviewed changes

dibarbet reviewed Dec 5, 2024

View reviewed changes

JoeRobich added 2 commits December 5, 2024 16:09

PR feedback

c3d1800

Update comment

9953341

ToddGrun reviewed Dec 6, 2024

View reviewed changes

src/LanguageServer/Microsoft.CodeAnalysis.LanguageServer/ExportProviderBuilder.cs Show resolved Hide resolved

dibarbet approved these changes Dec 6, 2024

View reviewed changes

src/LanguageServer/Microsoft.CodeAnalysis.LanguageServer/ExportProviderBuilder.cs Outdated Show resolved Hide resolved

src/LanguageServer/Microsoft.CodeAnalysis.LanguageServer/ExportProviderBuilder.cs Outdated Show resolved Hide resolved

build-analysis bot mentioned this pull request Dec 6, 2024

The active test run was aborted. Reason: Test host process crashed dotnet/dnceng#451

Closed

3 tasks

JoeRobich added 3 commits December 6, 2024 15:44

PR Feedback

ce0a590

Fixup

81c77bb

fixup

2acfc74

ToddGrun reviewed Dec 7, 2024

View reviewed changes

Add TestAccessor for ExportProviderBuilder

b768288

JoeRobich merged commit 7997469 into main Dec 10, 2024

dotnet-policy-service bot added this to the Next milestone Dec 10, 2024

This was referenced Dec 12, 2024

[Automated] PRs inserted in VS build feature.debugger.main-35611.202 #76388

Closed

[Automated] PRs inserted in VS build main-35611.165 #76389

Closed

[Automated] PRs inserted in VS build feature.debugger.shadowDebug-35613.136 #76432

Closed

JoeRobich deleted the dev/jorobich/cache-lsp-mef branch December 20, 2024 18:57

dibarbet modified the milestones: Next, 17.13 P3 Jan 7, 2025

nickodei reviewed Jan 23, 2025

View reviewed changes

JoeRobich mentioned this pull request Jan 23, 2025

LanguageServer MEF composition caching is broken on NixOS #76892

Open

davidwengier mentioned this pull request May 7, 2025

Cache our OOP MEF composition dotnet/razor#11830

Closed

Conversation

JoeRobich commented Dec 5, 2024

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

JoeRobich commented Dec 6, 2024

Uh oh!

Uh oh!

dibarbet left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants