Skip to content

Commit

Permalink
Reduce File I/O under the AnalyzerAssemblyLoader folder (dotnet#72412)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
ToddGrun authored Mar 19, 2024
1 parent 4880891 commit 02069ce
Show file tree
Hide file tree
Showing 8 changed files with 234 additions and 137 deletions.
52 changes: 47 additions & 5 deletions src/Compilers/Core/CodeAnalysisTest/AnalyzerAssemblyLoaderTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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<Assembly> assemblies, int? copyCount, params string[] assemblyPaths)
Expand Down Expand Up @@ -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]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

Expand All @@ -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);
Expand All @@ -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);
}
Expand All @@ -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);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
#if !NETCOREAPP

using System;
using System.Globalization;
using System.IO;
using System.Reflection;
using System.Threading;
Expand Down Expand Up @@ -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;
Expand Down
Loading

0 comments on commit 02069ce

Please sign in to comment.