diff --git a/src/Build.UnitTests/BackEnd/Scheduler_Tests.cs b/src/Build.UnitTests/BackEnd/Scheduler_Tests.cs
index e470f6be153..c8a163318f0 100644
--- a/src/Build.UnitTests/BackEnd/Scheduler_Tests.cs
+++ b/src/Build.UnitTests/BackEnd/Scheduler_Tests.cs
@@ -101,7 +101,7 @@ public void Dispose()
///
/// Verify that when a single request is submitted, we get a request assigned back out.
///
- [Fact(Skip = "https://github.com/dotnet/msbuild/issues/515")]
+ [Fact]
public void TestSimpleRequest()
{
CreateConfiguration(1, "foo.proj");
@@ -117,7 +117,7 @@ public void TestSimpleRequest()
///
/// Verify that when we submit a request and we already have results, we get the results back.
///
- [Fact(Skip = "https://github.com/dotnet/msbuild/issues/515")]
+ [Fact]
public void TestSimpleRequestWithCachedResultsSuccess()
{
CreateConfiguration(1, "foo.proj");
@@ -141,7 +141,7 @@ public void TestSimpleRequestWithCachedResultsSuccess()
///
/// Verify that when we submit a request with failing results, we get the results back.
///
- [Fact(Skip = "https://github.com/dotnet/msbuild/issues/515")]
+ [Fact]
public void TestSimpleRequestWithCachedResultsFail()
{
CreateConfiguration(1, "foo.proj");
@@ -165,7 +165,7 @@ public void TestSimpleRequestWithCachedResultsFail()
///
/// Verify that when we submit a child request with results cached, we get those results back.
///
- [Fact(Skip = "https://github.com/dotnet/msbuild/issues/515")]
+ [Fact]
public void TestChildRequest()
{
CreateConfiguration(1, "foo.proj");
@@ -195,7 +195,7 @@ public void TestChildRequest()
///
/// Verify that when multiple requests are submitted, the first one in is the first one out.
///
- [Fact(Skip = "https://github.com/dotnet/msbuild/issues/515")]
+ [Fact]
public void TestMultipleRequests()
{
CreateConfiguration(1, "foo.proj");
@@ -213,7 +213,7 @@ public void TestMultipleRequests()
///
/// Verify that when multiple requests are submitted with results cached, we get the results back.
///
- [Fact(Skip = "https://github.com/dotnet/msbuild/issues/515")]
+ [Fact]
public void TestMultipleRequestsWithSomeResults()
{
CreateConfiguration(1, "foo.proj");
@@ -235,7 +235,7 @@ public void TestMultipleRequestsWithSomeResults()
///
/// Verify that when multiple requests are submitted with results cached, we get the results back.
///
- [Fact(Skip = "https://github.com/dotnet/msbuild/issues/515")]
+ [Fact]
public void TestMultipleRequestsWithAllResults()
{
CreateConfiguration(1, "foo.proj");
@@ -266,7 +266,7 @@ public void TestMultipleRequestsWithAllResults()
/// Verify that if the affinity of one of the requests is out-of-proc, we create an out-of-proc node (but only one)
/// even if the max node count = 1.
///
- [Fact(Skip = "https://github.com/dotnet/msbuild/issues/515")]
+ [Fact]
public void TestOutOfProcNodeCreatedWhenAffinityIsOutOfProc()
{
CreateConfiguration(1, "foo.proj");
@@ -288,7 +288,7 @@ public void TestOutOfProcNodeCreatedWhenAffinityIsOutOfProc()
/// Verify that if the affinity of our requests is out-of-proc, that many out-of-proc nodes will
/// be made (assuming it does not exceed MaxNodeCount)
///
- [Fact(Skip = "https://github.com/dotnet/msbuild/issues/515")]
+ [Fact]
public void TestOutOfProcNodesCreatedWhenAffinityIsOutOfProc()
{
_host.BuildParameters.MaxNodeCount = 4;
@@ -313,7 +313,7 @@ public void TestOutOfProcNodesCreatedWhenAffinityIsOutOfProc()
/// we still won't create any new nodes if they're all for the same configuration --
/// they'd end up all being assigned to the same node.
///
- [Fact(Skip = "https://github.com/dotnet/msbuild/issues/515")]
+ [Fact]
public void TestNoNewNodesCreatedForMultipleRequestsWithSameConfiguration()
{
_host.BuildParameters.MaxNodeCount = 3;
@@ -336,7 +336,7 @@ public void TestNoNewNodesCreatedForMultipleRequestsWithSameConfiguration()
/// Verify that if the affinity of our requests is "any", we will not create more than
/// MaxNodeCount nodes (1 IP node + MaxNodeCount - 1 OOP nodes)
///
- [Fact(Skip = "https://github.com/dotnet/msbuild/issues/515")]
+ [Fact]
public void TestMaxNodeCountNotExceededWithRequestsOfAffinityAny()
{
_host.BuildParameters.MaxNodeCount = 3;
@@ -366,7 +366,7 @@ public void TestMaxNodeCountNotExceededWithRequestsOfAffinityAny()
/// node will service an Any request instead of an inproc request, leaving only one non-inproc request for the second round
/// of node creation.
///
- [Fact(Skip = "https://github.com/dotnet/msbuild/issues/515")]
+ [Fact]
public void VerifyRequestOrderingDoesNotAffectNodeCreationCountWithInProcAndAnyRequests()
{
// Since we're creating our own BuildManager, we need to make sure that the default
@@ -414,7 +414,7 @@ public void VerifyRequestOrderingDoesNotAffectNodeCreationCountWithInProcAndAnyR
/// Verify that if the affinity of our requests is out-of-proc, we will create as many as
/// MaxNodeCount out-of-proc nodes
///
- [Fact(Skip = "https://github.com/dotnet/msbuild/issues/515")]
+ [Fact]
public void TestMaxNodeCountOOPNodesCreatedForOOPAffinitizedRequests()
{
_host.BuildParameters.MaxNodeCount = 3;
@@ -444,7 +444,7 @@ public void TestMaxNodeCountOOPNodesCreatedForOOPAffinitizedRequests()
/// is less than MaxNodeCount, that we only create MaxNodeCount - 1 OOP nodes (for a total of MaxNodeCount
/// nodes, when the inproc node is included)
///
- [Fact(Skip = "https://github.com/dotnet/msbuild/issues/515")]
+ [Fact]
public void TestMaxNodeCountNodesNotExceededWithSomeOOPRequests1()
{
_host.BuildParameters.MaxNodeCount = 3;
@@ -474,7 +474,7 @@ public void TestMaxNodeCountNodesNotExceededWithSomeOOPRequests1()
/// is less than MaxNodeCount, that we only create MaxNodeCount - 1 OOP nodes (for a total of MaxNodeCount
/// nodes, when the inproc node is included)
///
- [Fact(Skip = "https://github.com/dotnet/msbuild/issues/515")]
+ [Fact]
public void TestMaxNodeCountNodesNotExceededWithSomeOOPRequests2()
{
_host.BuildParameters.MaxNodeCount = 3;
@@ -511,7 +511,7 @@ public void SchedulerShouldHonorDisableInprocNode()
/// Make sure that traversal projects are marked with an affinity of "InProc", which means that
/// even if multiple are available, we should still only have the single inproc node.
///
- [Fact(Skip = "https://github.com/dotnet/msbuild/issues/515")]
+ [Fact]
public void TestTraversalAffinityIsInProc()
{
_host.BuildParameters.MaxNodeCount = 3;
@@ -560,7 +560,7 @@ public void TestProxyAffinityIsInProc()
/// With something approximating the BuildManager's build loop, make sure that we don't end up
/// trying to create more nodes than we can actually support.
///
- [Fact(Skip = "https://github.com/dotnet/msbuild/issues/515")]
+ [Fact]
public void VerifyNoOverCreationOfNodesWithBuildLoop()
{
// Since we're creating our own BuildManager, we need to make sure that the default
@@ -615,7 +615,7 @@ public void BuildResultNotPlacedInCurrentCacheIfConfigExistsInOverrideCache()
///
/// Verify that if we get two requests but one of them is a failure, we only get the failure result back.
///
- [Fact(Skip = "https://github.com/dotnet/msbuild/issues/515")]
+ [Fact]
public void TestTwoRequestsWithFirstFailure()
{
CreateConfiguration(1, "foo.proj");
@@ -634,7 +634,7 @@ public void TestTwoRequestsWithFirstFailure()
///
/// Verify that if we get two requests but one of them is a failure, we only get the failure result back.
///
- [Fact(Skip = "https://github.com/dotnet/msbuild/issues/515")]
+ [Fact]
public void TestTwoRequestsWithSecondFailure()
{
CreateConfiguration(1, "foo.proj");
@@ -653,7 +653,7 @@ public void TestTwoRequestsWithSecondFailure()
///
/// Verify that if we get three requests but one of them is a failure, we only get the failure result back.
///
- [Fact(Skip = "https://github.com/dotnet/msbuild/issues/515")]
+ [Fact]
public void TestThreeRequestsWithOneFailure()
{
CreateConfiguration(1, "foo.proj");
@@ -673,7 +673,7 @@ public void TestThreeRequestsWithOneFailure()
///
/// Verify that providing a result to the only outstanding request results in build complete.
///
- [Fact(Skip = "https://github.com/dotnet/msbuild/issues/515")]
+ [Fact]
public void TestResult()
{
CreateConfiguration(1, "foo.proj");
@@ -697,7 +697,7 @@ public void TestResult()
///
/// Tests that the detailed summary setting causes the summary to be produced.
///
- [Fact(Skip = "https://github.com/dotnet/msbuild/issues/515")]
+ [Fact]
public void TestDetailedSummary()
{
string contents = ObjectModelHelpers.CleanupFileContents(@"
diff --git a/src/Build/BackEnd/Components/Scheduler/Scheduler.cs b/src/Build/BackEnd/Components/Scheduler/Scheduler.cs
index 6bcb954c1f3..a3141a4dc87 100644
--- a/src/Build/BackEnd/Components/Scheduler/Scheduler.cs
+++ b/src/Build/BackEnd/Components/Scheduler/Scheduler.cs
@@ -2239,49 +2239,55 @@ private bool RequestOrAnyItIsBlockedByCanBeServiced(SchedulableRequest request)
}
///
- /// Determines if we have a matching request somewhere, and if so, assigns the same request ID. Otherwise
- /// assigns a new request id.
+ /// Determines if we have a matching request somewhere, and if so, assigns the same request ID.
+ /// Otherwise assigns a new request id.
///
- ///
- /// UNDONE: (Performance) This algorithm should be modified so we don't have to iterate over all of the
- /// requests to find a matching one. A HashSet with proper equality semantics and a good hash code for the BuildRequest
- /// would speed this considerably, especially for large numbers of projects in a build.
- ///
/// The request whose ID should be assigned
private void AssignGlobalRequestId(BuildRequest request)
{
- bool assignNewId = false;
- if (request.GlobalRequestId == BuildRequest.InvalidGlobalRequestId && _schedulingData.GetRequestsAssignedToConfigurationCount(request.ConfigurationId) > 0)
+ // Quick exit if already assigned or if there are no requests for this configuration
+ if (request.GlobalRequestId != BuildRequest.InvalidGlobalRequestId
+ || _schedulingData.GetRequestsAssignedToConfigurationCount(request.ConfigurationId) == 0)
+ {
+ request.GlobalRequestId = _nextGlobalRequestId++;
+ return;
+ }
+
+ HashSet requestTargetsSet = new(request.Targets, StringComparer.OrdinalIgnoreCase);
+
+ // Look for matching requests in the configuration
+ foreach (SchedulableRequest existingRequest in _schedulingData.GetRequestsAssignedToConfiguration(request.ConfigurationId))
{
- foreach (SchedulableRequest existingRequest in _schedulingData.GetRequestsAssignedToConfiguration(request.ConfigurationId))
+ if (TargetsMatch(requestTargetsSet, existingRequest.BuildRequest.Targets))
{
- if (existingRequest.BuildRequest.Targets.Count == request.Targets.Count)
- {
- List leftTargets = new List(existingRequest.BuildRequest.Targets);
- List rightTargets = new List(request.Targets);
+ request.GlobalRequestId = existingRequest.BuildRequest.GlobalRequestId;
+ return;
+ }
+ }
- leftTargets.Sort(StringComparer.OrdinalIgnoreCase);
- rightTargets.Sort(StringComparer.OrdinalIgnoreCase);
- for (int i = 0; i < leftTargets.Count; i++)
- {
- if (!leftTargets[i].Equals(rightTargets[i], StringComparison.OrdinalIgnoreCase))
- {
- assignNewId = true;
- break;
- }
- }
+ // No matching request found, assign a new ID
+ request.GlobalRequestId = _nextGlobalRequestId++;
+ }
- if (!assignNewId)
- {
- request.GlobalRequestId = existingRequest.BuildRequest.GlobalRequestId;
- return;
- }
- }
+ ///
+ /// Determines if two target collections contain the same targets, ignoring order and case.
+ ///
+ private bool TargetsMatch(HashSet firstTargetsSet, List secondTargetsList)
+ {
+ if (firstTargetsSet.Count != secondTargetsList.Count)
+ {
+ return false;
+ }
+
+ foreach (string target in secondTargetsList)
+ {
+ if (!firstTargetsSet.Contains(target))
+ {
+ return false;
}
}
- request.GlobalRequestId = _nextGlobalRequestId;
- _nextGlobalRequestId++;
+ return true;
}
///