From b67c97ba8cde49ccce0c47b6dd5296fa675a5719 Mon Sep 17 00:00:00 2001 From: Jeff Kluge Date: Tue, 9 Jan 2018 08:39:18 -0800 Subject: [PATCH 1/4] Implementation --- .../BackEnd/BuildManager/BuildManager.cs | 3 + .../MainNodeSdkResolverService.cs | 15 ++--- .../SdkResolution/SdkResolverService.cs | 55 +++++++++++++++++-- src/Framework/Sdk/SdkResolverContext.cs | 6 ++ 4 files changed, 68 insertions(+), 11 deletions(-) diff --git a/src/Build/BackEnd/BuildManager/BuildManager.cs b/src/Build/BackEnd/BuildManager/BuildManager.cs index f2c86d59c06..01575947a29 100644 --- a/src/Build/BackEnd/BuildManager/BuildManager.cs +++ b/src/Build/BackEnd/BuildManager/BuildManager.cs @@ -1664,6 +1664,9 @@ private void CheckSubmissionCompletenessAndRemove(BuildSubmission submission) // Clear all cached SDKs for the submission SdkResolverService.ClearCache(submission.SubmissionId); + + // Clear SDK resolver cache for the submission + BackEnd.SdkResolution.SdkResolverService.Instance.ClearCache(submission.SubmissionId); } if (_buildSubmissions.Count == 0) diff --git a/src/Build/BackEnd/Components/SdkResolution/MainNodeSdkResolverService.cs b/src/Build/BackEnd/Components/SdkResolution/MainNodeSdkResolverService.cs index 7de2cee8b47..0f1704bc7a0 100644 --- a/src/Build/BackEnd/Components/SdkResolution/MainNodeSdkResolverService.cs +++ b/src/Build/BackEnd/Components/SdkResolution/MainNodeSdkResolverService.cs @@ -83,7 +83,7 @@ public override void PacketReceived(int node, INodePacket packet) /// public override string ResolveSdk(int submissionId, SdkReference sdk, LoggingContext loggingContext, ElementLocation sdkReferenceLocation, string solutionPath, string projectPath) { - SdkResult result = GetSdkResult(submissionId, sdk, loggingContext, sdkReferenceLocation, solutionPath, projectPath); + SdkResult result = GetSdkResultAndCache(submissionId, sdk, loggingContext, sdkReferenceLocation, solutionPath, projectPath); return result?.Path; } @@ -99,7 +99,7 @@ public override string ResolveSdk(int submissionId, SdkReference sdk, LoggingCon /// The full path to the solution, if any, that is being built. /// The full path to the project that referenced the SDK. /// An containing information about the SDK if one was resolved, otherwise null. - private SdkResult GetSdkResult(int submissionId, SdkReference sdk, LoggingContext loggingContext, ElementLocation sdkReferenceLocation, string solutionPath, string projectPath) + private SdkResult GetSdkResultAndCache(int submissionId, SdkReference sdk, LoggingContext loggingContext, ElementLocation sdkReferenceLocation, string solutionPath, string projectPath) { ErrorUtilities.VerifyThrowInternalNull(sdk, nameof(sdk)); ErrorUtilities.VerifyThrowInternalNull(loggingContext, nameof(loggingContext)); @@ -109,7 +109,7 @@ private SdkResult GetSdkResult(int submissionId, SdkReference sdk, LoggingContex if (Traits.Instance.EscapeHatches.DisableSdkResolutionCache) { - result = GetSdkResult(sdk, loggingContext, sdkReferenceLocation, solutionPath, projectPath); + result = GetSdkResult(submissionId, sdk, loggingContext, sdkReferenceLocation, solutionPath, projectPath); } else { @@ -123,7 +123,7 @@ private SdkResult GetSdkResult(int submissionId, SdkReference sdk, LoggingContex */ result = cached.GetOrAdd( sdk.Name, - key => GetSdkResult(sdk, loggingContext, sdkReferenceLocation, solutionPath, projectPath)); + key => GetSdkResult(submissionId, sdk, loggingContext, sdkReferenceLocation, solutionPath, projectPath)); } if (!SdkResolverService.IsReferenceSameVersion(sdk, result.Version)) @@ -138,15 +138,16 @@ private SdkResult GetSdkResult(int submissionId, SdkReference sdk, LoggingContex /// /// Resolves the specified SDK without caching the result. /// + /// The current build submission ID that is resolving an SDK. /// The containing information about the SDK to resolve. /// The to use when logging messages during resolution. /// The of the element that referenced the SDK. /// The full path to the solution, if any, that is being built. /// The full path to the project that referenced the SDK. /// An containing information about the SDK if one was resolved, otherwise null. - private SdkResult GetSdkResult(SdkReference sdk, LoggingContext loggingContext, ElementLocation sdkReferenceLocation, string solutionPath, string projectPath) + private SdkResult GetSdkResult(int submissionId, SdkReference sdk, LoggingContext loggingContext, ElementLocation sdkReferenceLocation, string solutionPath, string projectPath) { - SdkResult sdkResult = SdkResolverService.Instance.GetSdkResult(sdk, loggingContext, sdkReferenceLocation, solutionPath, projectPath); + SdkResult sdkResult = SdkResolverService.Instance.GetSdkResult(submissionId, sdk, loggingContext, sdkReferenceLocation, solutionPath, projectPath); if (!SdkResolverService.IsReferenceSameVersion(sdk, sdkResult.Version)) { @@ -220,7 +221,7 @@ private void ProcessRequests() ILoggingService loggingService = Host.GetComponent(BuildComponentType.LoggingService) as ILoggingService; // This call is usually cached so is very fast but can take longer for a new SDK that is downloaded. Other queued threads for different SDKs will complete sooner and continue on which unblocks evaluations - SdkResult result = GetSdkResult(request.SubmissionId, sdkReference, new EvaluationLoggingContext(loggingService, request.BuildEventContext, request.ProjectPath), request.ElementLocation, request.SolutionPath, request.ProjectPath); + SdkResult result = GetSdkResultAndCache(request.SubmissionId, sdkReference, new EvaluationLoggingContext(loggingService, request.BuildEventContext, request.ProjectPath), request.ElementLocation, request.SolutionPath, request.ProjectPath); // Create a response SdkResolverResponse response = new SdkResolverResponse(result.Path, result.Version); diff --git a/src/Build/BackEnd/Components/SdkResolution/SdkResolverService.cs b/src/Build/BackEnd/Components/SdkResolution/SdkResolverService.cs index 9a080e916f7..9f6313ebe61 100644 --- a/src/Build/BackEnd/Components/SdkResolution/SdkResolverService.cs +++ b/src/Build/BackEnd/Components/SdkResolution/SdkResolverService.cs @@ -7,8 +7,8 @@ using Microsoft.Build.Framework; using Microsoft.Build.Shared; using System; +using System.Collections.Concurrent; using System.Collections.Generic; -using Microsoft.Build.Collections; namespace Microsoft.Build.BackEnd.SdkResolution { @@ -30,6 +30,11 @@ internal sealed class SdkResolverService : ISdkResolverService /// private readonly object _lockObject = new object(); + /// + /// Stores resolver state by build submission ID. + /// + private readonly ConcurrentDictionary> _resolverStateBySubmission = new ConcurrentDictionary>(); + /// /// Stores the list of SDK resolvers which were loaded. /// @@ -73,18 +78,22 @@ public static bool IsReferenceSameVersion(SdkReference sdk, string version) /// public void ClearCache(int submissionId) { + ConcurrentDictionary notused; + + _resolverStateBySubmission.TryRemove(submissionId, out notused); } /// /// Resolves and SDK and gets a result. /// + /// The build submission ID that the resolution request is for. /// The containing information about the referenced SDK. /// The to use when logging messages during resolution. /// The of the element which referenced the SDK. /// The full path to the solution, if any, that is being built. /// The full path to that referenced the SDK. /// An containing information of the SDK if it could be resolved, otherwise null. - public SdkResult GetSdkResult(SdkReference sdk, LoggingContext loggingContext, ElementLocation sdkReferenceLocation, string solutionPath, string projectPath) + public SdkResult GetSdkResult(int submissionId, SdkReference sdk, LoggingContext loggingContext, ElementLocation sdkReferenceLocation, string solutionPath, string projectPath) { // Lazy initialize the SDK resolvers if (_resolvers == null) @@ -101,12 +110,18 @@ public SdkResult GetSdkResult(SdkReference sdk, LoggingContext loggingContext, E foreach (SdkResolver sdkResolver in _resolvers) { - SdkResolverContext context = new SdkResolverContext(buildEngineLogger, projectPath, solutionPath, ProjectCollection.Version); + SdkResolverContext context = new SdkResolverContext(buildEngineLogger, projectPath, solutionPath, ProjectCollection.Version) + { + State = GetResolverState(submissionId, sdkResolver) + }; SdkResultFactory resultFactory = new SdkResultFactory(sdk); try { SdkResult result = (SdkResult) sdkResolver.Resolve(sdk, context, resultFactory); + + SetResolverState(submissionId, sdkResolver, context.State); + if (result == null) { continue; @@ -151,7 +166,7 @@ public SdkResult GetSdkResult(SdkReference sdk, LoggingContext loggingContext, E /// public string ResolveSdk(int submissionId, SdkReference sdk, LoggingContext loggingContext, ElementLocation sdkReferenceLocation, string solutionPath, string projectPath) { - SdkResult result = GetSdkResult(sdk, loggingContext, sdkReferenceLocation, solutionPath, projectPath); + SdkResult result = GetSdkResult(submissionId, sdk, loggingContext, sdkReferenceLocation, solutionPath, projectPath); return result?.Path; } @@ -184,6 +199,27 @@ private static void LogWarnings(LoggingContext loggingContext, ElementLocation l } } + private object GetResolverState(int submissionId, SdkResolver resolver) + { + // Do not fetch state for resolution requests that are not associated with a valid build submission ID + if (submissionId != BuildEventContext.InvalidSubmissionId) + { + ConcurrentDictionary resolverState; + + if (_resolverStateBySubmission.TryGetValue(submissionId, out resolverState)) + { + object state; + + if (resolverState.TryGetValue(resolver, out state)) + { + return state; + } + } + } + + return null; + } + private void Initialize(LoggingContext loggingContext, ElementLocation location) { lock (_lockObject) @@ -196,5 +232,16 @@ private void Initialize(LoggingContext loggingContext, ElementLocation location) _resolvers = _sdkResolverLoader.LoadResolvers(loggingContext, location); } } + + private void SetResolverState(int submissionId, SdkResolver resolver, object state) + { + // Do not set state for resolution requests that are not associated with a valid build submission ID + if (submissionId != BuildEventContext.InvalidSubmissionId) + { + ConcurrentDictionary resolverState = _resolverStateBySubmission.GetOrAdd(submissionId, new ConcurrentDictionary(Environment.ProcessorCount, _resolvers.Count)); + + resolverState.AddOrUpdate(resolver, state, (sdkResolver, obj) => state); + } + } } } diff --git a/src/Framework/Sdk/SdkResolverContext.cs b/src/Framework/Sdk/SdkResolverContext.cs index f1e9ee6dcd5..87987056c5b 100644 --- a/src/Framework/Sdk/SdkResolverContext.cs +++ b/src/Framework/Sdk/SdkResolverContext.cs @@ -33,5 +33,11 @@ public abstract class SdkResolverContext /// /// public virtual Version MSBuildVersion { get; protected set; } + + /// + /// Gets or sets any custom state for current build. This allows resolvers to maintain state between resolutions. + /// This property is not thread-safe. + /// + public virtual object State { get; set; } } } From aa578fb290215df2d8eb3e10df17afd5fac1f84f Mon Sep 17 00:00:00 2001 From: Jeff Kluge Date: Tue, 9 Jan 2018 08:39:30 -0800 Subject: [PATCH 2/4] Unit tests --- .../BackEnd/SdkResolverService_Tests.cs | 60 ++++++++++++++++++- 1 file changed, 59 insertions(+), 1 deletion(-) diff --git a/src/Build.UnitTests/BackEnd/SdkResolverService_Tests.cs b/src/Build.UnitTests/BackEnd/SdkResolverService_Tests.cs index 9297271bc7f..fc878100c7c 100644 --- a/src/Build.UnitTests/BackEnd/SdkResolverService_Tests.cs +++ b/src/Build.UnitTests/BackEnd/SdkResolverService_Tests.cs @@ -98,6 +98,38 @@ public void AssertFirstResolverErrorsSupressedWhenResolved() Assert.DoesNotContain("ERROR", logResult); } + [Fact] + public void AssertResolverHasStatePreserved() + { + const int submissionId = 5; + + SdkResolverService.Instance.InitializeForTests(new MockLoaderStrategy()); + + SdkReference sdk = new SdkReference("othersdk", "1.0", "minimumVersion"); + + // First call should not know state + SdkResolverService.Instance.ResolveSdk(submissionId, sdk, _loggingContext, new MockElementLocation("file"), "sln", "projectPath").ShouldBe("resolverpath"); + + // Second call should have received state + SdkResolverService.Instance.ResolveSdk(submissionId, sdk, _loggingContext, new MockElementLocation("file"), "sln", "projectPath").ShouldBe(MockSdkResolverWithState.Expected); + } + + [Fact] + public void AssertResolverStateNotPreserved() + { + const int submissionId = BuildEventContext.InvalidSubmissionId; + + SdkResolverService.Instance.InitializeForTests(new MockLoaderStrategy()); + + SdkReference sdk = new SdkReference("othersdk", "1.0", "minimumVersion"); + + // First call should not know state + SdkResolverService.Instance.ResolveSdk(submissionId, sdk, _loggingContext, new MockElementLocation("file"), "sln", "projectPath").ShouldBe("resolverpath"); + + // Second call should have received state + SdkResolverService.Instance.ResolveSdk(submissionId, sdk, _loggingContext, new MockElementLocation("file"), "sln", "projectPath").ShouldBe("resolverpath"); + } + [Theory] [InlineData(null, "1.0", true)] [InlineData("1.0", "1.0", true)] @@ -130,7 +162,8 @@ internal override IList LoadResolvers(LoggingContext loggingContext { new MockSdkResolver1(), new MockSdkResolver2(), - new MockResolverReturnsNull() + new MockResolverReturnsNull(), + new MockSdkResolverWithState() }; if (_includeErrorResolver) @@ -185,6 +218,31 @@ public override SdkResultBase Resolve(SdkReference sdk, SdkResolverContextBase r } } + private class MockSdkResolverWithState : SdkResolver + { + public const string Expected = "01713226A202458F97D9074168DF2618"; + + public override string Name => nameof(MockSdkResolverWithState); + + public override int Priority => 3; + + public override SdkResultBase Resolve(SdkReference sdkReference, SdkResolverContextBase resolverContext, SdkResultFactoryBase factory) + { + if (sdkReference.Name.Equals("notfound")) + { + return null; + } + if (resolverContext.State != null) + { + return factory.IndicateSuccess((string) resolverContext.State, "1.0"); + } + + resolverContext.State = Expected; + + return factory.IndicateSuccess("resolverpath", "1.0"); + } + } + private class MockSdkResolverThrows : SdkResolver { public override string Name => nameof(MockSdkResolverThrows); From a2e381b6d15e9d4035d0849902ca02e2b531f89a Mon Sep 17 00:00:00 2001 From: Jeff Kluge Date: Tue, 9 Jan 2018 08:42:37 -0800 Subject: [PATCH 3/4] Update public API --- ref/net46/Microsoft.Build.Framework/Microsoft.Build.Framework.cs | 1 + .../Microsoft.Build.Framework/Microsoft.Build.Framework.cs | 1 + 2 files changed, 2 insertions(+) diff --git a/ref/net46/Microsoft.Build.Framework/Microsoft.Build.Framework.cs b/ref/net46/Microsoft.Build.Framework/Microsoft.Build.Framework.cs index c558d801a74..451210a6a6a 100644 --- a/ref/net46/Microsoft.Build.Framework/Microsoft.Build.Framework.cs +++ b/ref/net46/Microsoft.Build.Framework/Microsoft.Build.Framework.cs @@ -446,6 +446,7 @@ protected SdkResolverContext() { } public virtual System.Version MSBuildVersion { [System.Runtime.CompilerServices.CompilerGeneratedAttribute]get { throw null; } [System.Runtime.CompilerServices.CompilerGeneratedAttribute]protected set { } } public virtual string ProjectFilePath { [System.Runtime.CompilerServices.CompilerGeneratedAttribute]get { throw null; } [System.Runtime.CompilerServices.CompilerGeneratedAttribute]protected set { } } public virtual string SolutionFilePath { [System.Runtime.CompilerServices.CompilerGeneratedAttribute]get { throw null; } [System.Runtime.CompilerServices.CompilerGeneratedAttribute]protected set { } } + public virtual object State { [System.Runtime.CompilerServices.CompilerGeneratedAttribute]get { throw null; } [System.Runtime.CompilerServices.CompilerGeneratedAttribute]set { } } } public abstract partial class SdkResult { diff --git a/ref/netstandard1.3/Microsoft.Build.Framework/Microsoft.Build.Framework.cs b/ref/netstandard1.3/Microsoft.Build.Framework/Microsoft.Build.Framework.cs index 3715fbbad0f..e9ce5e9b222 100644 --- a/ref/netstandard1.3/Microsoft.Build.Framework/Microsoft.Build.Framework.cs +++ b/ref/netstandard1.3/Microsoft.Build.Framework/Microsoft.Build.Framework.cs @@ -443,6 +443,7 @@ protected SdkResolverContext() { } public virtual System.Version MSBuildVersion { [System.Runtime.CompilerServices.CompilerGeneratedAttribute]get { throw null; } [System.Runtime.CompilerServices.CompilerGeneratedAttribute]protected set { } } public virtual string ProjectFilePath { [System.Runtime.CompilerServices.CompilerGeneratedAttribute]get { throw null; } [System.Runtime.CompilerServices.CompilerGeneratedAttribute]protected set { } } public virtual string SolutionFilePath { [System.Runtime.CompilerServices.CompilerGeneratedAttribute]get { throw null; } [System.Runtime.CompilerServices.CompilerGeneratedAttribute]protected set { } } + public virtual object State { [System.Runtime.CompilerServices.CompilerGeneratedAttribute]get { throw null; } [System.Runtime.CompilerServices.CompilerGeneratedAttribute]set { } } } public abstract partial class SdkResult { From 7b1d14264093d947fb288a92e92c73983196d14f Mon Sep 17 00:00:00 2001 From: Jeff Kluge Date: Tue, 9 Jan 2018 09:58:45 -0800 Subject: [PATCH 4/4] Address comments --- src/Build/BackEnd/BuildManager/BuildManager.cs | 3 --- .../Components/SdkResolution/MainNodeSdkResolverService.cs | 4 ++++ 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/src/Build/BackEnd/BuildManager/BuildManager.cs b/src/Build/BackEnd/BuildManager/BuildManager.cs index 01575947a29..f2c86d59c06 100644 --- a/src/Build/BackEnd/BuildManager/BuildManager.cs +++ b/src/Build/BackEnd/BuildManager/BuildManager.cs @@ -1664,9 +1664,6 @@ private void CheckSubmissionCompletenessAndRemove(BuildSubmission submission) // Clear all cached SDKs for the submission SdkResolverService.ClearCache(submission.SubmissionId); - - // Clear SDK resolver cache for the submission - BackEnd.SdkResolution.SdkResolverService.Instance.ClearCache(submission.SubmissionId); } if (_buildSubmissions.Count == 0) diff --git a/src/Build/BackEnd/Components/SdkResolution/MainNodeSdkResolverService.cs b/src/Build/BackEnd/Components/SdkResolution/MainNodeSdkResolverService.cs index 0f1704bc7a0..a805f788f9c 100644 --- a/src/Build/BackEnd/Components/SdkResolution/MainNodeSdkResolverService.cs +++ b/src/Build/BackEnd/Components/SdkResolution/MainNodeSdkResolverService.cs @@ -66,7 +66,11 @@ public override void ClearCache(int submissionId) { ConcurrentDictionary entry; + // Clear our cache of resolved SDKs _cache.TryRemove(submissionId, out entry); + + // Also clear any cache for the SdkResolverService singleton which is the central place where resolution happens + SdkResolverService.Instance.ClearCache(submissionId); } ///