From 2115bd9028a5e3e7ac0b10b9a38a5d374c93667d Mon Sep 17 00:00:00 2001 From: Siegfried Pammer Date: Fri, 4 Sep 2026 09:38:18 +0200 Subject: [PATCH] Read the file system once per reference closure, not per reference Resolving one assembly resolves its whole reference closure, and every reference in it asked the same framework directories the same questions. The worst of it was the scan for the closest version folder of a shared framework: a directory listing plus a recursive file search, repeated per reference and per runtime pack - 42 scans for two distinct answers when decompiling ICSharpCode.ILSpyX.dll. The scan result is only safe to keep for a bounded time: a runtime can be installed or removed while ILSpy runs, and reloading an assembly list has to see that. So it is kept for the length of an explicitly opened scope, which the type system opens around the closure it resolves and closes again afterwards; outside a scope the file system is read as before. The scope owns what was read, so two of them on one resolver do not stack - the first to end takes it, and the other reads the file system again. BeginSnapshot is on IAssemblyResolver rather than an interface of its own: it is core functionality of a resolver, and one implementation is not an abstraction. This breaks the interface for implementors outside this repository, who opt out by returning null - which is what the three resolvers here that hold nothing do. The remaining probes cost nothing to fix: the preferred runtime pack was listed among the defaults it already belongs to, so its directory was scanned twice for every reference that is not in it, and one package folder was probed once per assembly the package contains. Measured over 27 references with a fresh resolver each time: 3.3 ms per assembly before, 3.1 ms without a scope, 1.1 ms with one. Assisted-by: Claude:claude-opus-5:Claude Code --- .../ResolutionSnapshotTests.cs | 135 ++++++++++++++++++ .../DuplicateAssemblyReferenceTests.cs | 3 + .../Metadata/AssemblyReferences.cs | 6 + .../Metadata/DotNetCorePathFinder.cs | 40 ++++-- .../Metadata/UniversalAssemblyResolver.cs | 62 ++++++-- .../TypeSystem/DecompilerTypeSystem.cs | 8 +- ICSharpCode.ILSpyX/LoadedAssembly.cs | 6 + ICSharpCode.ILSpyX/LoadedPackage.cs | 3 + .../MissingReferencesTests.cs | 3 + TestTools/nugetfuzz.cs | 2 + 10 files changed, 244 insertions(+), 24 deletions(-) create mode 100644 ICSharpCode.Decompiler.Tests/ResolutionSnapshotTests.cs diff --git a/ICSharpCode.Decompiler.Tests/ResolutionSnapshotTests.cs b/ICSharpCode.Decompiler.Tests/ResolutionSnapshotTests.cs new file mode 100644 index 000000000..73b8d4989 --- /dev/null +++ b/ICSharpCode.Decompiler.Tests/ResolutionSnapshotTests.cs @@ -0,0 +1,135 @@ +// Copyright (c) 2026 Siegfried Pammer +// +// Permission is hereby granted, free of charge, to any person obtaining a copy of this +// software and associated documentation files (the "Software"), to deal in the Software +// without restriction, including without limitation the rights to use, copy, modify, merge, +// publish, distribute, sublicense, and/or sell copies of the Software, and to permit persons +// to whom the Software is furnished to do so, subject to the following conditions: +// +// The above copyright notice and this permission notice shall be included in all copies or +// substantial portions of the Software. +// +// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR IMPLIED, +// INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS FOR A PARTICULAR +// PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE +// FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR +// OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER +// DEALINGS IN THE SOFTWARE. + +using System.IO; + +using ICSharpCode.Decompiler.Metadata; + +using NUnit.Framework; + +namespace ICSharpCode.Decompiler.Tests +{ + [TestFixture] + public class ResolutionSnapshotTests + { + static UniversalAssemblyResolver Resolver() + { + return new UniversalAssemblyResolver(null, throwOnError: false, targetFramework: null); + } + + [Test] + public void DirectoryIsScannedOnceWhileASnapshotIsOpen() + { + var resolver = Resolver(); + int scans = 0; + + using (resolver.BeginSnapshot()) + { + for (int i = 0; i < 3; i++) + { + Assert.That(resolver.GetOrAddVersionFolder("/shared/pack", _ => { scans++; return "9.0.0"; }), + Is.EqualTo("9.0.0")); + } + } + + Assert.That(scans, Is.EqualTo(1)); + } + + [Test] + public void DirectoryIsScannedEveryTimeWithoutASnapshot() + { + var resolver = Resolver(); + int scans = 0; + + for (int i = 0; i < 3; i++) + { + resolver.GetOrAddVersionFolder("/shared/pack", _ => { scans++; return "9.0.0"; }); + } + + Assert.That(scans, Is.EqualTo(3)); + } + + [Test] + public void WhatWasScannedIsForgottenWhenTheSnapshotEnds() + { + // The point of the scope: outside it the file system is read afresh, so an assembly + // list reloaded after a runtime was installed or removed sees the new state. + var resolver = Resolver(); + int scans = 0; + + using (resolver.BeginSnapshot()) + { + resolver.GetOrAddVersionFolder("/shared/pack", _ => { scans++; return "9.0.0"; }); + } + using (resolver.BeginSnapshot()) + { + resolver.GetOrAddVersionFolder("/shared/pack", _ => { scans++; return "10.0.0"; }); + } + + Assert.That(scans, Is.EqualTo(2)); + } + + [Test] + public void ASecondScopeDoesNotStackWithTheFirst() + { + // Two decompilations can overlap on the resolver a LoadedAssembly owns. Whichever scope + // ends first takes the cache with it and the other reads the file system again, which + // costs that one its head start and nothing else. + var resolver = Resolver(); + int scans = 0; + + var outer = resolver.BeginSnapshot(); + var inner = resolver.BeginSnapshot(); + inner.Dispose(); + resolver.GetOrAddVersionFolder("/shared/pack", _ => { scans++; return "9.0.0"; }); + outer.Dispose(); + + Assert.That(scans, Is.EqualTo(1), "the scan happens, it is simply not held any more"); + } + + [Test] + public void ResolverFindsTheSameFileInsideAndOutsideASnapshot() + { + string directory = Path.Combine(Path.GetTempPath(), "ILSpySnapshotTest_" + Path.GetRandomFileName()); + Directory.CreateDirectory(directory); + try + { + string file = Path.Combine(directory, "SomeLibrary.dll"); + File.WriteAllBytes(file, new byte[0]); + + var resolver = new UniversalAssemblyResolver(null, throwOnError: false, targetFramework: null); + resolver.AddSearchDirectory(directory); + var reference = AssemblyNameReference.Parse("SomeLibrary, Version=1.0.0.0, Culture=neutral, PublicKeyToken=null"); + + string outside = resolver.FindAssemblyFile(reference); + string inside; + using (resolver.BeginSnapshot()) + { + inside = resolver.FindAssemblyFile(reference); + } + + Assert.That(outside, Is.EqualTo(file)); + Assert.That(inside, Is.EqualTo(file)); + } + finally + { + Directory.Delete(directory, recursive: true); + } + } + } +} diff --git a/ICSharpCode.Decompiler.Tests/TypeSystem/DuplicateAssemblyReferenceTests.cs b/ICSharpCode.Decompiler.Tests/TypeSystem/DuplicateAssemblyReferenceTests.cs index 00df00eeb..d3def0338 100644 --- a/ICSharpCode.Decompiler.Tests/TypeSystem/DuplicateAssemblyReferenceTests.cs +++ b/ICSharpCode.Decompiler.Tests/TypeSystem/DuplicateAssemblyReferenceTests.cs @@ -173,6 +173,9 @@ namespace ICSharpCode.Decompiler.Tests.TypeSystem /// class VersionedResolver : IAssemblyResolver { + /// + public IDisposable BeginSnapshot() => null; + static readonly string runtimeDirectory = Path.GetDirectoryName(typeof(object).Assembly.Location); readonly string directory; diff --git a/ICSharpCode.Decompiler/Metadata/AssemblyReferences.cs b/ICSharpCode.Decompiler/Metadata/AssemblyReferences.cs index baf120fec..f37fa25f4 100644 --- a/ICSharpCode.Decompiler/Metadata/AssemblyReferences.cs +++ b/ICSharpCode.Decompiler/Metadata/AssemblyReferences.cs @@ -64,6 +64,12 @@ namespace ICSharpCode.Decompiler.Metadata MetadataFile? ResolveModule(MetadataFile mainModule, string moduleName); Task ResolveAsync(IAssemblyReference reference); Task ResolveModuleAsync(MetadataFile mainModule, string moduleName); + + /// + /// Lets the resolver hold what it reads from the file system until the scope is disposed, + /// so that a whole reference closure asks the same directories once. Null to hold nothing. + /// + IDisposable? BeginSnapshot(); #endif } diff --git a/ICSharpCode.Decompiler/Metadata/DotNetCorePathFinder.cs b/ICSharpCode.Decompiler/Metadata/DotNetCorePathFinder.cs index d24cd5a0f..7f21542b1 100644 --- a/ICSharpCode.Decompiler/Metadata/DotNetCorePathFinder.cs +++ b/ICSharpCode.Decompiler/Metadata/DotNetCorePathFinder.cs @@ -78,6 +78,12 @@ namespace ICSharpCode.Decompiler.Metadata readonly string dotnetBasePath = FindDotNetExeDirectory(); readonly string preferredRuntimePack; + /// + /// The resolver this path finder belongs to, which holds any open scope. Null when nobody + /// owns it, and then every scan below reads the file system. + /// + internal UniversalAssemblyResolver Owner { get; set; } + public DotNetCorePathFinder(TargetFrameworkIdentifier targetFramework, Version targetFrameworkVersion, string preferredRuntimePack) { @@ -108,6 +114,10 @@ namespace ICSharpCode.Decompiler.Metadata { packages = LoadPackageInfos(depsJsonFileName, targetFrameworkIdString).ToArray(); + // Every runtime component of a package resolves to the same folder, so without + // this the same directory is probed once per assembly the package contains, for + // every reference that is looked up. + var knownPackageBasePaths = new HashSet(); foreach (var path in LookupPaths) { if (string.IsNullOrWhiteSpace(path)) @@ -120,7 +130,7 @@ namespace ICSharpCode.Decompiler.Metadata { var itemPath = Path.GetDirectoryName(item); var fullPath = Path.Combine(path, p.Name, p.Version, itemPath).ToLowerInvariant(); - if (Directory.Exists(fullPath)) + if (knownPackageBasePaths.Add(fullPath) && Directory.Exists(fullPath)) packageBasePaths.Add(fullPath); } } @@ -146,13 +156,15 @@ namespace ICSharpCode.Decompiler.Metadata { foreach (var basePath in searchPaths.Concat(packageBasePaths)) { - if (File.Exists(Path.Combine(basePath, name.Name + ".dll"))) + var file = Path.Combine(basePath, name.Name + ".dll"); + if (File.Exists(file)) { - return Path.Combine(basePath, name.Name + ".dll"); + return file; } - else if (File.Exists(Path.Combine(basePath, name.Name + ".exe"))) + file = Path.Combine(basePath, name.Name + ".exe"); + if (File.Exists(file)) { - return Path.Combine(basePath, name.Name + ".exe"); + return file; } } @@ -223,7 +235,9 @@ namespace ICSharpCode.Decompiler.Metadata if (preferredRuntimePack != null) { - runtimePacks = new[] { preferredRuntimePack }.Concat(runtimePacks); + // The preferred pack is usually one of the defaults as well; listing it twice means + // scanning its directory twice for every reference that is not in it. + runtimePacks = new[] { preferredRuntimePack }.Concat(RuntimePacks.Where(p => p != preferredRuntimePack)); } foreach (string pack in runtimePacks) @@ -232,14 +246,18 @@ namespace ICSharpCode.Decompiler.Metadata string basePath = Path.Combine(dotnetBasePath, "shared", pack); if (!Directory.Exists(basePath)) continue; - var closestVersion = GetClosestVersionFolder(basePath, targetFrameworkVersion); - if (File.Exists(Path.Combine(basePath, closestVersion, name.Name + ".dll"))) + var closestVersion = Owner != null + ? Owner.GetOrAddVersionFolder(basePath, p => GetClosestVersionFolder(p, targetFrameworkVersion)) + : GetClosestVersionFolder(basePath, targetFrameworkVersion); + var file = Path.Combine(basePath, closestVersion, name.Name + ".dll"); + if (File.Exists(file)) { - return Path.Combine(basePath, closestVersion, name.Name + ".dll"); + return file; } - else if (File.Exists(Path.Combine(basePath, closestVersion, name.Name + ".exe"))) + file = Path.Combine(basePath, closestVersion, name.Name + ".exe"); + if (File.Exists(file)) { - return Path.Combine(basePath, closestVersion, name.Name + ".exe"); + return file; } } runtimePack = null; diff --git a/ICSharpCode.Decompiler/Metadata/UniversalAssemblyResolver.cs b/ICSharpCode.Decompiler/Metadata/UniversalAssemblyResolver.cs index afee1ed2b..2d91be352 100644 --- a/ICSharpCode.Decompiler/Metadata/UniversalAssemblyResolver.cs +++ b/ICSharpCode.Decompiler/Metadata/UniversalAssemblyResolver.cs @@ -19,6 +19,7 @@ #nullable enable using System; +using System.Collections.Concurrent; using System.Collections.Generic; using System.Diagnostics.CodeAnalysis; using System.IO; @@ -76,6 +77,7 @@ namespace ICSharpCode.Decompiler.Metadata } readonly Lazy dotNetCorePathFinder; + ConcurrentDictionary? versionFolders; readonly bool throwOnError; readonly PEStreamOptions streamOptions; readonly MetadataReaderOptions metadataOptions; @@ -85,6 +87,39 @@ namespace ICSharpCode.Decompiler.Metadata static readonly List gac_paths = GetGacPaths(); static readonly DecompilerRuntime decompilerRuntime; + /// + public IDisposable? BeginSnapshot() + { + versionFolders = new ConcurrentDictionary(); + return new Snapshot(this); + } + + /// + /// The version folder to use inside , determined by + /// once per open scope, and on every call outside one. + /// + internal string GetOrAddVersionFolder(string basePath, Func valueFactory) + { + return versionFolders is { } cache ? cache.GetOrAdd(basePath, valueFactory) : valueFactory(basePath); + } + + /// + /// Holds what the resolver read from the file system until it is disposed. Two of these on + /// one resolver do not stack: the first to end takes the cache with it, and the other reads + /// the file system again. + /// + sealed class Snapshot : IDisposable + { + readonly UniversalAssemblyResolver resolver; + + public Snapshot(UniversalAssemblyResolver resolver) + { + this.resolver = resolver; + } + + public void Dispose() => resolver.versionFolders = null; + } + public void AddSearchDirectory(string? directory) { directories.Add(directory); @@ -358,6 +393,7 @@ namespace ICSharpCode.Decompiler.Metadata dotNetCorePathFinder = new DotNetCorePathFinder(targetFrameworkIdentifier, targetFrameworkVersion, runtimePack); else dotNetCorePathFinder = new DotNetCorePathFinder(mainAssemblyFileName, targetFramework, runtimePack, targetFrameworkIdentifier, targetFrameworkVersion); + dotNetCorePathFinder.Owner = this; foreach (var directory in directories) { dotNetCorePathFinder.AddSearchDirectory(directory); @@ -555,9 +591,12 @@ namespace ICSharpCode.Decompiler.Metadata return IsZeroOrAllOnes(reference.Version) || reference.IsRetargetable; } + static readonly string[] assemblyExtensions = { ".dll", ".exe" }; + static readonly string[] windowsMetadataExtensions = { ".winmd", ".dll" }; + string? SearchDirectory(IAssemblyReference name, string directory) { - var extensions = name.IsWindowsRuntime ? new[] { ".winmd", ".dll" } : new[] { ".dll", ".exe" }; + var extensions = name.IsWindowsRuntime ? windowsMetadataExtensions : assemblyExtensions; foreach (var extension in extensions) { string file = Path.Combine(directory, name.Name + extension); @@ -772,17 +811,17 @@ namespace ICSharpCode.Decompiler.Metadata return null; } + static readonly string[] gacFolders = { "GAC_MSIL", "GAC_32", "GAC_64", "GAC" }; + static readonly string[] gacFolderPrefixes = { string.Empty, "v4.0_" }; + static string? GetAssemblyInNetGac(IAssemblyReference reference) { - var gacs = new[] { "GAC_MSIL", "GAC_32", "GAC_64", "GAC" }; - var prefixes = new[] { string.Empty, "v4.0_" }; - for (int i = 0; i < gac_paths.Count; i++) { - for (int j = 0; j < gacs.Length; j++) + for (int j = 0; j < gacFolders.Length; j++) { - var gac = Path.Combine(gac_paths[i], gacs[j]); - var file = GetAssemblyFile(reference, prefixes[i], gac); + var gac = Path.Combine(gac_paths[i], gacFolders[j]); + var file = GetAssemblyFile(reference, gacFolderPrefixes[i], gac); if (File.Exists(file)) return file; } @@ -792,10 +831,10 @@ namespace ICSharpCode.Decompiler.Metadata // the whole GAC rather than a fallback within one folder. for (int i = 0; i < gac_paths.Count; i++) { - for (int j = 0; j < gacs.Length; j++) + for (int j = 0; j < gacFolders.Length; j++) { - var gac = Path.Combine(gac_paths[i], gacs[j]); - var file = FindUnifiedAssemblyInGacFolder(reference, prefixes[i], gac); + var gac = Path.Combine(gac_paths[i], gacFolders[j]); + var file = FindUnifiedAssemblyInGacFolder(reference, gacFolderPrefixes[i], gac); if (file != null) return file; } @@ -865,10 +904,9 @@ namespace ICSharpCode.Decompiler.Metadata /// public static IEnumerable EnumerateGac() { - var gacs = new[] { "GAC_MSIL", "GAC_32", "GAC_64", "GAC" }; foreach (var path in GetGacPaths()) { - foreach (var gac in gacs) + foreach (var gac in gacFolders) { string rootPath = Path.Combine(path, gac); if (!Directory.Exists(rootPath)) diff --git a/ICSharpCode.Decompiler/TypeSystem/DecompilerTypeSystem.cs b/ICSharpCode.Decompiler/TypeSystem/DecompilerTypeSystem.cs index 87812d19d..1d0f0d7e5 100644 --- a/ICSharpCode.Decompiler/TypeSystem/DecompilerTypeSystem.cs +++ b/ICSharpCode.Decompiler/TypeSystem/DecompilerTypeSystem.cs @@ -287,7 +287,13 @@ namespace ICSharpCode.Decompiler.TypeSystem int referencedAssembliesResolved = 0; try { - referencedAssembliesResolved = await InitializeCoreAsync(mainModule, assemblyResolver).ConfigureAwait(false); + // The whole reference closure is resolved here, and every reference in it asks the + // same framework directories the same questions. A resolver that can hold what it + // read answers them once for this build and forgets it again afterwards. + using (assemblyResolver.BeginSnapshot()) + { + referencedAssembliesResolved = await InitializeCoreAsync(mainModule, assemblyResolver).ConfigureAwait(false); + } } finally { diff --git a/ICSharpCode.ILSpyX/LoadedAssembly.cs b/ICSharpCode.ILSpyX/LoadedAssembly.cs index d9f30d9d7..5380157eb 100644 --- a/ICSharpCode.ILSpyX/LoadedAssembly.cs +++ b/ICSharpCode.ILSpyX/LoadedAssembly.cs @@ -615,6 +615,12 @@ namespace ICSharpCode.ILSpyX /// public ReferenceLoadInfo LoadInfo => referenceLoadInfo; + /// + public IDisposable? BeginSnapshot() + { + return parent.GetUniversalResolver(applyWinRTProjections).BeginSnapshot(); + } + public MetadataFile? Resolve(IAssemblyReference reference) { return ResolveAsync(reference).GetAwaiter().GetResult(); diff --git a/ICSharpCode.ILSpyX/LoadedPackage.cs b/ICSharpCode.ILSpyX/LoadedPackage.cs index 35cc5368f..4571e77b3 100644 --- a/ICSharpCode.ILSpyX/LoadedPackage.cs +++ b/ICSharpCode.ILSpyX/LoadedPackage.cs @@ -299,6 +299,9 @@ namespace ICSharpCode.ILSpyX public sealed class PackageFolder : IAssemblyResolver { + /// + public IDisposable? BeginSnapshot() => null; + /// /// Gets the short name of the folder. /// diff --git a/ILSpy.BamlDecompiler.Tests/MissingReferencesTests.cs b/ILSpy.BamlDecompiler.Tests/MissingReferencesTests.cs index 196f4dbb7..8732ff953 100644 --- a/ILSpy.BamlDecompiler.Tests/MissingReferencesTests.cs +++ b/ILSpy.BamlDecompiler.Tests/MissingReferencesTests.cs @@ -50,6 +50,9 @@ namespace ILSpy.BamlDecompiler.Tests /// sealed class WpfHidingResolver : IAssemblyResolver { + /// + public IDisposable BeginSnapshot() => null; + static readonly HashSet hidden = new(StringComparer.OrdinalIgnoreCase) { "WindowsBase", "PresentationCore", "PresentationFramework", "PresentationUI", "System.Xaml" }; diff --git a/TestTools/nugetfuzz.cs b/TestTools/nugetfuzz.cs index e49df6efb..68e92f4ad 100644 --- a/TestTools/nugetfuzz.cs +++ b/TestTools/nugetfuzz.cs @@ -855,6 +855,8 @@ class LoggingResolver(IAssemblyResolver inner, List dirs, Func Resolutions = new(); readonly Dictionary loaded = new(); + public IDisposable? BeginSnapshot() => inner.BeginSnapshot(); + public MetadataFile? Resolve(IAssemblyReference reference) { var file = ResolveFromDirs(reference) ?? inner.Resolve(reference);