From 697900da29f8905dc87047259148ce0dc8b7bdb0 Mon Sep 17 00:00:00 2001 From: Brad King Date: Wed, 15 Nov 2023 15:17:36 -0500 Subject: cmCTestMultiProcessHandler: Manage affinity assignments with other resources --- Source/CTest/cmCTestMultiProcessHandler.cxx | 44 +++++++++++++++-------------- 1 file changed, 23 insertions(+), 21 deletions(-) diff --git a/Source/CTest/cmCTestMultiProcessHandler.cxx b/Source/CTest/cmCTestMultiProcessHandler.cxx index a9930df..d9da2d8 100644 --- a/Source/CTest/cmCTestMultiProcessHandler.cxx +++ b/Source/CTest/cmCTestMultiProcessHandler.cxx @@ -164,18 +164,7 @@ void cmCTestMultiProcessHandler::RunTests() void cmCTestMultiProcessHandler::StartTestProcess(int test) { - if (this->HaveAffinity && this->Properties[test]->WantAffinity) { - size_t needProcessors = this->GetProcessorsUsed(test); - assert(needProcessors <= this->ProcessorsAvailable.size()); - std::vector affinity; - affinity.reserve(needProcessors); - for (size_t i = 0; i < needProcessors; ++i) { - auto p = this->ProcessorsAvailable.begin(); - affinity.push_back(*p); - this->ProcessorsAvailable.erase(p); - } - this->Properties[test]->Affinity = std::move(affinity); - } + this->LockResources(test); cmCTestOptionalLog(this->CTest, HANDLER_VERBOSE_OUTPUT, "test " << test << "\n", this->Quiet); @@ -202,10 +191,6 @@ void cmCTestMultiProcessHandler::StartTestProcess(int test) } } - // Always lock the resources we'll be using, even if we fail to set the - // working directory because FinishTestProcess() will try to unlock them - this->LockResources(test); - if (!this->ResourceAvailabilityErrors[test].empty()) { std::ostringstream e; e << "Insufficient resources for test " << this->Properties[test]->Name @@ -414,19 +399,41 @@ void cmCTestMultiProcessHandler::SetStopTimePassed() void cmCTestMultiProcessHandler::LockResources(int index) { auto* properties = this->Properties[index]; + this->ProjectResourcesLocked.insert(properties->ProjectResources.begin(), properties->ProjectResources.end()); + if (properties->RunSerial) { this->SerialTestRunning = true; } + + if (this->HaveAffinity && properties->WantAffinity) { + size_t needProcessors = this->GetProcessorsUsed(index); + assert(needProcessors <= this->ProcessorsAvailable.size()); + std::vector affinity; + affinity.reserve(needProcessors); + for (size_t i = 0; i < needProcessors; ++i) { + auto p = this->ProcessorsAvailable.begin(); + affinity.push_back(*p); + this->ProcessorsAvailable.erase(p); + } + properties->Affinity = std::move(affinity); + } } void cmCTestMultiProcessHandler::UnlockResources(int index) { auto* properties = this->Properties[index]; + + for (auto p : properties->Affinity) { + this->ProcessorsAvailable.insert(p); + } + properties->Affinity.clear(); + for (std::string const& i : properties->ProjectResources) { this->ProjectResourcesLocked.erase(i); } + if (properties->RunSerial) { this->SerialTestRunning = false; } @@ -686,11 +693,6 @@ void cmCTestMultiProcessHandler::FinishTestProcess( this->UnlockResources(test); this->RunningCount -= this->GetProcessorsUsed(test); - for (auto p : properties->Affinity) { - this->ProcessorsAvailable.insert(p); - } - properties->Affinity.clear(); - runner.reset(); this->StartNextTestsOnIdle(); -- cgit v0.12 From 0950acb3372d90f21d384600dcd9d9250f87421c Mon Sep 17 00:00:00 2001 From: Brad King Date: Mon, 20 Nov 2023 20:11:44 -0500 Subject: cmCTestMultiProcessHandler: Manage concurrency slots with other resources --- Source/CTest/cmCTestMultiProcessHandler.cxx | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/Source/CTest/cmCTestMultiProcessHandler.cxx b/Source/CTest/cmCTestMultiProcessHandler.cxx index d9da2d8..3f1e019 100644 --- a/Source/CTest/cmCTestMultiProcessHandler.cxx +++ b/Source/CTest/cmCTestMultiProcessHandler.cxx @@ -170,7 +170,6 @@ void cmCTestMultiProcessHandler::StartTestProcess(int test) "test " << test << "\n", this->Quiet); // now remove the test itself this->ErasePendingTest(test); - this->RunningCount += this->GetProcessorsUsed(test); auto testRun = cm::make_unique(*this, test); @@ -398,6 +397,8 @@ void cmCTestMultiProcessHandler::SetStopTimePassed() void cmCTestMultiProcessHandler::LockResources(int index) { + this->RunningCount += this->GetProcessorsUsed(index); + auto* properties = this->Properties[index]; this->ProjectResourcesLocked.insert(properties->ProjectResources.begin(), @@ -437,6 +438,8 @@ void cmCTestMultiProcessHandler::UnlockResources(int index) if (properties->RunSerial) { this->SerialTestRunning = false; } + + this->RunningCount -= this->GetProcessorsUsed(index); } void cmCTestMultiProcessHandler::ErasePendingTest(int test) @@ -691,7 +694,6 @@ void cmCTestMultiProcessHandler::FinishTestProcess( this->WriteCheckpoint(test); this->DeallocateResources(test); this->UnlockResources(test); - this->RunningCount -= this->GetProcessorsUsed(test); runner.reset(); -- cgit v0.12 From b17c732e88e5681fd1ec90d587d796c44329028b Mon Sep 17 00:00:00 2001 From: Brad King Date: Thu, 2 Nov 2023 15:04:49 -0400 Subject: cmCTestMultiProcessHandler: Clarify role of StartTestProcess Focus its role on actually running the test process. Move administrative tasks to the call site. --- Source/CTest/cmCTestMultiProcessHandler.cxx | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/Source/CTest/cmCTestMultiProcessHandler.cxx b/Source/CTest/cmCTestMultiProcessHandler.cxx index 3f1e019..d910367 100644 --- a/Source/CTest/cmCTestMultiProcessHandler.cxx +++ b/Source/CTest/cmCTestMultiProcessHandler.cxx @@ -164,12 +164,8 @@ void cmCTestMultiProcessHandler::RunTests() void cmCTestMultiProcessHandler::StartTestProcess(int test) { - this->LockResources(test); - cmCTestOptionalLog(this->CTest, HANDLER_VERBOSE_OUTPUT, "test " << test << "\n", this->Quiet); - // now remove the test itself - this->ErasePendingTest(test); auto testRun = cm::make_unique(*this, test); @@ -591,8 +587,12 @@ void cmCTestMultiProcessHandler::StartNextTests() continue; } + // Lock resources needed by this test. + this->LockResources(test); + // The test is ready to run. numToStart -= processors; + this->ErasePendingTest(test); this->StartTest(test); } -- cgit v0.12 From 0432b921aecfee3cf400a1f1ad8e65a1f7bc5a01 Mon Sep 17 00:00:00 2001 From: Brad King Date: Thu, 2 Nov 2023 13:56:01 -0400 Subject: cmCTestMultiProcessHandler: Inline removal of pending test as it starts Avoid searching the entire list of ordered pending tests to remove one when we already know where it is. --- Source/CTest/cmCTestMultiProcessHandler.cxx | 17 +++++++---------- Source/CTest/cmCTestMultiProcessHandler.h | 1 - 2 files changed, 7 insertions(+), 11 deletions(-) diff --git a/Source/CTest/cmCTestMultiProcessHandler.cxx b/Source/CTest/cmCTestMultiProcessHandler.cxx index d910367..be210f4 100644 --- a/Source/CTest/cmCTestMultiProcessHandler.cxx +++ b/Source/CTest/cmCTestMultiProcessHandler.cxx @@ -438,13 +438,6 @@ void cmCTestMultiProcessHandler::UnlockResources(int index) this->RunningCount -= this->GetProcessorsUsed(index); } -void cmCTestMultiProcessHandler::ErasePendingTest(int test) -{ - this->PendingTests.erase(test); - this->OrderedTests.erase( - std::find(this->OrderedTests.begin(), this->OrderedTests.end(), test)); -} - inline size_t cmCTestMultiProcessHandler::GetProcessorsUsed(int test) { size_t processors = static_cast(this->Properties[test]->Processors); @@ -537,7 +530,8 @@ void cmCTestMultiProcessHandler::StartNextTests() ti != this->OrderedTests.end()) { // Increment the test iterator now because the current list // entry may be deleted below. - int test = *ti++; + auto cti = ti++; + int test = *cti; // We can only start a RUN_SERIAL test if no other tests are also // running. @@ -592,7 +586,8 @@ void cmCTestMultiProcessHandler::StartNextTests() // The test is ready to run. numToStart -= processors; - this->ErasePendingTest(test); + this->OrderedTests.erase(cti); + this->PendingTests.erase(test); this->StartTest(test); } @@ -1411,7 +1406,9 @@ void cmCTestMultiProcessHandler::CheckResume() void cmCTestMultiProcessHandler::RemoveTest(int index) { - this->ErasePendingTest(index); + this->OrderedTests.erase( + std::find(this->OrderedTests.begin(), this->OrderedTests.end(), index)); + this->PendingTests.erase(index); this->Properties.erase(index); this->Completed++; } diff --git a/Source/CTest/cmCTestMultiProcessHandler.h b/Source/CTest/cmCTestMultiProcessHandler.h index ffbe16f..1be04aa 100644 --- a/Source/CTest/cmCTestMultiProcessHandler.h +++ b/Source/CTest/cmCTestMultiProcessHandler.h @@ -127,7 +127,6 @@ protected: // Removes the checkpoint file void MarkFinished(); - void ErasePendingTest(int index); void FinishTestProcess(std::unique_ptr runner, bool started); void StartNextTestsOnIdle(); -- cgit v0.12