Skip to content

Commit

Permalink
Shared EvaluationContext for graph construction
Browse files Browse the repository at this point in the history
Fix dotnet#9678 by creating a single shared `EvaluationContext` in the `Graph`
object and using it when creating `ProjectInstance`s using the default
factory.

This is similar to how NuGet static-graph restore already works: https://github.com/NuGet/NuGet.Client/blob/b83566ec2369c4e9fd07e6f95d734dfe370a1e66/src/NuGet.Core/NuGet.Build.Tasks.Console/MSBuildStaticGraphRestore.cs#L885

Since we're evaluating the projects in the graph in parallel and "all at
once", the shared caches in the `EvaluationContext` should be a solid
improvement.
  • Loading branch information
rainersigwald committed Jan 24, 2024
1 parent 6d97976 commit f74240f
Show file tree
Hide file tree
Showing 3 changed files with 29 additions and 4 deletions.
6 changes: 4 additions & 2 deletions src/Build.UnitTests/Graph/ProjectGraph_Tests.cs
Expand Up @@ -9,6 +9,7 @@
using System.Text.RegularExpressions;
using Microsoft.Build.Construction;
using Microsoft.Build.Evaluation;
using Microsoft.Build.Evaluation.Context;
using Microsoft.Build.Exceptions;
using Microsoft.Build.Execution;
using Microsoft.Build.Framework;
Expand Down Expand Up @@ -107,10 +108,11 @@ public void ConstructWithSingleNodeWithProjectInstanceFactory()
(projectPath, globalProperties, projectCollection) =>
{
factoryCalled = true;
return ProjectGraph.DefaultProjectInstanceFactory(
return ProjectGraph.StaticProjectInstanceFactory(
projectPath,
globalProperties,
projectCollection);
projectCollection,
EvaluationContext.Create(EvaluationContext.SharingPolicy.Isolated));
});
projectGraph.ProjectNodes.Count.ShouldBe(1);
projectGraph.ProjectNodes.First().ProjectInstance.FullPath.ShouldBe(entryProject.Path);
Expand Down
22 changes: 20 additions & 2 deletions src/Build/Graph/ProjectGraph.cs
Expand Up @@ -11,6 +11,7 @@
using System.Text;
using System.Threading;
using Microsoft.Build.Evaluation;
using Microsoft.Build.Evaluation.Context;
using Microsoft.Build.Eventing;
using Microsoft.Build.Exceptions;
using Microsoft.Build.Execution;
Expand Down Expand Up @@ -56,6 +57,8 @@ public sealed class ProjectGraph

private readonly Lazy<IReadOnlyCollection<ProjectGraphNode>> _projectNodesTopologicallySorted;

private readonly EvaluationContext _evaluationContext = EvaluationContext.Create(EvaluationContext.SharingPolicy.Shared);

private GraphBuilder.GraphEdges Edges { get; }

internal GraphBuilder.GraphEdges TestOnly_Edges => Edges;
Expand Down Expand Up @@ -742,16 +745,31 @@ private static ImmutableList<string> ExpandDefaultTargets(ImmutableList<string>
return targets;
}

internal static ProjectInstance DefaultProjectInstanceFactory(
internal ProjectInstance DefaultProjectInstanceFactory(
string projectPath,
Dictionary<string, string> globalProperties,
ProjectCollection projectCollection)
{
return StaticProjectInstanceFactory(
projectPath,
globalProperties,
projectCollection,
_evaluationContext);
}

internal static ProjectInstance StaticProjectInstanceFactory(
string projectPath,
Dictionary<string, string> globalProperties,
ProjectCollection projectCollection,
EvaluationContext evaluationContext)
{
return new ProjectInstance(
projectPath,
globalProperties,
MSBuildConstants.CurrentToolsVersion,
projectCollection);
subToolsetVersion: null,
projectCollection,
evaluationContext);
}

private struct ProjectGraphBuildRequest : IEquatable<ProjectGraphBuildRequest>
Expand Down
5 changes: 5 additions & 0 deletions src/Build/Instance/ProjectInstance.cs
Expand Up @@ -248,6 +248,11 @@ public ProjectInstance(string projectFile, IDictionary<string, string> globalPro
{
}

public ProjectInstance(string projectFile, IDictionary<string, string> globalProperties, string toolsVersion, string subToolsetVersion, ProjectCollection projectCollection, EvaluationContext context, bool interactive = false)
: this(projectFile, globalProperties, toolsVersion, subToolsetVersion, projectCollection, projectLoadSettings: null, evaluationContext: context, directoryCacheFactory: null, interactive: false)
{
}

/// <summary>
/// Creates a ProjectInstance directly.
/// No intermediate Project object is created.
Expand Down

0 comments on commit f74240f

Please sign in to comment.