Cache SDK resolver data process-wide

documentation/wiki/ChangeWaves.md

@@ -25,6 +25,7 @@ A wave of features is set to "rotate out" (i.e. become standard functionality) t
 
 ### 17.10
 - [AppDomain configuration is serialized without using BinFmt](https://github.com/dotnet/msbuild/pull/9320) - feature can be opted out only if [BinaryFormatter](https://learn.microsoft.com/en-us/dotnet/api/system.runtime.serialization.formatters.binary.binaryformatter) is allowed at runtime by editing `MSBuild.runtimeconfig.json`
+- [Cache SDK resolver data process-wide](https://github.com/dotnet/msbuild/pull/9335)
 
 ### 17.8
 - [[RAR] Don't do I/O on SDK-provided references](https://github.com/dotnet/msbuild/pull/8688)

src/Build.UnitTests/BackEnd/SdkResolverLoader_Tests.cs

@@ -30,26 +30,21 @@ public class SdkResolverLoader_Tests
     {
         private readonly ITestOutputHelper _output;
         private readonly MockLogger _logger;
-        private readonly LoggingContext _loggingContext;
 
         public SdkResolverLoader_Tests(ITestOutputHelper output)
         {
             _output = output;
             _logger = new MockLogger(output);
             ILoggingService loggingService = LoggingService.CreateLoggingService(LoggerMode.Synchronous, 1);
             loggingService.RegisterLogger(_logger);
-
-            _loggingContext = new MockLoggingContext(
-                loggingService,
-                new BuildEventContext(0, 0, BuildEventContext.InvalidProjectContextId, 0, 0));
         }
 
         [Fact]
         public void AssertDefaultLoaderReturnsDefaultResolvers()
         {
             var loader = new SdkResolverLoader();
 
-            var resolvers = loader.LoadAllResolvers(_loggingContext, new MockElementLocation("file"));
+            var resolvers = loader.LoadAllResolvers(new MockElementLocation("file"));
 
             resolvers.Select(i => i.GetType().FullName).ShouldBe(new[] { typeof(DefaultSdkResolver).FullName });
 
@@ -131,7 +126,7 @@ public void VerifyThrowsWhenResolverFailsToLoad()
         {
             SdkResolverLoader sdkResolverLoader = new MockSdkResolverLoader
             {
-                LoadResolverAssemblyFunc = (resolverPath, loggingContext, location) => typeof(SdkResolverLoader_Tests).GetTypeInfo().Assembly,
+                LoadResolverAssemblyFunc = (resolverPath) => typeof(SdkResolverLoader_Tests).GetTypeInfo().Assembly,
                 FindPotentialSdkResolversFunc = (rootFolder, loc) => new List<string>
                 {
                     "myresolver.dll"
@@ -141,7 +136,7 @@ public void VerifyThrowsWhenResolverFailsToLoad()
 
             InvalidProjectFileException exception = Should.Throw<InvalidProjectFileException>(() =>
             {
-                sdkResolverLoader.LoadAllResolvers(_loggingContext, ElementLocation.EmptyLocation);
+                sdkResolverLoader.LoadAllResolvers(ElementLocation.EmptyLocation);
             });
 
             exception.Message.ShouldBe($"The SDK resolver type \"{nameof(MockSdkResolverThatDoesNotLoad)}\" failed to load. A8BB8B3131D3475D881ACD3AF8D75BD6");
@@ -163,7 +158,7 @@ public void VerifyThrowsWhenResolverHasNoPublicConstructor()
         {
             SdkResolverLoader sdkResolverLoader = new MockSdkResolverLoader
             {
-                LoadResolverAssemblyFunc = (resolverPath, loggingContext, location) => typeof(SdkResolverLoader_Tests).GetTypeInfo().Assembly,
+                LoadResolverAssemblyFunc = (resolverPath) => typeof(SdkResolverLoader_Tests).GetTypeInfo().Assembly,
                 FindPotentialSdkResolversFunc = (rootFolder, loc) => new List<string>
                 {
                     "myresolver.dll"
@@ -173,7 +168,7 @@ public void VerifyThrowsWhenResolverHasNoPublicConstructor()
 
             InvalidProjectFileException exception = Should.Throw<InvalidProjectFileException>(() =>
             {
-                sdkResolverLoader.LoadAllResolvers(_loggingContext, ElementLocation.EmptyLocation);
+                sdkResolverLoader.LoadAllResolvers(ElementLocation.EmptyLocation);
             });
 
             exception.Message.ShouldStartWith($"The SDK resolver type \"{nameof(MockSdkResolverNoPublicConstructor)}\" failed to load.");
@@ -195,7 +190,7 @@ public void VerifyWarningLoggedWhenResolverAssemblyCannotBeLoaded()
 
             SdkResolverLoader sdkResolverLoader = new MockSdkResolverLoader
             {
-                LoadResolverAssemblyFunc = (resolverPath, loggingContext, location) => throw new Exception(expectedMessage),
+                LoadResolverAssemblyFunc = (resolverPath) => throw new Exception(expectedMessage),
                 FindPotentialSdkResolversFunc = (rootFolder, loc) => new List<string>
                 {
                     assemblyPath,
@@ -204,7 +199,7 @@ public void VerifyWarningLoggedWhenResolverAssemblyCannotBeLoaded()
 
             InvalidProjectFileException exception = Should.Throw<InvalidProjectFileException>(() =>
             {
-                sdkResolverLoader.LoadAllResolvers(_loggingContext, ElementLocation.EmptyLocation);
+                sdkResolverLoader.LoadAllResolvers(ElementLocation.EmptyLocation);
             });
 
             exception.Message.ShouldBe($"The SDK resolver assembly \"{assemblyPath}\" could not be loaded. {expectedMessage}");
@@ -345,12 +340,12 @@ public void SdkResolverLoaderHonorsIncludeDefaultEnvVar()
                     Environment.SetEnvironmentVariable("MSBUILDINCLUDEDEFAULTSDKRESOLVER", "false");
                     SdkResolverLoader loader = new MockSdkResolverLoader()
                     {
-                        LoadResolversAction = (resolverPath, loggingContext, location, resolvers) =>
+                        LoadResolversAction = (resolverPath, location, resolvers) =>
                         {
                             resolvers.Add(new MockSdkResolverWithAssemblyPath(resolverPath));
                         }
                     };
-                    IList<SdkResolverBase> resolvers = loader.LoadAllResolvers(_loggingContext, new MockElementLocation("file"));
+                    IReadOnlyList<SdkResolverBase> resolvers = loader.LoadAllResolvers(new MockElementLocation("file"));
 
                     resolvers.Count.ShouldBe(0);
                 }
@@ -390,7 +385,7 @@ public void SdkResolverLoaderHonorsAdditionalResolversFolder()
                     Environment.SetEnvironmentVariable("MSBUILDADDITIONALSDKRESOLVERSFOLDER", additionalRoot);
 
                     SdkResolverLoader loader = new SdkResolverLoader();
-                    IList<string> resolvers = loader.FindPotentialSdkResolvers(testRoot, new MockElementLocation("file"));
+                    IReadOnlyList<string> resolvers = loader.FindPotentialSdkResolvers(testRoot, new MockElementLocation("file"));
 
                     resolvers.ShouldBeSameIgnoringOrder(new[] { resolver1Path, resolver2Path, resolver3Path });
                 }
@@ -457,22 +452,22 @@ public override SdkResultBase Resolve(SdkReference sdkReference, SdkResolverCont
 
         private sealed class MockSdkResolverLoader : SdkResolverLoader
         {
-            public Func<string, LoggingContext, ElementLocation, Assembly> LoadResolverAssemblyFunc { get; set; }
+            public Func<string, Assembly> LoadResolverAssemblyFunc { get; set; }
 
-            public Func<string, ElementLocation, IList<string>> FindPotentialSdkResolversFunc { get; set; }
+            public Func<string, ElementLocation, IReadOnlyList<string>> FindPotentialSdkResolversFunc { get; set; }
 
             public Func<Assembly, IEnumerable<Type>> GetResolverTypesFunc { get; set; }
 
-            public Action<string, LoggingContext, ElementLocation, List<SdkResolver>> LoadResolversAction { get; set; }
+            public Action<string, ElementLocation, List<SdkResolver>> LoadResolversAction { get; set; }
 
-            protected override Assembly LoadResolverAssembly(string resolverPath, LoggingContext loggingContext, ElementLocation location)
+            protected override Assembly LoadResolverAssembly(string resolverPath)
             {
                 if (LoadResolverAssemblyFunc != null)
                 {
-                    return LoadResolverAssemblyFunc(resolverPath, loggingContext, location);
+                    return LoadResolverAssemblyFunc(resolverPath);
                 }
 
-                return base.LoadResolverAssembly(resolverPath, loggingContext, location);
+                return base.LoadResolverAssembly(resolverPath);
             }
 
             protected override IEnumerable<Type> GetResolverTypes(Assembly assembly)
@@ -485,7 +480,7 @@ protected override IEnumerable<Type> GetResolverTypes(Assembly assembly)
                 return base.GetResolverTypes(assembly);
             }
 
-            internal override IList<string> FindPotentialSdkResolvers(string rootFolder, ElementLocation location)
+            internal override IReadOnlyList<string> FindPotentialSdkResolvers(string rootFolder, ElementLocation location)
             {
                 if (FindPotentialSdkResolversFunc != null)
                 {
@@ -495,14 +490,14 @@ internal override IList<string> FindPotentialSdkResolvers(string rootFolder, Ele
                 return base.FindPotentialSdkResolvers(rootFolder, location);
             }
 
-            protected override void LoadResolvers(string resolverPath, LoggingContext loggingContext, ElementLocation location, List<SdkResolver> resolvers)
+            protected override void LoadResolvers(string resolverPath, ElementLocation location, List<SdkResolver> resolvers)
             {
                 if (LoadResolversAction != null)
                 {
-                    LoadResolversAction(resolverPath, loggingContext, location, resolvers);
+                    LoadResolversAction(resolverPath, location, resolvers);
                     return;
                 }
-                base.LoadResolvers(resolverPath, loggingContext, location, resolvers);
+                base.LoadResolvers(resolverPath, location, resolvers);
             }
         }
     }

src/Build.UnitTests/BackEnd/SdkResolverService_Tests.cs

@@ -638,13 +638,12 @@ private MockLoaderStrategy()
                 _resolversWithPatterns = new List<(string ResolvableSdkPattern, SdkResolver Resolver)>();
             }
 
-            internal override IList<SdkResolver> LoadAllResolvers(LoggingContext loggingContext, ElementLocation location)
+            internal override IReadOnlyList<SdkResolver> LoadAllResolvers(ElementLocation location)
             {
                 return _resolvers.OrderBy(i => i.Priority).ToList();
             }
 
-            internal override IList<SdkResolverManifest> GetResolversManifests(LoggingContext loggingContext,
-            ElementLocation location)
+            internal override IReadOnlyList<SdkResolverManifest> GetResolversManifests(ElementLocation location)
             {
                 var manifests = new List<SdkResolverManifest>();
                 foreach (SdkResolver resolver in _resolvers)
@@ -663,7 +662,7 @@ internal override IList<SdkResolverManifest> GetResolversManifests(LoggingContex
                 return manifests;
             }
 
-            protected internal override IList<SdkResolver> LoadResolversFromManifest(SdkResolverManifest manifest, LoggingContext loggingContext, ElementLocation location)
+            protected internal override IReadOnlyList<SdkResolver> LoadResolversFromManifest(SdkResolverManifest manifest, ElementLocation location)
             {
                 var resolvers = new List<SdkResolver>();
                 foreach (var resolver in _resolvers)
@@ -683,7 +682,7 @@ protected internal override IList<SdkResolver> LoadResolversFromManifest(SdkReso
                 return resolvers.OrderBy(t => t.Priority).ToList();
             }
 
-            internal override IList<SdkResolver> GetDefaultResolvers(LoggingContext loggingContext, ElementLocation location)
+            internal override IReadOnlyList<SdkResolver> GetDefaultResolvers()
             {
                 return new List<SdkResolver>();
             }

src/Build/BackEnd/Components/SdkResolution/CachingSdkResolverLoader.cs

@@ -0,0 +1,91 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+
+using System.Collections.Concurrent;
+using System.Collections.Generic;
+using Microsoft.Build.Construction;
+using Microsoft.Build.Framework;
+
+namespace Microsoft.Build.BackEnd.SdkResolution
+{
+    /// <summary>
+    /// A subclass of <see cref="SdkResolverLoader"/> which creates resolver manifests and SDK resolvers only once and
+    /// then returns cached results.
+    /// </summary>
+    internal sealed class CachingSdkResolverLoader : SdkResolverLoader
+    {
+        /// <summary>
+        /// Cached list of default resolvers. Set eagerly.
+        /// </summary>
+        private readonly IReadOnlyList<SdkResolver> _defaultResolvers;
+
+        /// <summary>
+        /// Cached manifest -> resolver dictionary. Populated lazily.
+        /// </summary>
+        private readonly ConcurrentDictionary<SdkResolverManifest, IReadOnlyList<SdkResolver>> _resolversByManifest = new();
+
+        /// <summary>
+        /// Cached list of all resolvers. Set lazily.
+        /// </summary>
+        private IReadOnlyList<SdkResolver>? _allResolvers;
+
+        /// <summary>
+        /// Cached list of all resolver manifests. Set lazily.
+        /// </summary>
+        private IReadOnlyList<SdkResolverManifest>? _resolversManifests;
+
+        /// <summary>
+        /// A lock object protecting <see cref="_allResolvers"/> and <see cref="_resolversManifests"/>.
+        /// </summary>
+        private readonly object _lock = new();
+
+        /// <summary>
+        /// A static instance of <see cref="CachingSdkResolverLoader"/>.
+        /// </summary>
+        /// <remarks>
+        /// The set of available SDK resolvers is expected to be fixed for the given MSBuild installation so it should be safe to use
+        /// a static instance as opposed to creating <see cref="CachingSdkResolverLoader"/> or <see cref="SdkResolverLoader"/> for each
+        /// <see cref="SdkResolverService" /> instance.
+        /// </remarks>
+        public static CachingSdkResolverLoader Instance = new CachingSdkResolverLoader();
+
+        /// <summary>
+        /// Initializes a new instance by setting <see cref="_defaultResolvers"/>.
+        /// </summary>
+        public CachingSdkResolverLoader()
+        {
+            _defaultResolvers = base.GetDefaultResolvers();
+        }
+
+        #region SdkResolverLoader overrides
+
+        /// <inheritdoc />
+        internal override IReadOnlyList<SdkResolver> GetDefaultResolvers() => _defaultResolvers;
+
+        /// <inheritdoc />
+        internal override IReadOnlyList<SdkResolver> LoadAllResolvers(ElementLocation location)
+        {
+            lock (_lock)
+            {
+                return _allResolvers ??= base.LoadAllResolvers(location);
+            }
+        }
+
+        /// <inheritdoc />
+        internal override IReadOnlyList<SdkResolverManifest> GetResolversManifests(ElementLocation location)
+        {
+            lock (_lock)
+            {
+                return _resolversManifests ??= base.GetResolversManifests(location);
+            }
+        }
+
+        /// <inheritdoc />
+        protected internal override IReadOnlyList<SdkResolver> LoadResolversFromManifest(SdkResolverManifest manifest, ElementLocation location)
+        {
+            return _resolversByManifest.GetOrAdd(manifest, (manifest) => base.LoadResolversFromManifest(manifest, location));
+        }
+
+        #endregion
+    }
+}

src/Build/BackEnd/Components/SdkResolution/MainNodeSdkResolverService.cs

@@ -39,7 +39,7 @@ public static IBuildComponent CreateComponent(BuildComponentType type)
         }
 
         // Test hook
-        internal void InitializeForTests(SdkResolverLoader resolverLoader = null, IList<SdkResolver> resolvers = null)
+        internal void InitializeForTests(SdkResolverLoader resolverLoader = null, IReadOnlyList<SdkResolver> resolvers = null)
         {
             ((CachingSdkResolverService)_cachedSdkResolver).InitializeForTests(resolverLoader, resolvers);
         }