From 2e9a0e7ec802a5ef3a4b831f6b5009bfab0226db Mon Sep 17 00:00:00 2001 From: Joey Robichaud Date: Wed, 4 Dec 2024 23:39:10 -0800 Subject: [PATCH 1/7] Cache the MEF composition in the Roslyn LSP. --- .../LanguageServerTestComposition.cs | 2 +- .../ExportProviderBuilder.cs | 102 +++++++++++++++--- .../Program.cs | 4 +- 3 files changed, 94 insertions(+), 14 deletions(-) diff --git a/src/LanguageServer/Microsoft.CodeAnalysis.LanguageServer.UnitTests/Utilities/LanguageServerTestComposition.cs b/src/LanguageServer/Microsoft.CodeAnalysis.LanguageServer.UnitTests/Utilities/LanguageServerTestComposition.cs index b0406766a5b72..d56b4f2f6ae59 100644 --- a/src/LanguageServer/Microsoft.CodeAnalysis.LanguageServer.UnitTests/Utilities/LanguageServerTestComposition.cs +++ b/src/LanguageServer/Microsoft.CodeAnalysis.LanguageServer.UnitTests/Utilities/LanguageServerTestComposition.cs @@ -39,6 +39,6 @@ public static Task CreateExportProviderAsync( ExtensionLogDirectory: string.Empty); var extensionManager = ExtensionAssemblyManager.Create(serverConfiguration, loggerFactory); assemblyLoader = new CustomExportAssemblyLoader(extensionManager, loggerFactory); - return ExportProviderBuilder.CreateExportProviderAsync(extensionManager, assemblyLoader, devKitDependencyPath, loggerFactory); + return ExportProviderBuilder.CreateExportProviderAsync(extensionManager, assemblyLoader, devKitDependencyPath, cacheDirectory: null, loggerFactory); } } diff --git a/src/LanguageServer/Microsoft.CodeAnalysis.LanguageServer/ExportProviderBuilder.cs b/src/LanguageServer/Microsoft.CodeAnalysis.LanguageServer/ExportProviderBuilder.cs index 47139da193da3..c8ee428bb682d 100644 --- a/src/LanguageServer/Microsoft.CodeAnalysis.LanguageServer/ExportProviderBuilder.cs +++ b/src/LanguageServer/Microsoft.CodeAnalysis.LanguageServer/ExportProviderBuilder.cs @@ -3,6 +3,8 @@ // See the LICENSE file in the project root for more information. using System.Collections.Immutable; +using System.Security.Cryptography; +using System.Text; using Microsoft.CodeAnalysis.LanguageServer.Logging; using Microsoft.CodeAnalysis.LanguageServer.Services; using Microsoft.CodeAnalysis.Shared.Collections; @@ -18,6 +20,7 @@ public static async Task CreateExportProviderAsync( ExtensionAssemblyManager extensionManager, IAssemblyLoader assemblyLoader, string? devKitDependencyPath, + string? cacheDirectory, ILoggerFactory loggerFactory) { var logger = loggerFactory.CreateLogger(); @@ -38,17 +41,60 @@ public static async Task CreateExportProviderAsync( // Add the extension assemblies to the MEF catalog. assemblyPaths = assemblyPaths.Concat(extensionManager.ExtensionAssemblyPaths); - logger.LogTrace($"Composing MEF catalog using:{Environment.NewLine}{string.Join($" {Environment.NewLine}", assemblyPaths)}."); + // Get the cached MEF composition or create a new one. + var exportProviderFactory = await GetCompositionConfigurationAsync(assemblyPaths.ToImmutableArray(), assemblyLoader, cacheDirectory, logger); + + // Create an export provider, which represents a unique container of values. + // You can create as many of these as you want, but typically an app needs just one. + var exportProvider = exportProviderFactory.CreateExportProvider(); + + // Immediately set the logger factory, so that way it'll be available for the rest of the composition + exportProvider.GetExportedValue().SetFactory(loggerFactory); + + // Also add the ExtensionAssemblyManager so it will be available for the rest of the composition. + exportProvider.GetExportedValue().SetMefExtensionAssemblyManager(extensionManager); + + return exportProvider; + } + private static async Task GetCompositionConfigurationAsync( + ImmutableArray assemblyPaths, + IAssemblyLoader assemblyLoader, + string? cacheDirectory, + ILogger logger) + { // Create a MEF resolver that can resolve assemblies in the extension contexts. var resolver = new Resolver(assemblyLoader); + string? compositionCacheFile = cacheDirectory is not null + ? GetCompositionCacheFilePath(cacheDirectory, assemblyPaths) + : null; + + // Try to load a cached composition. + try + { + if (compositionCacheFile is not null && File.Exists(compositionCacheFile)) + { + logger.LogTrace($"Loading cached MEF catalog: {compositionCacheFile}"); + + CachedComposition cachedComposition = new(); + using FileStream cacheStream = new(compositionCacheFile, FileMode.Open, FileAccess.Read, FileShare.Read, 4096, useAsync: true); + return await cachedComposition.LoadExportProviderFactoryAsync(cacheStream, resolver); + } + } + catch (Exception ex) + { + // Log the error, and move on to recover by recreating the MEF composition. + logger.LogError($"Loading cached MEF composition failed: {ex}"); + } + + logger.LogTrace($"Composing MEF catalog using:{Environment.NewLine}{string.Join($" {Environment.NewLine}", assemblyPaths)}."); + var discovery = PartDiscovery.Combine( resolver, new AttributedPartDiscovery(resolver, isNonPublicSupported: true), // "NuGet MEF" attributes (Microsoft.Composition) new AttributedPartDiscoveryV1(resolver)); - // TODO - we should likely cache the catalog so we don't have to rebuild it every time. var catalog = ComposableCatalog.Create(resolver) .AddParts(await discovery.CreatePartsAsync(assemblyPaths)) .WithCompositionService(); // Makes an ICompositionService export available to MEF parts to import @@ -59,20 +105,52 @@ public static async Task CreateExportProviderAsync( // Verify we only have expected errors. ThrowOnUnexpectedErrors(config, catalog, logger); - // Prepare an ExportProvider factory based on this graph. - var exportProviderFactory = config.CreateExportProviderFactory(); + // Try to cache the composition. + if (compositionCacheFile is not null) + { + if (Path.GetDirectoryName(compositionCacheFile) is string directory) + { + Directory.CreateDirectory(directory); + } - // Create an export provider, which represents a unique container of values. - // You can create as many of these as you want, but typically an app needs just one. - var exportProvider = exportProviderFactory.CreateExportProvider(); + CachedComposition cachedComposition = new(); + var tempFilePath = Path.Combine(Path.GetTempPath(), Path.GetTempFileName()); + using (FileStream cacheStream = new(tempFilePath, FileMode.Create, FileAccess.Write, FileShare.None, 4096, useAsync: true)) + { + await cachedComposition.SaveAsync(config, cacheStream); + } - // Immediately set the logger factory, so that way it'll be available for the rest of the composition - exportProvider.GetExportedValue().SetFactory(loggerFactory); + File.Move(tempFilePath, compositionCacheFile, overwrite: true); + } - // Also add the ExtensionAssemblyManager so it will be available for the rest of the composition. - exportProvider.GetExportedValue().SetMefExtensionAssemblyManager(extensionManager); + // Prepare an ExportProvider factory based on this graph. + return config.CreateExportProviderFactory(); + } - return exportProvider; + private static string GetCompositionCacheFilePath(string cacheDirectory, ImmutableArray assemblyPaths) + { + // 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}"; + return Path.Combine(cacheDirectory, cacheSubdirectory, $"c#-languageserver.{ComputeAssemblyHash(assemblyPaths)}.mef-composition"); + + static string ComputeAssemblyHash(ImmutableArray assemblyPaths) + { + var assemblies = new StringBuilder(); + foreach (var assemblyPath in assemblyPaths) + { + // Include assembly path in the hash so that changes to the set of included + // assemblies cause the composition to be rebuilt. + assemblies.Append(assemblyPath); + // Include the last write time in the hash so that newer assemblies written + // to the same location cause the composition to be rebuilt. + assemblies.Append(File.GetLastWriteTimeUtc(assemblyPath).ToString("F")); + } + + var hash = SHA256.HashData(Encoding.UTF8.GetBytes(assemblies.ToString())); + // Convert to filename safe base64 string. + return Convert.ToBase64String(hash).Replace('+', '-').Replace('/', '_').TrimEnd('='); + } } private static void ThrowOnUnexpectedErrors(CompositionConfiguration configuration, ComposableCatalog catalog, ILogger logger) diff --git a/src/LanguageServer/Microsoft.CodeAnalysis.LanguageServer/Program.cs b/src/LanguageServer/Microsoft.CodeAnalysis.LanguageServer/Program.cs index efc71ddc547da..908ea6f6ce55f 100644 --- a/src/LanguageServer/Microsoft.CodeAnalysis.LanguageServer/Program.cs +++ b/src/LanguageServer/Microsoft.CodeAnalysis.LanguageServer/Program.cs @@ -86,7 +86,9 @@ static async Task RunAsync(ServerConfiguration serverConfiguration, Cancellation var assemblyLoader = new CustomExportAssemblyLoader(extensionManager, loggerFactory); 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"); + + using var exportProvider = await ExportProviderBuilder.CreateExportProviderAsync(extensionManager, assemblyLoader, serverConfiguration.DevKitDependencyPath, cacheDirectory, loggerFactory); // LSP server doesn't have the pieces yet to support 'balanced' mode for source-generators. Hardcode us to // 'automatic' for now. From c3d1800285c332b01509b2477d5839075b2e1952 Mon Sep 17 00:00:00 2001 From: Joey Robichaud Date: Thu, 5 Dec 2024 16:09:32 -0800 Subject: [PATCH 2/7] PR feedback --- .../ExportProviderBuilderTests.cs | 48 +++++++++++++ .../LspFileChangeWatcherTests.cs | 21 +++--- .../TelemetryReporterTests.cs | 15 ++-- .../AbstractLanguageServerHostTests.cs | 18 +++-- .../LanguageServerTestComposition.cs | 3 +- .../WorkspaceProjectFactoryServiceTests.cs | 6 +- .../ExportProviderBuilder.cs | 68 ++++++++++++------- .../Compiler/Core/Log/FunctionId.cs | 4 ++ 8 files changed, 131 insertions(+), 52 deletions(-) create mode 100644 src/LanguageServer/Microsoft.CodeAnalysis.LanguageServer.UnitTests/ExportProviderBuilderTests.cs diff --git a/src/LanguageServer/Microsoft.CodeAnalysis.LanguageServer.UnitTests/ExportProviderBuilderTests.cs b/src/LanguageServer/Microsoft.CodeAnalysis.LanguageServer.UnitTests/ExportProviderBuilderTests.cs new file mode 100644 index 0000000000000..a718c64961934 --- /dev/null +++ b/src/LanguageServer/Microsoft.CodeAnalysis.LanguageServer.UnitTests/ExportProviderBuilderTests.cs @@ -0,0 +1,48 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. +// See the LICENSE file in the project root for more information. + +using Xunit.Abstractions; + +namespace Microsoft.CodeAnalysis.LanguageServer.UnitTests +{ + public sealed class ExportProviderBuilderTests(ITestOutputHelper testOutputHelper) + : AbstractLanguageServerHostTests(testOutputHelper) + { + [Fact] + public async Task MefCompositionIsCached() + { + await using var testServer = await CreateLanguageServerAsync(includeDevKitComponents: false); + + var mefCompositions = Directory.EnumerateFiles(MefCacheDirectory.Path, "*.mef-composition", SearchOption.AllDirectories); + + Assert.Single(mefCompositions); + } + + [Fact] + public async Task MefCompositionIsReused() + { + await using var testServer = await CreateLanguageServerAsync(includeDevKitComponents: false); + + // Second test server with the same set of assemblies. + await using var testServer2 = await CreateLanguageServerAsync(includeDevKitComponents: false); + + var mefCompositions = Directory.EnumerateFiles(MefCacheDirectory.Path, "*.mef-composition", SearchOption.AllDirectories); + + Assert.Single(mefCompositions); + } + + [Fact] + public async Task MultipleMefCompositionsAreCached() + { + await using var testServer = await CreateLanguageServerAsync(includeDevKitComponents: false); + + // Second test server with a different set of assemblies. + await using var testServer2 = await CreateLanguageServerAsync(includeDevKitComponents: true); + + var mefCompositions = Directory.EnumerateFiles(MefCacheDirectory.Path, "*.mef-composition", SearchOption.AllDirectories); + + Assert.Equal(2, mefCompositions.Count()); + } + } +} diff --git a/src/LanguageServer/Microsoft.CodeAnalysis.LanguageServer.UnitTests/LspFileChangeWatcherTests.cs b/src/LanguageServer/Microsoft.CodeAnalysis.LanguageServer.UnitTests/LspFileChangeWatcherTests.cs index 9e3a07e88d383..900cf1a438c89 100644 --- a/src/LanguageServer/Microsoft.CodeAnalysis.LanguageServer.UnitTests/LspFileChangeWatcherTests.cs +++ b/src/LanguageServer/Microsoft.CodeAnalysis.LanguageServer.UnitTests/LspFileChangeWatcherTests.cs @@ -14,7 +14,8 @@ namespace Microsoft.CodeAnalysis.LanguageServer.UnitTests; -public class LspFileChangeWatcherTests : AbstractLanguageServerHostTests +public class LspFileChangeWatcherTests(ITestOutputHelper testOutputHelper) + : AbstractLanguageServerHostTests(testOutputHelper) { private readonly ClientCapabilities _clientCapabilitiesWithFileWatcherSupport = new ClientCapabilities { @@ -24,14 +25,10 @@ public class LspFileChangeWatcherTests : AbstractLanguageServerHostTests } }; - public LspFileChangeWatcherTests(ITestOutputHelper testOutputHelper) : base(testOutputHelper) - { - } - [Fact] public async Task LspFileWatcherNotSupportedWithoutClientSupport() { - await using var testLspServer = await TestLspServer.CreateAsync(new ClientCapabilities(), TestOutputLogger); + await using var testLspServer = await TestLspServer.CreateAsync(new ClientCapabilities(), TestOutputLogger, MefCacheDirectory.Path); Assert.False(LspFileChangeWatcher.SupportsLanguageServerHost(testLspServer.LanguageServerHost)); } @@ -39,7 +36,7 @@ public async Task LspFileWatcherNotSupportedWithoutClientSupport() [Fact] public async Task LspFileWatcherSupportedWithClientSupport() { - await using var testLspServer = await TestLspServer.CreateAsync(_clientCapabilitiesWithFileWatcherSupport, TestOutputLogger); + await using var testLspServer = await TestLspServer.CreateAsync(_clientCapabilitiesWithFileWatcherSupport, TestOutputLogger, MefCacheDirectory.Path); Assert.True(LspFileChangeWatcher.SupportsLanguageServerHost(testLspServer.LanguageServerHost)); } @@ -49,7 +46,7 @@ public async Task CreatingDirectoryWatchRequestsDirectoryWatch() { AsynchronousOperationListenerProvider.Enable(enable: true); - await using var testLspServer = await TestLspServer.CreateAsync(_clientCapabilitiesWithFileWatcherSupport, TestOutputLogger); + await using var testLspServer = await TestLspServer.CreateAsync(_clientCapabilitiesWithFileWatcherSupport, TestOutputLogger, MefCacheDirectory.Path); var lspFileChangeWatcher = new LspFileChangeWatcher( testLspServer.LanguageServerHost, testLspServer.ExportProvider.GetExportedValue()); @@ -57,8 +54,7 @@ public async Task CreatingDirectoryWatchRequestsDirectoryWatch() var dynamicCapabilitiesRpcTarget = new DynamicCapabilitiesRpcTarget(); testLspServer.AddClientLocalRpcTarget(dynamicCapabilitiesRpcTarget); - using var tempRoot = new TempRoot(); - var tempDirectory = tempRoot.CreateDirectory(); + var tempDirectory = TempRoot.CreateDirectory(); // Try creating a context and ensure we created the registration var context = lspFileChangeWatcher.CreateContext([new ProjectSystem.WatchedDirectory(tempDirectory.Path, extensionFilters: [])]); @@ -80,7 +76,7 @@ public async Task CreatingFileWatchRequestsFileWatch() { AsynchronousOperationListenerProvider.Enable(enable: true); - await using var testLspServer = await TestLspServer.CreateAsync(_clientCapabilitiesWithFileWatcherSupport, TestOutputLogger); + await using var testLspServer = await TestLspServer.CreateAsync(_clientCapabilitiesWithFileWatcherSupport, TestOutputLogger, MefCacheDirectory.Path); var lspFileChangeWatcher = new LspFileChangeWatcher( testLspServer.LanguageServerHost, testLspServer.ExportProvider.GetExportedValue()); @@ -88,8 +84,7 @@ public async Task CreatingFileWatchRequestsFileWatch() var dynamicCapabilitiesRpcTarget = new DynamicCapabilitiesRpcTarget(); testLspServer.AddClientLocalRpcTarget(dynamicCapabilitiesRpcTarget); - using var tempRoot = new TempRoot(); - var tempDirectory = tempRoot.CreateDirectory(); + var tempDirectory = TempRoot.CreateDirectory(); // Try creating a single file watch and ensure we created the registration var context = lspFileChangeWatcher.CreateContext([]); diff --git a/src/LanguageServer/Microsoft.CodeAnalysis.LanguageServer.UnitTests/TelemetryReporterTests.cs b/src/LanguageServer/Microsoft.CodeAnalysis.LanguageServer.UnitTests/TelemetryReporterTests.cs index d17821d269de5..40ca85d439650 100644 --- a/src/LanguageServer/Microsoft.CodeAnalysis.LanguageServer.UnitTests/TelemetryReporterTests.cs +++ b/src/LanguageServer/Microsoft.CodeAnalysis.LanguageServer.UnitTests/TelemetryReporterTests.cs @@ -9,16 +9,17 @@ namespace Microsoft.CodeAnalysis.LanguageServer.UnitTests; -public sealed class TelemetryReporterTests : AbstractLanguageServerHostTests +public sealed class TelemetryReporterTests(ITestOutputHelper testOutputHelper) + : AbstractLanguageServerHostTests(testOutputHelper) { - public TelemetryReporterTests(ITestOutputHelper testOutputHelper) - : base(testOutputHelper) - { - } - private async Task CreateReporterAsync() { - var exportProvider = await LanguageServerTestComposition.CreateExportProviderAsync(TestOutputLogger.Factory, includeDevKitComponents: true, out var _, out var _); + var exportProvider = await LanguageServerTestComposition.CreateExportProviderAsync( + TestOutputLogger.Factory, + includeDevKitComponents: true, + MefCacheDirectory.Path, + out var _, + out var _); // VS Telemetry requires this environment variable to be set. Environment.SetEnvironmentVariable("CommonPropertyBagPath", Path.GetTempFileName()); diff --git a/src/LanguageServer/Microsoft.CodeAnalysis.LanguageServer.UnitTests/Utilities/AbstractLanguageServerHostTests.cs b/src/LanguageServer/Microsoft.CodeAnalysis.LanguageServer.UnitTests/Utilities/AbstractLanguageServerHostTests.cs index 5b286f9873c04..72e68947fe666 100644 --- a/src/LanguageServer/Microsoft.CodeAnalysis.LanguageServer.UnitTests/Utilities/AbstractLanguageServerHostTests.cs +++ b/src/LanguageServer/Microsoft.CodeAnalysis.LanguageServer.UnitTests/Utilities/AbstractLanguageServerHostTests.cs @@ -3,6 +3,7 @@ // See the LICENSE file in the project root for more information. using Microsoft.CodeAnalysis.LanguageServer.LanguageServer; +using Microsoft.CodeAnalysis.Test.Utilities; using Microsoft.VisualStudio.Composition; using Nerdbank.Streams; using Roslyn.LanguageServer.Protocol; @@ -11,18 +12,27 @@ namespace Microsoft.CodeAnalysis.LanguageServer.UnitTests; -public abstract class AbstractLanguageServerHostTests +public abstract class AbstractLanguageServerHostTests : IDisposable { protected TestOutputLogger TestOutputLogger { get; } + protected TempRoot TempRoot { get; } + protected TempDirectory MefCacheDirectory { get; } protected AbstractLanguageServerHostTests(ITestOutputHelper testOutputHelper) { TestOutputLogger = new TestOutputLogger(testOutputHelper); + TempRoot = new(); + MefCacheDirectory = TempRoot.CreateDirectory(); } protected Task CreateLanguageServerAsync(bool includeDevKitComponents = true) { - return TestLspServer.CreateAsync(new ClientCapabilities(), TestOutputLogger, includeDevKitComponents); + return TestLspServer.CreateAsync(new ClientCapabilities(), TestOutputLogger, MefCacheDirectory.Path, includeDevKitComponents); + } + + public void Dispose() + { + TempRoot.Dispose(); } protected sealed class TestLspServer : IAsyncDisposable @@ -32,10 +42,10 @@ protected sealed class TestLspServer : IAsyncDisposable private ServerCapabilities? _serverCapabilities; - internal static async Task CreateAsync(ClientCapabilities clientCapabilities, TestOutputLogger logger, bool includeDevKitComponents = true) + internal static async Task CreateAsync(ClientCapabilities clientCapabilities, TestOutputLogger logger, string cacheDirectory, bool includeDevKitComponents = true) { var exportProvider = await LanguageServerTestComposition.CreateExportProviderAsync( - logger.Factory, includeDevKitComponents, out var _, out var assemblyLoader); + logger.Factory, includeDevKitComponents, cacheDirectory, out var _, out var assemblyLoader); var testLspServer = new TestLspServer(exportProvider, logger, assemblyLoader); var initializeResponse = await testLspServer.ExecuteRequestAsync(Methods.InitializeName, new InitializeParams { Capabilities = clientCapabilities }, CancellationToken.None); Assert.NotNull(initializeResponse?.Capabilities); diff --git a/src/LanguageServer/Microsoft.CodeAnalysis.LanguageServer.UnitTests/Utilities/LanguageServerTestComposition.cs b/src/LanguageServer/Microsoft.CodeAnalysis.LanguageServer.UnitTests/Utilities/LanguageServerTestComposition.cs index d56b4f2f6ae59..98273baaa9111 100644 --- a/src/LanguageServer/Microsoft.CodeAnalysis.LanguageServer.UnitTests/Utilities/LanguageServerTestComposition.cs +++ b/src/LanguageServer/Microsoft.CodeAnalysis.LanguageServer.UnitTests/Utilities/LanguageServerTestComposition.cs @@ -23,6 +23,7 @@ private static string GetDevKitExtensionPath() public static Task CreateExportProviderAsync( ILoggerFactory loggerFactory, bool includeDevKitComponents, + string cacheDirectory, out ServerConfiguration serverConfiguration, out IAssemblyLoader assemblyLoader) { @@ -39,6 +40,6 @@ public static Task CreateExportProviderAsync( ExtensionLogDirectory: string.Empty); var extensionManager = ExtensionAssemblyManager.Create(serverConfiguration, loggerFactory); assemblyLoader = new CustomExportAssemblyLoader(extensionManager, loggerFactory); - return ExportProviderBuilder.CreateExportProviderAsync(extensionManager, assemblyLoader, devKitDependencyPath, cacheDirectory: null, loggerFactory); + return ExportProviderBuilder.CreateExportProviderAsync(extensionManager, assemblyLoader, devKitDependencyPath, cacheDirectory, loggerFactory); } } diff --git a/src/LanguageServer/Microsoft.CodeAnalysis.LanguageServer.UnitTests/WorkspaceProjectFactoryServiceTests.cs b/src/LanguageServer/Microsoft.CodeAnalysis.LanguageServer.UnitTests/WorkspaceProjectFactoryServiceTests.cs index d8e07cd2cbd82..e1c81085d9ae8 100644 --- a/src/LanguageServer/Microsoft.CodeAnalysis.LanguageServer.UnitTests/WorkspaceProjectFactoryServiceTests.cs +++ b/src/LanguageServer/Microsoft.CodeAnalysis.LanguageServer.UnitTests/WorkspaceProjectFactoryServiceTests.cs @@ -7,17 +7,19 @@ using Microsoft.CodeAnalysis.Remote.ProjectSystem; using Microsoft.Extensions.Logging; using Microsoft.VisualStudio.Shell.ServiceBroker; +using Xunit.Abstractions; namespace Microsoft.CodeAnalysis.LanguageServer.UnitTests; -public class WorkspaceProjectFactoryServiceTests +public class WorkspaceProjectFactoryServiceTests(ITestOutputHelper testOutputHelper) + : AbstractLanguageServerHostTests(testOutputHelper) { [Fact] public async Task CreateProjectAndBatch() { var loggerFactory = new LoggerFactory(); using var exportProvider = await LanguageServerTestComposition.CreateExportProviderAsync( - loggerFactory, includeDevKitComponents: false, out var serverConfiguration, out var _); + loggerFactory, includeDevKitComponents: false, MefCacheDirectory.Path, out var serverConfiguration, out var _); exportProvider.GetExportedValue() .InitializeConfiguration(serverConfiguration); diff --git a/src/LanguageServer/Microsoft.CodeAnalysis.LanguageServer/ExportProviderBuilder.cs b/src/LanguageServer/Microsoft.CodeAnalysis.LanguageServer/ExportProviderBuilder.cs index c8ee428bb682d..af6027cdc34e8 100644 --- a/src/LanguageServer/Microsoft.CodeAnalysis.LanguageServer/ExportProviderBuilder.cs +++ b/src/LanguageServer/Microsoft.CodeAnalysis.LanguageServer/ExportProviderBuilder.cs @@ -3,7 +3,7 @@ // See the LICENSE file in the project root for more information. using System.Collections.Immutable; -using System.Security.Cryptography; +using System.IO.Hashing; using System.Text; using Microsoft.CodeAnalysis.LanguageServer.Logging; using Microsoft.CodeAnalysis.LanguageServer.Services; @@ -11,6 +11,7 @@ using Microsoft.Extensions.Logging; using Microsoft.VisualStudio.Composition; using Roslyn.Utilities; +using RoslynLog = Microsoft.CodeAnalysis.Internal.Log; namespace Microsoft.CodeAnalysis.LanguageServer; @@ -60,32 +61,35 @@ public static async Task CreateExportProviderAsync( private static async Task GetCompositionConfigurationAsync( ImmutableArray assemblyPaths, IAssemblyLoader assemblyLoader, - string? cacheDirectory, + string cacheDirectory, ILogger logger) { // Create a MEF resolver that can resolve assemblies in the extension contexts. var resolver = new Resolver(assemblyLoader); - string? compositionCacheFile = cacheDirectory is not null - ? GetCompositionCacheFilePath(cacheDirectory, assemblyPaths) - : null; + var compositionCacheFile = GetCompositionCacheFilePath(cacheDirectory, assemblyPaths); // Try to load a cached composition. try { - if (compositionCacheFile is not null && File.Exists(compositionCacheFile)) + if (File.Exists(compositionCacheFile)) { logger.LogTrace($"Loading cached MEF catalog: {compositionCacheFile}"); CachedComposition cachedComposition = new(); - using FileStream cacheStream = new(compositionCacheFile, FileMode.Open, FileAccess.Read, FileShare.Read, 4096, useAsync: true); - return await cachedComposition.LoadExportProviderFactoryAsync(cacheStream, resolver); + using FileStream cacheStream = new(compositionCacheFile, FileMode.Open, FileAccess.Read, FileShare.Read, bufferSize: 4096, useAsync: true); + var exportProviderFactory = await cachedComposition.LoadExportProviderFactoryAsync(cacheStream, resolver); + + RoslynLog.Logger.Log(RoslynLog.FunctionId.LSP_MEF_Cache_Load_Success); + + return exportProviderFactory; } } catch (Exception ex) { // Log the error, and move on to recover by recreating the MEF composition. logger.LogError($"Loading cached MEF composition failed: {ex}"); + RoslynLog.Logger.Log(RoslynLog.FunctionId.LSP_MEF_Cache_Load_Failure); } logger.LogTrace($"Composing MEF catalog using:{Environment.NewLine}{string.Join($" {Environment.NewLine}", assemblyPaths)}."); @@ -105,23 +109,10 @@ private static async Task GetCompositionConfigurationAsy // Verify we only have expected errors. ThrowOnUnexpectedErrors(config, catalog, logger); - // Try to cache the composition. - if (compositionCacheFile is not null) - { - if (Path.GetDirectoryName(compositionCacheFile) is string directory) - { - Directory.CreateDirectory(directory); - } - - CachedComposition cachedComposition = new(); - var tempFilePath = Path.Combine(Path.GetTempPath(), Path.GetTempFileName()); - using (FileStream cacheStream = new(tempFilePath, FileMode.Create, FileAccess.Write, FileShare.None, 4096, useAsync: true)) - { - await cachedComposition.SaveAsync(config, cacheStream); - } + RoslynLog.Logger.Log(RoslynLog.FunctionId.LSP_MEF_Cache_Built); - File.Move(tempFilePath, compositionCacheFile, overwrite: true); - } + // Try to cache the composition. + _ = WriteCompositionCacheAsync(compositionCacheFile, config, logger); // Prepare an ExportProvider factory based on this graph. return config.CreateExportProviderFactory(); @@ -136,6 +127,9 @@ private static string GetCompositionCacheFilePath(string cacheDirectory, Immutab static string ComputeAssemblyHash(ImmutableArray assemblyPaths) { + // Ensure AssemblyPaths are always in the same order. + assemblyPaths = assemblyPaths.Sort(); + var assemblies = new StringBuilder(); foreach (var assemblyPath in assemblyPaths) { @@ -147,12 +141,36 @@ static string ComputeAssemblyHash(ImmutableArray assemblyPaths) assemblies.Append(File.GetLastWriteTimeUtc(assemblyPath).ToString("F")); } - var hash = SHA256.HashData(Encoding.UTF8.GetBytes(assemblies.ToString())); + var hash = XxHash128.Hash(Encoding.UTF8.GetBytes(assemblies.ToString())); // Convert to filename safe base64 string. return Convert.ToBase64String(hash).Replace('+', '-').Replace('/', '_').TrimEnd('='); } } + private static async Task WriteCompositionCacheAsync(string compositionCacheFile, CompositionConfiguration config, ILogger logger) + { + try + { + if (Path.GetDirectoryName(compositionCacheFile) is string directory) + { + Directory.CreateDirectory(directory); + } + + CachedComposition cachedComposition = new(); + var tempFilePath = Path.Combine(Path.GetTempPath(), Path.GetTempFileName()); + using (FileStream cacheStream = new(tempFilePath, FileMode.Create, FileAccess.Write, FileShare.None, bufferSize: 4096, useAsync: true)) + { + await cachedComposition.SaveAsync(config, cacheStream); + } + + File.Move(tempFilePath, compositionCacheFile, overwrite: true); + } + catch (Exception ex) + { + logger.LogError($"Failed to save MEF cache: {ex}"); + } + } + private static void ThrowOnUnexpectedErrors(CompositionConfiguration configuration, ComposableCatalog catalog, ILogger logger) { // Verify that we have exactly the MEF errors that we expect. If we have less or more this needs to be updated to assert the expected behavior. diff --git a/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/Log/FunctionId.cs b/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/Log/FunctionId.cs index 4f123aa508e83..a441dca7a8d34 100644 --- a/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/Log/FunctionId.cs +++ b/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/Log/FunctionId.cs @@ -639,4 +639,8 @@ internal enum FunctionId VSCode_LanguageServer_Started = 860, VSCode_Project_Load_Started = 861, VSCode_Projects_Load_Completed = 862, + + LSP_MEF_Cache_Load_Success = 865, + LSP_MEF_Cache_Load_Failure = 866, + LSP_MEF_Cache_Built = 867 } From 9953341160f89faa315cb64141df0276431878d4 Mon Sep 17 00:00:00 2001 From: Joey Robichaud Date: Thu, 5 Dec 2024 16:12:04 -0800 Subject: [PATCH 3/7] Update comment --- .../ExportProviderBuilder.cs | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/LanguageServer/Microsoft.CodeAnalysis.LanguageServer/ExportProviderBuilder.cs b/src/LanguageServer/Microsoft.CodeAnalysis.LanguageServer/ExportProviderBuilder.cs index af6027cdc34e8..e670fffb6a5b0 100644 --- a/src/LanguageServer/Microsoft.CodeAnalysis.LanguageServer/ExportProviderBuilder.cs +++ b/src/LanguageServer/Microsoft.CodeAnalysis.LanguageServer/ExportProviderBuilder.cs @@ -120,9 +120,12 @@ private static async Task GetCompositionConfigurationAsy private static string GetCompositionCacheFilePath(string cacheDirectory, ImmutableArray assemblyPaths) { - // Include the .NET runtime version in the cache path so that running on a newer - // runtime causes the cache to be rebuilt. + // This should vary based on .NET runtime major version so that as some of our processes switch between our target + // .NET version and the user's selected SDK runtime version (which may be newer), the MEF cache is kept isolated. + // This can be important when the MEF catalog records full assembly names such as "System.Runtime, 8.0.0.0" yet + // we might be running on .NET 7 or .NET 8, depending on the particular session and user settings. var cacheSubdirectory = $".NET {Environment.Version.Major}"; + return Path.Combine(cacheDirectory, cacheSubdirectory, $"c#-languageserver.{ComputeAssemblyHash(assemblyPaths)}.mef-composition"); static string ComputeAssemblyHash(ImmutableArray assemblyPaths) From ce0a59040060a11a9169d3782dd3a7bceeff442b Mon Sep 17 00:00:00 2001 From: Joey Robichaud Date: Fri, 6 Dec 2024 15:44:34 -0800 Subject: [PATCH 4/7] PR Feedback --- .../ExportProviderBuilder.cs | 8 ++------ .../Compiler/Core/Log/FunctionId.cs | 4 ---- 2 files changed, 2 insertions(+), 10 deletions(-) diff --git a/src/LanguageServer/Microsoft.CodeAnalysis.LanguageServer/ExportProviderBuilder.cs b/src/LanguageServer/Microsoft.CodeAnalysis.LanguageServer/ExportProviderBuilder.cs index e670fffb6a5b0..47fbb1b1bcb1f 100644 --- a/src/LanguageServer/Microsoft.CodeAnalysis.LanguageServer/ExportProviderBuilder.cs +++ b/src/LanguageServer/Microsoft.CodeAnalysis.LanguageServer/ExportProviderBuilder.cs @@ -11,7 +11,6 @@ using Microsoft.Extensions.Logging; using Microsoft.VisualStudio.Composition; using Roslyn.Utilities; -using RoslynLog = Microsoft.CodeAnalysis.Internal.Log; namespace Microsoft.CodeAnalysis.LanguageServer; @@ -80,8 +79,6 @@ private static async Task GetCompositionConfigurationAsy using FileStream cacheStream = new(compositionCacheFile, FileMode.Open, FileAccess.Read, FileShare.Read, bufferSize: 4096, useAsync: true); var exportProviderFactory = await cachedComposition.LoadExportProviderFactoryAsync(cacheStream, resolver); - RoslynLog.Logger.Log(RoslynLog.FunctionId.LSP_MEF_Cache_Load_Success); - return exportProviderFactory; } } @@ -89,7 +86,6 @@ private static async Task GetCompositionConfigurationAsy { // Log the error, and move on to recover by recreating the MEF composition. logger.LogError($"Loading cached MEF composition failed: {ex}"); - RoslynLog.Logger.Log(RoslynLog.FunctionId.LSP_MEF_Cache_Load_Failure); } logger.LogTrace($"Composing MEF catalog using:{Environment.NewLine}{string.Join($" {Environment.NewLine}", assemblyPaths)}."); @@ -109,8 +105,6 @@ private static async Task GetCompositionConfigurationAsy // Verify we only have expected errors. ThrowOnUnexpectedErrors(config, catalog, logger); - RoslynLog.Logger.Log(RoslynLog.FunctionId.LSP_MEF_Cache_Built); - // Try to cache the composition. _ = WriteCompositionCacheAsync(compositionCacheFile, config, logger); @@ -154,6 +148,8 @@ private static async Task WriteCompositionCacheAsync(string compositionCacheFile { try { + await Task.Yield(); + if (Path.GetDirectoryName(compositionCacheFile) is string directory) { Directory.CreateDirectory(directory); diff --git a/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/Log/FunctionId.cs b/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/Log/FunctionId.cs index a441dca7a8d34..4f123aa508e83 100644 --- a/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/Log/FunctionId.cs +++ b/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/Log/FunctionId.cs @@ -639,8 +639,4 @@ internal enum FunctionId VSCode_LanguageServer_Started = 860, VSCode_Project_Load_Started = 861, VSCode_Projects_Load_Completed = 862, - - LSP_MEF_Cache_Load_Success = 865, - LSP_MEF_Cache_Load_Failure = 866, - LSP_MEF_Cache_Built = 867 } From 81c77bb7ef564cb5d999cc63c275d09f8402e803 Mon Sep 17 00:00:00 2001 From: Joey Robichaud Date: Fri, 6 Dec 2024 15:51:04 -0800 Subject: [PATCH 5/7] Fixup --- .../ExportProviderBuilder.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/LanguageServer/Microsoft.CodeAnalysis.LanguageServer/ExportProviderBuilder.cs b/src/LanguageServer/Microsoft.CodeAnalysis.LanguageServer/ExportProviderBuilder.cs index 47fbb1b1bcb1f..379cf98333cd3 100644 --- a/src/LanguageServer/Microsoft.CodeAnalysis.LanguageServer/ExportProviderBuilder.cs +++ b/src/LanguageServer/Microsoft.CodeAnalysis.LanguageServer/ExportProviderBuilder.cs @@ -106,7 +106,7 @@ private static async Task GetCompositionConfigurationAsy ThrowOnUnexpectedErrors(config, catalog, logger); // Try to cache the composition. - _ = WriteCompositionCacheAsync(compositionCacheFile, config, logger); + _ = WriteCompositionCacheAsync(compositionCacheFile, config, logger).ReportNonFatalErrorAsync(); // Prepare an ExportProvider factory based on this graph. return config.CreateExportProviderFactory(); From 2acfc7423233757af637210edad90986eae34521 Mon Sep 17 00:00:00 2001 From: Joey Robichaud Date: Fri, 6 Dec 2024 16:42:20 -0800 Subject: [PATCH 6/7] fixup --- .../ExportProviderBuilder.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/LanguageServer/Microsoft.CodeAnalysis.LanguageServer/ExportProviderBuilder.cs b/src/LanguageServer/Microsoft.CodeAnalysis.LanguageServer/ExportProviderBuilder.cs index 379cf98333cd3..0d86219ccdc8c 100644 --- a/src/LanguageServer/Microsoft.CodeAnalysis.LanguageServer/ExportProviderBuilder.cs +++ b/src/LanguageServer/Microsoft.CodeAnalysis.LanguageServer/ExportProviderBuilder.cs @@ -20,7 +20,7 @@ public static async Task CreateExportProviderAsync( ExtensionAssemblyManager extensionManager, IAssemblyLoader assemblyLoader, string? devKitDependencyPath, - string? cacheDirectory, + string cacheDirectory, ILoggerFactory loggerFactory) { var logger = loggerFactory.CreateLogger(); From b7682881edb100b4bcfbabd19a7fbfde0bc4478e Mon Sep 17 00:00:00 2001 From: Joey Robichaud Date: Tue, 10 Dec 2024 01:22:20 -0800 Subject: [PATCH 7/7] Add TestAccessor for ExportProviderBuilder --- .../ExportProviderBuilderTests.cs | 35 ++++++++++++++++--- .../ExportProviderBuilder.cs | 16 ++++++++- 2 files changed, 45 insertions(+), 6 deletions(-) diff --git a/src/LanguageServer/Microsoft.CodeAnalysis.LanguageServer.UnitTests/ExportProviderBuilderTests.cs b/src/LanguageServer/Microsoft.CodeAnalysis.LanguageServer.UnitTests/ExportProviderBuilderTests.cs index a718c64961934..6b7684ac3697d 100644 --- a/src/LanguageServer/Microsoft.CodeAnalysis.LanguageServer.UnitTests/ExportProviderBuilderTests.cs +++ b/src/LanguageServer/Microsoft.CodeAnalysis.LanguageServer.UnitTests/ExportProviderBuilderTests.cs @@ -14,9 +14,9 @@ public async Task MefCompositionIsCached() { await using var testServer = await CreateLanguageServerAsync(includeDevKitComponents: false); - var mefCompositions = Directory.EnumerateFiles(MefCacheDirectory.Path, "*.mef-composition", SearchOption.AllDirectories); + await AssertCacheWriteWasAttemptedAsync(); - Assert.Single(mefCompositions); + AssertCachedCompositionCountEquals(expectedCount: 1); } [Fact] @@ -24,12 +24,14 @@ public async Task MefCompositionIsReused() { await using var testServer = await CreateLanguageServerAsync(includeDevKitComponents: false); + await AssertCacheWriteWasAttemptedAsync(); + // Second test server with the same set of assemblies. await using var testServer2 = await CreateLanguageServerAsync(includeDevKitComponents: false); - var mefCompositions = Directory.EnumerateFiles(MefCacheDirectory.Path, "*.mef-composition", SearchOption.AllDirectories); + AssertNoCacheWriteWasAttempted(); - Assert.Single(mefCompositions); + AssertCachedCompositionCountEquals(expectedCount: 1); } [Fact] @@ -37,12 +39,35 @@ public async Task MultipleMefCompositionsAreCached() { await using var testServer = await CreateLanguageServerAsync(includeDevKitComponents: false); + await AssertCacheWriteWasAttemptedAsync(); + // Second test server with a different set of assemblies. await using var testServer2 = await CreateLanguageServerAsync(includeDevKitComponents: true); + await AssertCacheWriteWasAttemptedAsync(); + + AssertCachedCompositionCountEquals(expectedCount: 2); + } + + private async Task AssertCacheWriteWasAttemptedAsync() + { + var cacheWriteTask = ExportProviderBuilder.TestAccessor.GetCacheWriteTask(); + Assert.NotNull(cacheWriteTask); + + await cacheWriteTask; + } + + private void AssertNoCacheWriteWasAttempted() + { + var cacheWriteTask2 = ExportProviderBuilder.TestAccessor.GetCacheWriteTask(); + Assert.Null(cacheWriteTask2); + } + + private void AssertCachedCompositionCountEquals(int expectedCount) + { var mefCompositions = Directory.EnumerateFiles(MefCacheDirectory.Path, "*.mef-composition", SearchOption.AllDirectories); - Assert.Equal(2, mefCompositions.Count()); + Assert.Equal(expectedCount, mefCompositions.Count()); } } } diff --git a/src/LanguageServer/Microsoft.CodeAnalysis.LanguageServer/ExportProviderBuilder.cs b/src/LanguageServer/Microsoft.CodeAnalysis.LanguageServer/ExportProviderBuilder.cs index 0d86219ccdc8c..a956bf0906829 100644 --- a/src/LanguageServer/Microsoft.CodeAnalysis.LanguageServer/ExportProviderBuilder.cs +++ b/src/LanguageServer/Microsoft.CodeAnalysis.LanguageServer/ExportProviderBuilder.cs @@ -16,6 +16,9 @@ namespace Microsoft.CodeAnalysis.LanguageServer; internal sealed class ExportProviderBuilder { + // For testing purposes, track the last cache write task. + private static Task? _cacheWriteTask; + public static async Task CreateExportProviderAsync( ExtensionAssemblyManager extensionManager, IAssemblyLoader assemblyLoader, @@ -23,6 +26,10 @@ public static async Task CreateExportProviderAsync( string cacheDirectory, ILoggerFactory loggerFactory) { + // Clear any previous cache write task, so that it is easy to discern whether + // a cache write was attempted. + _cacheWriteTask = null; + var logger = loggerFactory.CreateLogger(); var baseDirectory = AppContext.BaseDirectory; @@ -106,7 +113,7 @@ private static async Task GetCompositionConfigurationAsy ThrowOnUnexpectedErrors(config, catalog, logger); // Try to cache the composition. - _ = WriteCompositionCacheAsync(compositionCacheFile, config, logger).ReportNonFatalErrorAsync(); + _cacheWriteTask = WriteCompositionCacheAsync(compositionCacheFile, config, logger).ReportNonFatalErrorAsync(); // Prepare an ExportProvider factory based on this graph. return config.CreateExportProviderFactory(); @@ -199,4 +206,11 @@ private static void ThrowOnUnexpectedErrors(CompositionConfiguration configurati } } } + + internal static class TestAccessor + { +#pragma warning disable VSTHRD200 // Use "Async" suffix for async methods + public static Task? GetCacheWriteTask() => _cacheWriteTask; +#pragma warning restore VSTHRD200 // Use "Async" suffix for async methods + } }