From 3c4767f4670be61e42c258db281a06067662217c Mon Sep 17 00:00:00 2001 From: Brad King Date: Wed, 15 Nov 2023 15:07:02 -0500 Subject: cmCTestTestHandler: Clarify name of member storing RESOURCE_LOCK property The property represents project-defined resources. --- Source/CTest/cmCTestMultiProcessHandler.cxx | 18 +++++++++--------- Source/CTest/cmCTestMultiProcessHandler.h | 2 +- Source/CTest/cmCTestTestHandler.cxx | 2 +- Source/CTest/cmCTestTestHandler.h | 2 +- 4 files changed, 12 insertions(+), 12 deletions(-) diff --git a/Source/CTest/cmCTestMultiProcessHandler.cxx b/Source/CTest/cmCTestMultiProcessHandler.cxx index bfdec9f..a45b3db 100644 --- a/Source/CTest/cmCTestMultiProcessHandler.cxx +++ b/Source/CTest/cmCTestMultiProcessHandler.cxx @@ -401,8 +401,8 @@ void cmCTestMultiProcessHandler::SetStopTimePassed() void cmCTestMultiProcessHandler::LockResources(int index) { auto* properties = this->Properties[index]; - this->LockedResources.insert(properties->LockedResources.begin(), - properties->LockedResources.end()); + this->ProjectResourcesLocked.insert(properties->ProjectResources.begin(), + properties->ProjectResources.end()); if (properties->RunSerial) { this->SerialTestRunning = true; } @@ -411,8 +411,8 @@ void cmCTestMultiProcessHandler::LockResources(int index) void cmCTestMultiProcessHandler::UnlockResources(int index) { auto* properties = this->Properties[index]; - for (std::string const& i : properties->LockedResources) { - this->LockedResources.erase(i); + for (std::string const& i : properties->ProjectResources) { + this->ProjectResourcesLocked.erase(i); } if (properties->RunSerial) { this->SerialTestRunning = false; @@ -449,9 +449,9 @@ std::string cmCTestMultiProcessHandler::GetName(int test) bool cmCTestMultiProcessHandler::StartTest(int test) { - // Check for locked resources - for (std::string const& i : this->Properties[test]->LockedResources) { - if (cm::contains(this->LockedResources, i)) { + // Check for locked project resources. + for (std::string const& i : this->Properties[test]->ProjectResources) { + if (cm::contains(this->ProjectResourcesLocked, i)) { return false; } } @@ -1093,9 +1093,9 @@ static Json::Value DumpCTestProperties( properties.append(DumpCTestProperty( "REQUIRED_FILES", DumpToJsonArray(testProperties.RequiredFiles))); } - if (!testProperties.LockedResources.empty()) { + if (!testProperties.ProjectResources.empty()) { properties.append(DumpCTestProperty( - "RESOURCE_LOCK", DumpToJsonArray(testProperties.LockedResources))); + "RESOURCE_LOCK", DumpToJsonArray(testProperties.ProjectResources))); } if (testProperties.RunSerial) { properties.append( diff --git a/Source/CTest/cmCTestMultiProcessHandler.h b/Source/CTest/cmCTestMultiProcessHandler.h index c01089c..6832786 100644 --- a/Source/CTest/cmCTestMultiProcessHandler.h +++ b/Source/CTest/cmCTestMultiProcessHandler.h @@ -194,7 +194,7 @@ protected: std::vector* Passed; std::vector* Failed; std::vector LastTestsFailed; - std::set LockedResources; + std::set ProjectResourcesLocked; std::map>>> AllocatedResources; diff --git a/Source/CTest/cmCTestTestHandler.cxx b/Source/CTest/cmCTestTestHandler.cxx index ba15366..1918b2c 100644 --- a/Source/CTest/cmCTestTestHandler.cxx +++ b/Source/CTest/cmCTestTestHandler.cxx @@ -2233,7 +2233,7 @@ bool cmCTestTestHandler::SetTestsProperties( } else if (key == "RESOURCE_LOCK"_s) { cmList lval{ val }; - rt.LockedResources.insert(lval.begin(), lval.end()); + rt.ProjectResources.insert(lval.begin(), lval.end()); } else if (key == "FIXTURES_SETUP"_s) { cmList lval{ val }; diff --git a/Source/CTest/cmCTestTestHandler.h b/Source/CTest/cmCTestTestHandler.h index 23f0a76..b81fcd5 100644 --- a/Source/CTest/cmCTestTestHandler.h +++ b/Source/CTest/cmCTestTestHandler.h @@ -165,7 +165,7 @@ public: std::vector Environment; std::vector EnvironmentModification; std::vector Labels; - std::set LockedResources; + std::set ProjectResources; // RESOURCE_LOCK std::set FixturesSetup; std::set FixturesCleanup; std::set FixturesRequired; -- cgit v0.12 From ad3df3ce4dfe772c0a95eefd88628b4a800803d9 Mon Sep 17 00:00:00 2001 From: Brad King Date: Fri, 20 Oct 2023 15:34:48 -0400 Subject: cmCTestMultiProcessHandler: Exclude dependent tests earlier Tests with unfinished dependencies should not be considered at all when looking for tests that fit within the load and concurrency limits. --- Source/CTest/cmCTestMultiProcessHandler.cxx | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/Source/CTest/cmCTestMultiProcessHandler.cxx b/Source/CTest/cmCTestMultiProcessHandler.cxx index a45b3db..3759af1 100644 --- a/Source/CTest/cmCTestMultiProcessHandler.cxx +++ b/Source/CTest/cmCTestMultiProcessHandler.cxx @@ -460,14 +460,7 @@ bool cmCTestMultiProcessHandler::StartTest(int test) return false; } - // if there are no depends left then run this test - if (this->PendingTests[test].Depends.empty()) { - return this->StartTestProcess(test); - } - // This test was not able to start because it is waiting - // on depends to run - this->DeallocateResources(test); - return false; + return this->StartTestProcess(test); } void cmCTestMultiProcessHandler::StartNextTests() @@ -553,6 +546,11 @@ void cmCTestMultiProcessHandler::StartNextTests() continue; } + // Exclude tests that depend on unfinished tests. + if (!this->PendingTests[test].Depends.empty()) { + continue; + } + size_t processors = this->GetProcessorsUsed(test); if (this->TestLoad > 0) { // Exclude tests that are too big to fit in the spare load. -- cgit v0.12 From 5ff0b4ed57e4ccea77e78e128765e3ecc7e86f50 Mon Sep 17 00:00:00 2001 From: Brad King Date: Fri, 20 Oct 2023 18:16:01 -0400 Subject: cmCTestMultiProcessHandler: Consolidate test readiness checks --- Source/CTest/cmCTestMultiProcessHandler.cxx | 24 +++++++++++++----------- 1 file changed, 13 insertions(+), 11 deletions(-) diff --git a/Source/CTest/cmCTestMultiProcessHandler.cxx b/Source/CTest/cmCTestMultiProcessHandler.cxx index 3759af1..d97148d 100644 --- a/Source/CTest/cmCTestMultiProcessHandler.cxx +++ b/Source/CTest/cmCTestMultiProcessHandler.cxx @@ -449,17 +449,6 @@ std::string cmCTestMultiProcessHandler::GetName(int test) bool cmCTestMultiProcessHandler::StartTest(int test) { - // Check for locked project resources. - for (std::string const& i : this->Properties[test]->ProjectResources) { - if (cm::contains(this->ProjectResourcesLocked, i)) { - return false; - } - } - - if (!this->AllocateResources(test)) { - return false; - } - return this->StartTestProcess(test); } @@ -576,6 +565,19 @@ void cmCTestMultiProcessHandler::StartNextTests() continue; } + // Exclude tests that depend on currently-locked project resources. + for (std::string const& i : this->Properties[test]->ProjectResources) { + if (cm::contains(this->ProjectResourcesLocked, i)) { + continue; + } + } + + // Allocate system resources needed by this test. + if (!this->AllocateResources(test)) { + continue; + } + + // The test is ready to run. if (this->StartTest(test)) { numToStart -= processors; } -- cgit v0.12 From 61e98ca33baf3fa6762e5e44286fb93b2a4a173c Mon Sep 17 00:00:00 2001 From: Brad King Date: Mon, 20 Nov 2023 20:23:38 -0500 Subject: cmCTestMultiProcessHandler: Factor out loop startup and teardown --- Source/CTest/cmCTestMultiProcessHandler.cxx | 14 ++++++++++++-- Source/CTest/cmCTestMultiProcessHandler.h | 3 +++ 2 files changed, 15 insertions(+), 2 deletions(-) diff --git a/Source/CTest/cmCTestMultiProcessHandler.cxx b/Source/CTest/cmCTestMultiProcessHandler.cxx index d97148d..5c3e999 100644 --- a/Source/CTest/cmCTestMultiProcessHandler.cxx +++ b/Source/CTest/cmCTestMultiProcessHandler.cxx @@ -125,6 +125,16 @@ bool cmCTestMultiProcessHandler::Complete() return this->Completed == this->Total; } +void cmCTestMultiProcessHandler::InitializeLoop() +{ + this->Loop.init(); +} + +void cmCTestMultiProcessHandler::FinalizeLoop() +{ + this->Loop.reset(); +} + void cmCTestMultiProcessHandler::RunTests() { this->CheckResume(); @@ -133,10 +143,10 @@ void cmCTestMultiProcessHandler::RunTests() } this->TestHandler->SetMaxIndex(this->FindMaxIndex()); - this->Loop.init(); + this->InitializeLoop(); this->StartNextTests(); uv_run(this->Loop, UV_RUN_DEFAULT); - this->Loop.reset(); + this->FinalizeLoop(); if (!this->StopTimePassed && !this->CheckStopOnFailure()) { assert(this->Complete()); diff --git a/Source/CTest/cmCTestMultiProcessHandler.h b/Source/CTest/cmCTestMultiProcessHandler.h index 6832786..d51a33a 100644 --- a/Source/CTest/cmCTestMultiProcessHandler.h +++ b/Source/CTest/cmCTestMultiProcessHandler.h @@ -148,6 +148,9 @@ protected: bool CheckStopTimePassed(); void SetStopTimePassed(); + void InitializeLoop(); + void FinalizeLoop(); + void LockResources(int index); void UnlockResources(int index); -- cgit v0.12 From 9d8415c17bff2319aa5eb9cbd272b557160d9efc Mon Sep 17 00:00:00 2001 From: Brad King Date: Mon, 20 Nov 2023 18:39:08 -0500 Subject: cmCTestMultiProcessHandler: Clarify test-load retry timer infrastructure --- Source/CTest/cmCTestMultiProcessHandler.cxx | 49 +++++++++++++---------------- Source/CTest/cmCTestMultiProcessHandler.h | 6 ++-- 2 files changed, 23 insertions(+), 32 deletions(-) diff --git a/Source/CTest/cmCTestMultiProcessHandler.cxx b/Source/CTest/cmCTestMultiProcessHandler.cxx index 5c3e999..bb6bb9d 100644 --- a/Source/CTest/cmCTestMultiProcessHandler.cxx +++ b/Source/CTest/cmCTestMultiProcessHandler.cxx @@ -128,10 +128,12 @@ bool cmCTestMultiProcessHandler::Complete() void cmCTestMultiProcessHandler::InitializeLoop() { this->Loop.init(); + this->StartNextTestsOnTimer_.init(*this->Loop, this); } void cmCTestMultiProcessHandler::FinalizeLoop() { + this->StartNextTestsOnTimer_.reset(); this->Loop.reset(); } @@ -464,22 +466,12 @@ bool cmCTestMultiProcessHandler::StartTest(int test) void cmCTestMultiProcessHandler::StartNextTests() { - if (this->TestLoadRetryTimer.get() != nullptr) { - // This timer may be waiting to call StartNextTests again. - // Since we have been called it is no longer needed. - uv_timer_stop(this->TestLoadRetryTimer); - } + // One or more events may be scheduled to call this method again. + // Since this method has been called they are no longer needed. + this->StartNextTestsOnTimer_.stop(); - if (this->PendingTests.empty()) { - this->TestLoadRetryTimer.reset(); - return; - } - - if (this->CheckStopTimePassed()) { - return; - } - - if (this->CheckStopOnFailure() && !this->Failed->empty()) { + if (this->PendingTests.empty() || this->CheckStopTimePassed() || + (this->CheckStopOnFailure() && !this->Failed->empty())) { return; } @@ -627,23 +619,24 @@ void cmCTestMultiProcessHandler::StartNextTests() } cmCTestLog(this->CTest, HANDLER_VERBOSE_OUTPUT, "*****" << std::endl); - // Wait between 1 and 5 seconds before trying again. - unsigned int milliseconds = (cmSystemTools::RandomSeed() % 5 + 1) * 1000; - if (this->FakeLoadForTesting) { - milliseconds = 10; - } - if (this->TestLoadRetryTimer.get() == nullptr) { - this->TestLoadRetryTimer.init(*this->Loop, this); - } - this->TestLoadRetryTimer.start( - &cmCTestMultiProcessHandler::OnTestLoadRetryCB, milliseconds, 0); + // Try again later when the load might be lower. + this->StartNextTestsOnTimer(); } } -void cmCTestMultiProcessHandler::OnTestLoadRetryCB(uv_timer_t* timer) +void cmCTestMultiProcessHandler::StartNextTestsOnTimer() { - auto* self = static_cast(timer->data); - self->StartNextTests(); + // Wait between 1 and 5 seconds before trying again. + unsigned int const milliseconds = this->FakeLoadForTesting + ? 10 + : (cmSystemTools::RandomSeed() % 5 + 1) * 1000; + this->StartNextTestsOnTimer_.start( + [](uv_timer_t* timer) { + uv_timer_stop(timer); + auto* self = static_cast(timer->data); + self->StartNextTests(); + }, + milliseconds, 0); } void cmCTestMultiProcessHandler::FinishTestProcess( diff --git a/Source/CTest/cmCTestMultiProcessHandler.h b/Source/CTest/cmCTestMultiProcessHandler.h index d51a33a..cac35e3 100644 --- a/Source/CTest/cmCTestMultiProcessHandler.h +++ b/Source/CTest/cmCTestMultiProcessHandler.h @@ -14,8 +14,6 @@ #include -#include - #include "cmCTest.h" #include "cmCTestResourceAllocator.h" #include "cmCTestResourceSpec.h" @@ -132,7 +130,7 @@ protected: void ErasePendingTest(int index); void FinishTestProcess(std::unique_ptr runner, bool started); - static void OnTestLoadRetryCB(uv_timer_t* timer); + void StartNextTestsOnTimer(); void RemoveTest(int index); // Check if we need to resume an interrupted test set @@ -209,7 +207,7 @@ protected: unsigned long TestLoad; unsigned long FakeLoadForTesting; cm::uv_loop_ptr Loop; - cm::uv_timer_ptr TestLoadRetryTimer; + cm::uv_timer_ptr StartNextTestsOnTimer_; cmCTestTestHandler* TestHandler; cmCTest* CTest; bool HasCycles; -- cgit v0.12 From e528cd795fb33775f326af87628370d7798fab10 Mon Sep 17 00:00:00 2001 From: Brad King Date: Mon, 20 Nov 2023 18:49:50 -0500 Subject: cmCTestMultiProcessHandler: Start new tests asynchronously When a test finishes, defer starting new tests until the next loop iteration. That way, if multiple tests finish in a single loop iteration, we can free all of their resources first, and then start a new batch of tests. --- Source/CTest/cmCTestMultiProcessHandler.cxx | 17 +++++++++++++++-- Source/CTest/cmCTestMultiProcessHandler.h | 2 ++ 2 files changed, 17 insertions(+), 2 deletions(-) diff --git a/Source/CTest/cmCTestMultiProcessHandler.cxx b/Source/CTest/cmCTestMultiProcessHandler.cxx index bb6bb9d..a673370 100644 --- a/Source/CTest/cmCTestMultiProcessHandler.cxx +++ b/Source/CTest/cmCTestMultiProcessHandler.cxx @@ -128,12 +128,14 @@ bool cmCTestMultiProcessHandler::Complete() void cmCTestMultiProcessHandler::InitializeLoop() { this->Loop.init(); + this->StartNextTestsOnIdle_.init(*this->Loop, this); this->StartNextTestsOnTimer_.init(*this->Loop, this); } void cmCTestMultiProcessHandler::FinalizeLoop() { this->StartNextTestsOnTimer_.reset(); + this->StartNextTestsOnIdle_.reset(); this->Loop.reset(); } @@ -146,7 +148,7 @@ void cmCTestMultiProcessHandler::RunTests() this->TestHandler->SetMaxIndex(this->FindMaxIndex()); this->InitializeLoop(); - this->StartNextTests(); + this->StartNextTestsOnIdle(); uv_run(this->Loop, UV_RUN_DEFAULT); this->FinalizeLoop(); @@ -468,6 +470,7 @@ void cmCTestMultiProcessHandler::StartNextTests() { // One or more events may be scheduled to call this method again. // Since this method has been called they are no longer needed. + this->StartNextTestsOnIdle_.stop(); this->StartNextTestsOnTimer_.stop(); if (this->PendingTests.empty() || this->CheckStopTimePassed() || @@ -624,6 +627,16 @@ void cmCTestMultiProcessHandler::StartNextTests() } } +void cmCTestMultiProcessHandler::StartNextTestsOnIdle() +{ + // Start more tests on the next loop iteration. + this->StartNextTestsOnIdle_.start([](uv_idle_t* idle) { + uv_idle_stop(idle); + auto* self = static_cast(idle->data); + self->StartNextTests(); + }); +} + void cmCTestMultiProcessHandler::StartNextTestsOnTimer() { // Wait between 1 and 5 seconds before trying again. @@ -682,7 +695,7 @@ void cmCTestMultiProcessHandler::FinishTestProcess( runner.reset(); if (started) { - this->StartNextTests(); + this->StartNextTestsOnIdle(); } } diff --git a/Source/CTest/cmCTestMultiProcessHandler.h b/Source/CTest/cmCTestMultiProcessHandler.h index cac35e3..cc230e7 100644 --- a/Source/CTest/cmCTestMultiProcessHandler.h +++ b/Source/CTest/cmCTestMultiProcessHandler.h @@ -130,6 +130,7 @@ protected: void ErasePendingTest(int index); void FinishTestProcess(std::unique_ptr runner, bool started); + void StartNextTestsOnIdle(); void StartNextTestsOnTimer(); void RemoveTest(int index); @@ -207,6 +208,7 @@ protected: unsigned long TestLoad; unsigned long FakeLoadForTesting; cm::uv_loop_ptr Loop; + cm::uv_idle_ptr StartNextTestsOnIdle_; cm::uv_timer_ptr StartNextTestsOnTimer_; cmCTestTestHandler* TestHandler; cmCTest* CTest; -- cgit v0.12 From 086a41c0f3fec3068b9bf1b7ebe094bf1958ee1b Mon Sep 17 00:00:00 2001 From: Brad King Date: Mon, 20 Nov 2023 19:09:03 -0500 Subject: cmCTestMultiProcessHandler: Simplify test startup batching Once a test is ready to run, count it against the number of tests to start in the current batch whether or not the test process actually starts successfully. If a test process does fail to start, simply schedule a new startup batch on the next loop iteration. --- Source/CTest/cmCTestMultiProcessHandler.cxx | 23 ++++++++++------------- Source/CTest/cmCTestMultiProcessHandler.h | 4 ++-- Source/CTest/cmCTestRunTest.cxx | 5 +---- Source/CTest/cmCTestRunTest.h | 2 +- 4 files changed, 14 insertions(+), 20 deletions(-) diff --git a/Source/CTest/cmCTestMultiProcessHandler.cxx b/Source/CTest/cmCTestMultiProcessHandler.cxx index a673370..a9930df 100644 --- a/Source/CTest/cmCTestMultiProcessHandler.cxx +++ b/Source/CTest/cmCTestMultiProcessHandler.cxx @@ -162,7 +162,7 @@ void cmCTestMultiProcessHandler::RunTests() this->UpdateCostData(); } -bool cmCTestMultiProcessHandler::StartTestProcess(int test) +void cmCTestMultiProcessHandler::StartTestProcess(int test) { if (this->HaveAffinity && this->Properties[test]->WantAffinity) { size_t needProcessors = this->GetProcessorsUsed(test); @@ -241,7 +241,7 @@ bool cmCTestMultiProcessHandler::StartTestProcess(int test) e << "Resource spec file:\n\n " << this->ResourceSpecFile; cmCTestRunTest::StartFailure(std::move(testRun), this->Total, e.str(), "Insufficient resources"); - return false; + return; } cmWorkingDirectory workdir(this->Properties[test]->Directory); @@ -251,13 +251,12 @@ bool cmCTestMultiProcessHandler::StartTestProcess(int test) this->Properties[test]->Directory + " : " + std::strerror(workdir.GetLastResult()), "Failed to change working directory"); - return false; + return; } // Ownership of 'testRun' has moved to another structure. // When the test finishes, FinishTestProcess will be called. - return cmCTestRunTest::StartTest(std::move(testRun), this->Completed, - this->Total); + cmCTestRunTest::StartTest(std::move(testRun), this->Completed, this->Total); } bool cmCTestMultiProcessHandler::AllocateResources(int index) @@ -461,9 +460,9 @@ std::string cmCTestMultiProcessHandler::GetName(int test) return this->Properties[test]->Name; } -bool cmCTestMultiProcessHandler::StartTest(int test) +void cmCTestMultiProcessHandler::StartTest(int test) { - return this->StartTestProcess(test); + this->StartTestProcess(test); } void cmCTestMultiProcessHandler::StartNextTests() @@ -583,9 +582,8 @@ void cmCTestMultiProcessHandler::StartNextTests() } // The test is ready to run. - if (this->StartTest(test)) { - numToStart -= processors; - } + numToStart -= processors; + this->StartTest(test); } if (allTestsFailedTestLoadCheck) { @@ -694,9 +692,8 @@ void cmCTestMultiProcessHandler::FinishTestProcess( properties->Affinity.clear(); runner.reset(); - if (started) { - this->StartNextTestsOnIdle(); - } + + this->StartNextTestsOnIdle(); } void cmCTestMultiProcessHandler::UpdateCostData() diff --git a/Source/CTest/cmCTestMultiProcessHandler.h b/Source/CTest/cmCTestMultiProcessHandler.h index cc230e7..ffbe16f 100644 --- a/Source/CTest/cmCTestMultiProcessHandler.h +++ b/Source/CTest/cmCTestMultiProcessHandler.h @@ -108,8 +108,8 @@ protected: // Start the next test or tests as many as are allowed by // ParallelLevel void StartNextTests(); - bool StartTestProcess(int test); - bool StartTest(int test); + void StartTestProcess(int test); + void StartTest(int test); // Mark the checkpoint for the given test void WriteCheckpoint(int index); diff --git a/Source/CTest/cmCTestRunTest.cxx b/Source/CTest/cmCTestRunTest.cxx index e96dc1f..483b3b4 100644 --- a/Source/CTest/cmCTestRunTest.cxx +++ b/Source/CTest/cmCTestRunTest.cxx @@ -530,7 +530,7 @@ std::string cmCTestRunTest::GetTestPrefix(size_t completed, size_t total) const return outputStream.str(); } -bool cmCTestRunTest::StartTest(std::unique_ptr runner, +void cmCTestRunTest::StartTest(std::unique_ptr runner, size_t completed, size_t total) { auto* testRun = runner.get(); @@ -539,10 +539,7 @@ bool cmCTestRunTest::StartTest(std::unique_ptr runner, if (!testRun->StartTest(completed, total)) { testRun->FinalizeTest(false); - return false; } - - return true; } // Starts the execution of a test. Returns once it has started diff --git a/Source/CTest/cmCTestRunTest.h b/Source/CTest/cmCTestRunTest.h index 154abca..71d0865 100644 --- a/Source/CTest/cmCTestRunTest.h +++ b/Source/CTest/cmCTestRunTest.h @@ -56,7 +56,7 @@ public: // Read and store output. Returns true if it must be called again. void CheckOutput(std::string const& line); - static bool StartTest(std::unique_ptr runner, + static void StartTest(std::unique_ptr runner, size_t completed, size_t total); static bool StartAgain(std::unique_ptr runner, size_t completed); -- cgit v0.12