Skip to content

Commit

Permalink
Allow SDK resolvers to preserve state (#2849)
Browse files Browse the repository at this point in the history
SDK resolvers can get and set the `State` property on the `SdkResolverContext` they receive.  This state is preserved for the same resolver during the same build submission.  When there is no build submission, the state is not preserved.  Resolvers will need to take account for these considerations.

This will allow the NuGet-based SDK resolver to only look for and parse `global.json` once per build.

Address item 2 in #2803
  • Loading branch information
jeffkl authored Jan 9, 2018
1 parent 03d1435 commit 5c28a15
Show file tree
Hide file tree
Showing 6 changed files with 130 additions and 12 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
{
Expand Down
60 changes: 59 additions & 1 deletion src/Build.UnitTests/BackEnd/SdkResolverService_Tests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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)]
Expand Down Expand Up @@ -130,7 +162,8 @@ internal override IList<SdkResolver> LoadResolvers(LoggingContext loggingContext
{
new MockSdkResolver1(),
new MockSdkResolver2(),
new MockResolverReturnsNull()
new MockResolverReturnsNull(),
new MockSdkResolverWithState()
};

if (_includeErrorResolver)
Expand Down Expand Up @@ -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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,11 @@ public override void ClearCache(int submissionId)
{
ConcurrentDictionary<string, SdkResult> 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);
}

/// <inheritdoc cref="INodePacketHandler.PacketReceived"/>
Expand All @@ -83,7 +87,7 @@ public override void PacketReceived(int node, INodePacket packet)
/// <inheritdoc cref="ISdkResolverService.ResolveSdk"/>
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;
}
Expand All @@ -99,7 +103,7 @@ public override string ResolveSdk(int submissionId, SdkReference sdk, LoggingCon
/// <param name="solutionPath">The full path to the solution, if any, that is being built.</param>
/// <param name="projectPath">The full path to the project that referenced the SDK.</param>
/// <returns>An <see cref="SdkResult"/> containing information about the SDK if one was resolved, otherwise <code>null</code>.</returns>
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));
Expand All @@ -109,7 +113,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
{
Expand All @@ -123,7 +127,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))
Expand All @@ -138,15 +142,16 @@ private SdkResult GetSdkResult(int submissionId, SdkReference sdk, LoggingContex
/// <summary>
/// Resolves the specified SDK without caching the result.
/// </summary>
/// <param name="submissionId">The current build submission ID that is resolving an SDK.</param>
/// <param name="sdk">The <see cref="SdkReference"/> containing information about the SDK to resolve.</param>
/// <param name="loggingContext">The <see cref="LoggingContext"/> to use when logging messages during resolution.</param>
/// <param name="sdkReferenceLocation">The <see cref="ElementLocation"/> of the element that referenced the SDK.</param>
/// <param name="solutionPath">The full path to the solution, if any, that is being built.</param>
/// <param name="projectPath">The full path to the project that referenced the SDK.</param>
/// <returns>An <see cref="SdkResult"/> containing information about the SDK if one was resolved, otherwise <code>null</code>.</returns>
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))
{
Expand Down Expand Up @@ -220,7 +225,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);
Expand Down
55 changes: 51 additions & 4 deletions src/Build/BackEnd/Components/SdkResolution/SdkResolverService.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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
{
Expand All @@ -30,6 +30,11 @@ internal sealed class SdkResolverService : ISdkResolverService
/// </summary>
private readonly object _lockObject = new object();

/// <summary>
/// Stores resolver state by build submission ID.
/// </summary>
private readonly ConcurrentDictionary<int, ConcurrentDictionary<SdkResolver, object>> _resolverStateBySubmission = new ConcurrentDictionary<int, ConcurrentDictionary<SdkResolver, object>>();

/// <summary>
/// Stores the list of SDK resolvers which were loaded.
/// </summary>
Expand Down Expand Up @@ -73,18 +78,22 @@ public static bool IsReferenceSameVersion(SdkReference sdk, string version)
/// <inheritdoc cref="ISdkResolverService.ClearCache"/>
public void ClearCache(int submissionId)
{
ConcurrentDictionary<SdkResolver, object> notused;

_resolverStateBySubmission.TryRemove(submissionId, out notused);
}

/// <summary>
/// Resolves and SDK and gets a result.
/// </summary>
/// <param name="submissionId">The build submission ID that the resolution request is for.</param>
/// <param name="sdk">The <see cref="SdkReference"/> containing information about the referenced SDK.</param>
/// <param name="loggingContext">The <see cref="LoggingContext"/> to use when logging messages during resolution.</param>
/// <param name="sdkReferenceLocation">The <see cref="ElementLocation"/> of the element which referenced the SDK.</param>
/// <param name="solutionPath">The full path to the solution, if any, that is being built.</param>
/// <param name="projectPath">The full path to that referenced the SDK.</param>
/// <returns>An <see cref="SdkResult"/> containing information of the SDK if it could be resolved, otherwise <code>null</code>.</returns>
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)
Expand All @@ -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;
Expand Down Expand Up @@ -151,7 +166,7 @@ public SdkResult GetSdkResult(SdkReference sdk, LoggingContext loggingContext, E
/// <inheritdoc cref="ISdkResolverService.ResolveSdk"/>
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;
}
Expand Down Expand Up @@ -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<SdkResolver, object> 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)
Expand All @@ -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<SdkResolver, object> resolverState = _resolverStateBySubmission.GetOrAdd(submissionId, new ConcurrentDictionary<SdkResolver, object>(Environment.ProcessorCount, _resolvers.Count));

resolverState.AddOrUpdate(resolver, state, (sdkResolver, obj) => state);
}
}
}
}
6 changes: 6 additions & 0 deletions src/Framework/Sdk/SdkResolverContext.cs
Original file line number Diff line number Diff line change
Expand Up @@ -33,5 +33,11 @@ public abstract class SdkResolverContext
/// </remarks>
/// </summary>
public virtual Version MSBuildVersion { get; protected set; }

/// <summary>
/// Gets or sets any custom state for current build. This allows resolvers to maintain state between resolutions.
/// This property is not thread-safe.
/// </summary>
public virtual object State { get; set; }
}
}

0 comments on commit 5c28a15

Please sign in to comment.