From 02069cef3ee4f08b37a1b5f2371c26041c41a5bd Mon Sep 17 00:00:00 2001 From: Todd Grunke Date: Tue, 19 Mar 2024 07:15:33 -0700 Subject: [PATCH] Reduce File I/O under the AnalyzerAssemblyLoader folder (#72412) Assemblies and their corresponding resource dlls are copied/deleted under this folder on solution open. When opening Rosly, I see about 700 dlls copied/deleted under this folder. Over 90% of these dlls are resource dlls, not something in use for my setup. This change separates the code that ensures the assembly is shadow-copied properly from the code that ensures that it's supporting resource assemblies are shadow-copied properly. Resource assemblies as copied on assembly load demand. With this change locally, I see over 90% reduction in the number of these file I/O operations. --- .../AnalyzerAssemblyLoaderTests.cs | 52 +++++- .../AnalyzerAssemblyLoader.Core.cs | 11 +- .../AnalyzerAssemblyLoader.Desktop.cs | 26 ++- .../AnalyzerAssemblyLoader.cs | 160 +++++++++--------- .../DefaultAnalyzerAssemblyLoader.cs | 13 +- .../ShadowCopyAnalyzerAssemblyLoader.cs | 95 ++++++----- .../Remote/SnapshotSerializationTests.cs | 5 +- .../Host/RemoteAnalyzerAssemblyLoader.cs | 9 +- 8 files changed, 234 insertions(+), 137 deletions(-) diff --git a/src/Compilers/Core/CodeAnalysisTest/AnalyzerAssemblyLoaderTests.cs b/src/Compilers/Core/CodeAnalysisTest/AnalyzerAssemblyLoaderTests.cs index 1baae26943926..5c47171fb8a6f 100644 --- a/src/Compilers/Core/CodeAnalysisTest/AnalyzerAssemblyLoaderTests.cs +++ b/src/Compilers/Core/CodeAnalysisTest/AnalyzerAssemblyLoaderTests.cs @@ -5,23 +5,21 @@ using System; using System.Collections.Generic; using System.Collections.Immutable; +using System.Globalization; using System.IO; using System.Linq; using System.Reflection; using System.Runtime.CompilerServices; -using System.Runtime.InteropServices; using System.Text; using Microsoft.CodeAnalysis.CSharp; using Microsoft.CodeAnalysis.Diagnostics; -using Microsoft.CodeAnalysis.PooledObjects; using Microsoft.CodeAnalysis.Test.Utilities; using Roslyn.Test.Utilities; using Roslyn.Utilities; using Xunit; using Xunit.Abstractions; -using Xunit.Sdk; using Microsoft.CodeAnalysis.VisualBasic; -using Microsoft.CodeAnalysis.VisualBasic.Syntax; + #if NETCOREAPP using Roslyn.Test.Utilities.CoreClr; using System.Runtime.Loader; @@ -367,11 +365,32 @@ string getExpectedLoadPath(string path) if (path.EndsWith(".resources.dll", StringComparison.Ordinal)) { - return loader.GetRealSatelliteLoadPath(path) ?? ""; + return getRealSatelliteLoadPath(path) ?? ""; } return loader.GetRealAnalyzerLoadPath(path ?? ""); } + // When PreparePathToLoad is overridden this returns the most recent + // real path for the given analyzer satellite assembly path + string? getRealSatelliteLoadPath(string originalSatelliteFullPath) + { + // This is a satellite assembly, need to find the mapped path of the real assembly, then + // adjust that mapped path for the suffix of the satellite assembly + // + // Example of dll and it's corresponding satellite assembly + // + // c:\some\path\en-GB\util.resources.dll + // c:\some\path\util.dll + var assemblyFileName = Path.ChangeExtension(Path.GetFileNameWithoutExtension(originalSatelliteFullPath), ".dll"); + + var assemblyDir = Path.GetDirectoryName(originalSatelliteFullPath)!; + var cultureInfo = CultureInfo.GetCultureInfo(Path.GetFileName(assemblyDir)); + assemblyDir = Path.GetDirectoryName(assemblyDir)!; + + // Real assembly is located in the directory above this one + var assemblyPath = Path.Combine(assemblyDir, assemblyFileName); + return loader.GetRealSatelliteLoadPath(assemblyPath, cultureInfo); + } } private static void VerifyAssemblies(AnalyzerAssemblyLoader loader, IEnumerable assemblies, int? copyCount, params string[] assemblyPaths) @@ -1347,8 +1366,31 @@ public void AssemblyLoading_Resources(AnalyzerTestKind kind) // dlls don't apply for this count. VerifyDependencyAssemblies(loader, copyCount: 1, analyzerPath, analyzerResourcesPath); }); + } + [Theory] + [CombinatorialData] + public void AssemblyLoading_ResourcesInParent(AnalyzerTestKind kind) + { + Run(kind, static (AnalyzerAssemblyLoader loader, AssemblyLoadTestFixture testFixture) => + { + using var temp = new TempRoot(); + var tempDir = temp.CreateDirectory(); + var analyzerPath = tempDir.CreateFile("AnalyzerWithLoc.dll").CopyContentFrom(testFixture.AnalyzerWithLoc).Path; + var analyzerResourcesPath = tempDir.CreateDirectory("es").CreateFile("AnalyzerWithLoc.resources.dll").CopyContentFrom(testFixture.AnalyzerWithLocResourceEnGB).Path; + loader.AddDependencyLocation(analyzerPath); + var assembly = loader.LoadFromPath(analyzerPath); + var methodInfo = assembly + .GetType("AnalyzerWithLoc.Util")! + .GetMethod("Exec", BindingFlags.Static | BindingFlags.Public)!; + methodInfo.Invoke(null, ["es-ES"]); + + // The copy count is 1 here as only one real assembly was copied, the resource + // dlls don't apply for this count. + VerifyDependencyAssemblies(loader, copyCount: 1, analyzerPath, analyzerResourcesPath); + }); } + #if NETCOREAPP [Theory] diff --git a/src/Compilers/Core/Portable/DiagnosticAnalyzer/AnalyzerAssemblyLoader.Core.cs b/src/Compilers/Core/Portable/DiagnosticAnalyzer/AnalyzerAssemblyLoader.Core.cs index 25530780cd541..5e2a1d99f08dc 100644 --- a/src/Compilers/Core/Portable/DiagnosticAnalyzer/AnalyzerAssemblyLoader.Core.cs +++ b/src/Compilers/Core/Portable/DiagnosticAnalyzer/AnalyzerAssemblyLoader.Core.cs @@ -137,7 +137,7 @@ public DirectoryLoadContext(string directory, AnalyzerAssemblyLoader loader, Ass var assemblyPath = Path.Combine(Directory, simpleName + ".dll"); if (_loader.IsAnalyzerDependencyPath(assemblyPath)) { - (_, var loadPath, _) = _loader.GetAssemblyInfoForPath(assemblyPath); + (_, var loadPath) = _loader.GetAssemblyInfoForPath(assemblyPath); return loadCore(loadPath); } @@ -149,11 +149,11 @@ public DirectoryLoadContext(string directory, AnalyzerAssemblyLoader loader, Ass // loader has a mode where it loads from Stream though and the runtime will not handle // that automatically. Rather than bifurate our loading behavior between Disk and // Stream both modes just handle satellite loading directly - if (!string.IsNullOrEmpty(assemblyName.CultureName) && simpleName.EndsWith(".resources", StringComparison.Ordinal)) + if (assemblyName.CultureInfo is not null && simpleName.EndsWith(".resources", StringComparison.Ordinal)) { var analyzerFileName = Path.ChangeExtension(simpleName, ".dll"); var analyzerFilePath = Path.Combine(Directory, analyzerFileName); - var satelliteLoadPath = _loader.GetSatelliteInfoForPath(analyzerFilePath, assemblyName.CultureName); + var satelliteLoadPath = _loader.GetRealSatelliteLoadPath(analyzerFilePath, assemblyName.CultureInfo); if (satelliteLoadPath is not null) { return loadCore(satelliteLoadPath); @@ -166,7 +166,8 @@ public DirectoryLoadContext(string directory, AnalyzerAssemblyLoader loader, Ass // be necessary but msbuild target defaults have caused a number of customers to // fall into this path. See discussion here for where it comes up // https://github.com/dotnet/roslyn/issues/56442 - if (_loader.GetBestPath(assemblyName) is string bestRealPath) + var (_, bestRealPath) = _loader.GetBestPath(assemblyName); + if (bestRealPath is not null) { return loadCore(bestRealPath); } @@ -193,7 +194,7 @@ protected override IntPtr LoadUnmanagedDll(string unmanagedDllName) var assemblyPath = Path.Combine(Directory, unmanagedDllName + ".dll"); if (_loader.IsAnalyzerDependencyPath(assemblyPath)) { - (_, var loadPath, _) = _loader.GetAssemblyInfoForPath(assemblyPath); + (_, var loadPath) = _loader.GetAssemblyInfoForPath(assemblyPath); return LoadUnmanagedDllFromPath(loadPath); } diff --git a/src/Compilers/Core/Portable/DiagnosticAnalyzer/AnalyzerAssemblyLoader.Desktop.cs b/src/Compilers/Core/Portable/DiagnosticAnalyzer/AnalyzerAssemblyLoader.Desktop.cs index 338a506d841e1..b4d000e335a9d 100644 --- a/src/Compilers/Core/Portable/DiagnosticAnalyzer/AnalyzerAssemblyLoader.Desktop.cs +++ b/src/Compilers/Core/Portable/DiagnosticAnalyzer/AnalyzerAssemblyLoader.Desktop.cs @@ -5,6 +5,7 @@ #if !NETCOREAPP using System; +using System.Globalization; using System.IO; using System.Reflection; using System.Threading; @@ -99,11 +100,30 @@ internal bool EnsureResolvedUnhooked() { try { + const string resourcesExtension = ".resources"; var assemblyName = new AssemblyName(args.Name); - string? bestPath = GetBestPath(assemblyName); - if (bestPath is not null) + var simpleName = assemblyName.Name; + var isSatelliteAssembly = + assemblyName.CultureInfo is not null && + simpleName.EndsWith(resourcesExtension, StringComparison.Ordinal); + + if (isSatelliteAssembly) + { + // Satellite assemblies should get the best path information using the + // non-resource part of the assembly name. Once the path information is obtained + // GetSatelliteInfoForPath will translate to the resource assembly path. + assemblyName.Name = simpleName[..^resourcesExtension.Length]; + } + + var (originalPath, realPath) = GetBestPath(assemblyName); + if (isSatelliteAssembly && originalPath is not null) + { + realPath = GetRealSatelliteLoadPath(originalPath, assemblyName.CultureInfo); + } + + if (realPath is not null) { - return Assembly.LoadFrom(bestPath); + return Assembly.LoadFrom(realPath); } return null; diff --git a/src/Compilers/Core/Portable/DiagnosticAnalyzer/AnalyzerAssemblyLoader.cs b/src/Compilers/Core/Portable/DiagnosticAnalyzer/AnalyzerAssemblyLoader.cs index 106f1c0b357d4..6ec884ede8396 100644 --- a/src/Compilers/Core/Portable/DiagnosticAnalyzer/AnalyzerAssemblyLoader.cs +++ b/src/Compilers/Core/Portable/DiagnosticAnalyzer/AnalyzerAssemblyLoader.cs @@ -5,11 +5,10 @@ using System; using System.Collections.Generic; using System.Collections.Immutable; -using System.Diagnostics; +using System.Globalization; using System.IO; using System.Linq; using System.Reflection; -using Microsoft.CodeAnalysis.PooledObjects; using Roslyn.Utilities; namespace Microsoft.CodeAnalysis @@ -41,7 +40,17 @@ internal abstract partial class AnalyzerAssemblyLoader : IAnalyzerAssemblyLoader /// /// Access must be guarded by /// - private readonly Dictionary SatelliteCultureNames)?> _analyzerAssemblyInfoMap = new(); + private readonly Dictionary _analyzerAssemblyInfoMap = new(); + + /// + /// Mapping of analyzer dependency original full path and culture to the real satellite + /// assembly path. If the satellite assembly doesn't exist for the original analyzer and + /// culture, the real path value stored will be null. + /// + /// + /// Access must be guarded by + /// + private readonly Dictionary<(string OriginalAnalyzerPath, CultureInfo CultureInfo), string?> _analyzerSatelliteAssemblyRealPaths = new(); /// /// Maps analyzer dependency simple names to the set of original full paths it was loaded from. This _only_ @@ -94,7 +103,7 @@ public void AddDependencyLocation(string fullPath) _knownAssemblyPathsBySimpleName[simpleName] = paths.Add(fullPath); } - // This type assumses the file system is static for the duration of the + // This type assumes the file system is static for the duration of the // it's instance. Repeated calls to this method, even if the underlying // file system contents, should reuse the results of the first call. _ = _analyzerAssemblyInfoMap.TryAdd(fullPath, null); @@ -105,7 +114,7 @@ public Assembly LoadFromPath(string originalAnalyzerPath) { CompilerPathUtilities.RequireAbsolutePath(originalAnalyzerPath, nameof(originalAnalyzerPath)); - (AssemblyName? assemblyName, _, _) = GetAssemblyInfoForPath(originalAnalyzerPath); + (AssemblyName? assemblyName, _) = GetAssemblyInfoForPath(originalAnalyzerPath); // Not a managed assembly, nothing else to do if (assemblyName is null) @@ -132,7 +141,7 @@ public Assembly LoadFromPath(string originalAnalyzerPath) /// because we only want information for registered paths. Using unregistered paths inside the /// implementation should result in errors. /// - protected (AssemblyName? AssemblyName, string RealAssemblyPath, ImmutableHashSet SatelliteCultureNames) GetAssemblyInfoForPath(string originalAnalyzerPath) + protected (AssemblyName? AssemblyName, string RealAssemblyPath) GetAssemblyInfoForPath(string originalAnalyzerPath) { lock (_guard) { @@ -147,8 +156,7 @@ public Assembly LoadFromPath(string originalAnalyzerPath) } } - var resourceAssemblyCultureNames = getResourceAssemblyCultureNames(originalAnalyzerPath); - string realPath = PreparePathToLoad(originalAnalyzerPath, resourceAssemblyCultureNames); + string realPath = PreparePathToLoad(originalAnalyzerPath); AssemblyName? assemblyName; try { @@ -164,64 +172,77 @@ public Assembly LoadFromPath(string originalAnalyzerPath) lock (_guard) { - _analyzerAssemblyInfoMap[originalAnalyzerPath] = (assemblyName, realPath, resourceAssemblyCultureNames); + _analyzerAssemblyInfoMap[originalAnalyzerPath] = (assemblyName, realPath); } - return (assemblyName, realPath, resourceAssemblyCultureNames); + return (assemblyName, realPath); + } + + /// + /// Get the path a satellite assembly should be loaded from for the given original + /// analyzer path and culture + /// + /// + /// This is used during assembly resolve for satellite assemblies to determine the + /// path from where the satellite assembly should be loaded for the specified culture. + /// This method calls to ensure this path + /// contains the satellite assembly. + /// + internal string? GetRealSatelliteLoadPath(string originalAnalyzerPath, CultureInfo cultureInfo) + { + string? realSatelliteAssemblyPath = null; - // Discover the culture names for any satellite dlls related to this analyzer. These - // need to be understood when handling the resource loading in certain cases. - static ImmutableHashSet getResourceAssemblyCultureNames(string originalAnalyzerPath) + lock (_guard) { - var path = Path.GetDirectoryName(originalAnalyzerPath)!; - using var enumerator = Directory.EnumerateDirectories(path, "*").GetEnumerator(); - if (!enumerator.MoveNext()) + if (_analyzerSatelliteAssemblyRealPaths.TryGetValue((originalAnalyzerPath, cultureInfo), out realSatelliteAssemblyPath)) { - return ImmutableHashSet.Empty; + return realSatelliteAssemblyPath; } + } + + var actualCultureName = getSatelliteCultureName(originalAnalyzerPath, cultureInfo); + if (actualCultureName != null) + { + realSatelliteAssemblyPath = PrepareSatelliteAssemblyToLoad(originalAnalyzerPath, actualCultureName); + } + + lock (_guard) + { + _analyzerSatelliteAssemblyRealPaths[(originalAnalyzerPath, cultureInfo)] = realSatelliteAssemblyPath; + } + + return realSatelliteAssemblyPath; + // Discover the most specific culture name to use for the specified analyzer path and culture + static string? getSatelliteCultureName(string originalAnalyzerPath, CultureInfo cultureInfo) + { + var path = Path.GetDirectoryName(originalAnalyzerPath)!; var resourceFileName = GetSatelliteFileName(Path.GetFileName(originalAnalyzerPath)); - var builder = ImmutableHashSet.CreateBuilder(StringComparer.OrdinalIgnoreCase); - do + + while (cultureInfo != CultureInfo.InvariantCulture) { - var resourceFilePath = Path.Combine(enumerator.Current, resourceFileName); + var resourceFilePath = Path.Combine(path, cultureInfo.Name, resourceFileName); + if (File.Exists(resourceFilePath)) { - builder.Add(Path.GetFileName(enumerator.Current)); + return cultureInfo.Name; } - } - while (enumerator.MoveNext()); - return builder.ToImmutableHashSet(); - } - } + cultureInfo = cultureInfo.Parent; + } - /// - /// Get the real load path of the satellite assembly given the original path to the analyzer - /// and the desired culture name. - /// - protected string? GetSatelliteInfoForPath(string originalAnalyzerPath, string cultureName) - { - var (_, realAssemblyPath, satelliteCultureNames) = GetAssemblyInfoForPath(originalAnalyzerPath); - if (!satelliteCultureNames.Contains(cultureName)) - { return null; } - - var satelliteFileName = GetSatelliteFileName(Path.GetFileName(realAssemblyPath)); - var dir = Path.GetDirectoryName(realAssemblyPath)!; - return Path.Combine(dir, cultureName, satelliteFileName); } /// - /// Return the best path for loading an assembly with the specified . This - /// return is a real path to load, not an original path. + /// Return the best (original, real) path information for loading an assembly with the specified . /// - protected string? GetBestPath(AssemblyName requestedName) + protected (string? BestOriginalPath, string? BestRealPath) GetBestPath(AssemblyName requestedName) { if (requestedName.Name is null) { - return null; + return (null, null); } ImmutableHashSet? paths; @@ -229,17 +250,18 @@ static ImmutableHashSet getResourceAssemblyCultureNames(string originalA { if (!_knownAssemblyPathsBySimpleName.TryGetValue(requestedName.Name, out paths)) { - return null; + return (null, null); } } // Sort the candidate paths by ordinal, to ensure determinism with the same inputs if you were to have // multiple assemblies providing the same version. - string? bestPath = null; + string? bestRealPath = null; + string? bestOriginalPath = null; AssemblyName? bestName = null; foreach (var candidateOriginalPath in paths.OrderBy(StringComparer.Ordinal)) { - (AssemblyName? candidateName, string candidateRealPath, _) = GetAssemblyInfoForPath(candidateOriginalPath); + (AssemblyName? candidateName, string candidateRealPath) = GetAssemblyInfoForPath(candidateOriginalPath); if (candidateName is null) { continue; @@ -249,18 +271,19 @@ static ImmutableHashSet getResourceAssemblyCultureNames(string originalA { if (candidateName.Version == requestedName.Version) { - return candidateRealPath; + return (candidateOriginalPath, candidateRealPath); } if (bestName is null || candidateName.Version > bestName.Version) { - bestPath = candidateRealPath; + bestOriginalPath = candidateOriginalPath; + bestRealPath = candidateRealPath; bestName = candidateName; } } } - return bestPath; + return (bestOriginalPath, bestRealPath); } protected static string GetSatelliteFileName(string assemblyFileName) => @@ -270,15 +293,18 @@ protected static string GetSatelliteFileName(string assemblyFileName) => /// When overridden in a derived class, allows substituting an assembly path after we've /// identified the context to load an assembly in, but before the assembly is actually /// loaded from disk. This is used to substitute out the original path with the shadow-copied version. - /// - /// In the case the is moved to a new location then - /// the resource DLLs for the specified must also be - /// moved _but_ retain their original relative location. /// - protected abstract string PreparePathToLoad(string assemblyFilePath, ImmutableHashSet resourceAssemblyCultureNames); + protected abstract string PreparePathToLoad(string assemblyFilePath); + + /// + /// When overridden in a derived class, allows substituting a satellite assembly path after we've + /// identified the context to load a satellite assembly in, but before the satellite assembly is actually + /// loaded from disk. This is used to substitute out the original path with the shadow-copied version. + /// + protected abstract string PrepareSatelliteAssemblyToLoad(string assemblyFilePath, string cultureName); /// - /// When is overridden this returns the most recent + /// When is overridden this returns the most recent /// real path calculated for the /// internal string GetRealAnalyzerLoadPath(string originalFullPath) @@ -294,30 +320,6 @@ internal string GetRealAnalyzerLoadPath(string originalFullPath) } } - /// - /// When is overridden this returns the most recent - /// real path for the given analyzer satellite assembly path - /// - internal string? GetRealSatelliteLoadPath(string originalSatelliteFullPath) - { - // This is a satellite assembly, need to find the mapped path of the real assembly, then - // adjust that mapped path for the suffix of the satellite assembly - // - // Example of dll and it's corresponding satellite assembly - // - // c:\some\path\en-GB\util.resources.dll - // c:\some\path\util.dll - var assemblyFileName = Path.ChangeExtension(Path.GetFileNameWithoutExtension(originalSatelliteFullPath), ".dll"); - - var assemblyDir = Path.GetDirectoryName(originalSatelliteFullPath)!; - var cultureName = Path.GetFileName(assemblyDir); - assemblyDir = Path.GetDirectoryName(assemblyDir)!; - - // Real assembly is located in the directory above this one - var assemblyPath = Path.Combine(assemblyDir, assemblyFileName); - return GetSatelliteInfoForPath(assemblyPath, cultureName); - } - internal (string OriginalAssemblyPath, string RealAssemblyPath)[] GetPathMapSnapshot() { lock (_guard) diff --git a/src/Compilers/Core/Portable/DiagnosticAnalyzer/DefaultAnalyzerAssemblyLoader.cs b/src/Compilers/Core/Portable/DiagnosticAnalyzer/DefaultAnalyzerAssemblyLoader.cs index 10ebd0c290689..624768168d521 100644 --- a/src/Compilers/Core/Portable/DiagnosticAnalyzer/DefaultAnalyzerAssemblyLoader.cs +++ b/src/Compilers/Core/Portable/DiagnosticAnalyzer/DefaultAnalyzerAssemblyLoader.cs @@ -32,7 +32,18 @@ internal DefaultAnalyzerAssemblyLoader(System.Runtime.Loader.AssemblyLoadContext /// /// The default implementation is to simply load in place. /// - protected override string PreparePathToLoad(string fullPath, ImmutableHashSet satelliteCultureNames) => fullPath; + protected override string PreparePathToLoad(string fullPath) => fullPath; + + /// + /// The default implementation is to simply load in place. + /// + protected override string PrepareSatelliteAssemblyToLoad(string assemblyFilePath, string cultureName) + { + var directory = Path.GetDirectoryName(assemblyFilePath)!; + var fileName = GetSatelliteFileName(Path.GetFileName(assemblyFilePath)); + + return Path.Combine(directory, cultureName, fileName); + } /// /// Return an which does not lock assemblies on disk that is diff --git a/src/Compilers/Core/Portable/DiagnosticAnalyzer/ShadowCopyAnalyzerAssemblyLoader.cs b/src/Compilers/Core/Portable/DiagnosticAnalyzer/ShadowCopyAnalyzerAssemblyLoader.cs index db53a1ae198fc..40592947a8ac4 100644 --- a/src/Compilers/Core/Portable/DiagnosticAnalyzer/ShadowCopyAnalyzerAssemblyLoader.cs +++ b/src/Compilers/Core/Portable/DiagnosticAnalyzer/ShadowCopyAnalyzerAssemblyLoader.cs @@ -6,13 +6,9 @@ using System.Collections.Concurrent; using System.Collections.Generic; using System.IO; -using System.Linq; using System.Threading; using System.Threading.Tasks; -using System.Collections.Immutable; using Roslyn.Utilities; -using System.Reflection.PortableExecutable; -using System.Reflection.Metadata; #if NETCOREAPP using System.Runtime.Loader; @@ -39,6 +35,7 @@ internal sealed class ShadowCopyAnalyzerAssemblyLoader : AnalyzerAssemblyLoader private readonly Lazy<(string directory, Mutex)> _shadowCopyDirectoryAndMutex; private readonly ConcurrentDictionary> _mvidPathMap = new ConcurrentDictionary>(); + private readonly ConcurrentDictionary<(Guid, string), Task> _mvidSatelliteAssemblyPathMap = new ConcurrentDictionary<(Guid, string), Task>(); internal string BaseDirectory => _baseDirectory; @@ -129,22 +126,62 @@ private void DeleteLeftoverDirectories() } } - protected override string PreparePathToLoad(string originalAnalyzerPath, ImmutableHashSet cultureNames) + protected override string PreparePathToLoad(string originalAnalyzerPath) { var mvid = AssemblyUtilities.ReadMvid(originalAnalyzerPath); - if (_mvidPathMap.TryGetValue(mvid, out Task? copyTask)) + + return PrepareLoad(_mvidPathMap, mvid, copyAnalyzerContents); + + string copyAnalyzerContents() + { + var analyzerFileName = Path.GetFileName(originalAnalyzerPath); + var shadowDirectory = Path.Combine(_shadowCopyDirectoryAndMutex.Value.directory, mvid.ToString()); + var shadowAnalyzerPath = Path.Combine(shadowDirectory, analyzerFileName); + CopyFile(originalAnalyzerPath, shadowAnalyzerPath); + + return shadowAnalyzerPath; + } + } + + protected override string PrepareSatelliteAssemblyToLoad(string originalAnalyzerPath, string cultureName) + { + var mvid = AssemblyUtilities.ReadMvid(originalAnalyzerPath); + + return PrepareLoad(_mvidSatelliteAssemblyPathMap, (mvid, cultureName), copyAnalyzerContents); + + string copyAnalyzerContents() + { + var analyzerFileName = Path.GetFileName(originalAnalyzerPath); + var shadowDirectory = Path.Combine(_shadowCopyDirectoryAndMutex.Value.directory, mvid.ToString()); + var shadowAnalyzerPath = Path.Combine(shadowDirectory, analyzerFileName); + + var originalDirectory = Path.GetDirectoryName(originalAnalyzerPath)!; + var satelliteFileName = GetSatelliteFileName(analyzerFileName); + + var originalSatellitePath = Path.Combine(originalDirectory, cultureName, satelliteFileName); + var shadowSatellitePath = Path.Combine(shadowDirectory, cultureName, satelliteFileName); + CopyFile(originalSatellitePath, shadowSatellitePath); + + return shadowSatellitePath; + } + } + + private static string PrepareLoad(ConcurrentDictionary> mvidPathMap, TKey mvidKey, Func copyContents) + where TKey : notnull + { + if (mvidPathMap.TryGetValue(mvidKey, out Task? copyTask)) { return copyTask.Result; } var tcs = new TaskCompletionSource(); - var task = _mvidPathMap.GetOrAdd(mvid, tcs.Task); + var task = mvidPathMap.GetOrAdd(mvidKey, tcs.Task); if (object.ReferenceEquals(task, tcs.Task)) { // This thread won and we need to do the copy. try { - var shadowAnalyzerPath = copyAnalyzerContents(); + var shadowAnalyzerPath = copyContents(); tcs.SetResult(shadowAnalyzerPath); return shadowAnalyzerPath; } @@ -159,43 +196,19 @@ protected override string PreparePathToLoad(string originalAnalyzerPath, Immutab // This thread lost and we need to wait for the winner to finish the copy. return task.Result; } + } - string copyAnalyzerContents() + private static void CopyFile(string originalPath, string shadowCopyPath) + { + var directory = Path.GetDirectoryName(shadowCopyPath); + if (directory is null) { - var analyzerFileName = Path.GetFileName(originalAnalyzerPath); - var shadowDirectory = Path.Combine(_shadowCopyDirectoryAndMutex.Value.directory, mvid.ToString()); - var shadowAnalyzerPath = Path.Combine(shadowDirectory, analyzerFileName); - copyFile(originalAnalyzerPath, shadowAnalyzerPath); - - if (cultureNames.IsEmpty) - { - return shadowAnalyzerPath; - } - - var originalDirectory = Path.GetDirectoryName(originalAnalyzerPath)!; - var satelliteFileName = GetSatelliteFileName(analyzerFileName); - foreach (var cultureName in cultureNames) - { - var originalSatellitePath = Path.Combine(originalDirectory, cultureName, satelliteFileName); - var shadowSatellitePath = Path.Combine(shadowDirectory, cultureName, satelliteFileName); - copyFile(originalSatellitePath, shadowSatellitePath); - } - - return shadowAnalyzerPath; + throw new ArgumentException($"Shadow copy path '{shadowCopyPath}' must not be the root directory"); } - static void copyFile(string originalPath, string shadowCopyPath) - { - var directory = Path.GetDirectoryName(shadowCopyPath); - if (directory is null) - { - throw new ArgumentException($"Shadow copy path '{shadowCopyPath}' must not be the root directory"); - } - - _ = Directory.CreateDirectory(directory); - File.Copy(originalPath, shadowCopyPath); - ClearReadOnlyFlagOnFile(new FileInfo(shadowCopyPath)); - } + _ = Directory.CreateDirectory(directory); + File.Copy(originalPath, shadowCopyPath); + ClearReadOnlyFlagOnFile(new FileInfo(shadowCopyPath)); } private static void ClearReadOnlyFlagOnFiles(string directoryPath) diff --git a/src/VisualStudio/Core/Test.Next/Remote/SnapshotSerializationTests.cs b/src/VisualStudio/Core/Test.Next/Remote/SnapshotSerializationTests.cs index c0758bc51aeb3..44e0d9b7ccaaa 100644 --- a/src/VisualStudio/Core/Test.Next/Remote/SnapshotSerializationTests.cs +++ b/src/VisualStudio/Core/Test.Next/Remote/SnapshotSerializationTests.cs @@ -709,7 +709,10 @@ private static AnalyzerFileReference CreateShadowCopiedAnalyzerReference(TempRoo private class MissingAnalyzerLoader : AnalyzerAssemblyLoader { - protected override string PreparePathToLoad(string fullPath, ImmutableHashSet cultureNames) + protected override string PreparePathToLoad(string fullPath) + => throw new FileNotFoundException(fullPath); + + protected override string PrepareSatelliteAssemblyToLoad(string fullPath, string cultureName) => throw new FileNotFoundException(fullPath); } diff --git a/src/Workspaces/Remote/ServiceHub/Host/RemoteAnalyzerAssemblyLoader.cs b/src/Workspaces/Remote/ServiceHub/Host/RemoteAnalyzerAssemblyLoader.cs index 108cdd343422f..7417225aaebba 100644 --- a/src/Workspaces/Remote/ServiceHub/Host/RemoteAnalyzerAssemblyLoader.cs +++ b/src/Workspaces/Remote/ServiceHub/Host/RemoteAnalyzerAssemblyLoader.cs @@ -2,7 +2,6 @@ // The .NET Foundation licenses this file to you under the MIT license. // See the LICENSE file in the project root for more information. -using System.Collections.Immutable; using System.IO; namespace Microsoft.CodeAnalysis.Remote.Diagnostics @@ -21,10 +20,16 @@ public RemoteAnalyzerAssemblyLoader(string baseDirectory) _baseDirectory = baseDirectory; } - protected override string PreparePathToLoad(string fullPath, ImmutableHashSet cultureNames) + protected override string PreparePathToLoad(string fullPath) { var fixedPath = Path.GetFullPath(Path.Combine(_baseDirectory, Path.GetFileName(fullPath))); return File.Exists(fixedPath) ? fixedPath : fullPath; } + + protected override string PrepareSatelliteAssemblyToLoad(string fullPath, string cultureName) + { + var fixedPath = Path.GetFullPath(Path.Combine(_baseDirectory, cultureName, Path.GetFileName(fullPath))); + return File.Exists(fixedPath) ? fixedPath : fullPath; + } } }