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; } ///