From c896dc78f970aa154a722e65dc099d2c6d3b3bcd Mon Sep 17 00:00:00 2001 From: Brad King Date: Fri, 17 Nov 2023 18:31:26 -0500 Subject: Tests: Cover ctest waiting on insufficient non-zero spare test-load --- Tests/RunCMake/CTestCommandLine/RunCMakeTest.cmake | 10 ++++++---- .../CTestCommandLine/test-load-wait-stdout.txt | 21 --------------------- .../CTestCommandLine/test-load-wait1-stdout.txt | 21 +++++++++++++++++++++ .../ctest_test/CTestTestLoadWait-stdout.txt | 15 --------------- .../ctest_test/CTestTestLoadWait1-stdout.txt | 15 +++++++++++++++ Tests/RunCMake/ctest_test/RunCMakeTest.cmake | 16 ++++++++++------ Tests/RunCMake/ctest_test/TestLoadWait-stdout.txt | 15 --------------- Tests/RunCMake/ctest_test/TestLoadWait1-stdout.txt | 15 +++++++++++++++ 8 files changed, 67 insertions(+), 61 deletions(-) delete mode 100644 Tests/RunCMake/CTestCommandLine/test-load-wait-stdout.txt create mode 100644 Tests/RunCMake/CTestCommandLine/test-load-wait1-stdout.txt delete mode 100644 Tests/RunCMake/ctest_test/CTestTestLoadWait-stdout.txt create mode 100644 Tests/RunCMake/ctest_test/CTestTestLoadWait1-stdout.txt delete mode 100644 Tests/RunCMake/ctest_test/TestLoadWait-stdout.txt create mode 100644 Tests/RunCMake/ctest_test/TestLoadWait1-stdout.txt diff --git a/Tests/RunCMake/CTestCommandLine/RunCMakeTest.cmake b/Tests/RunCMake/CTestCommandLine/RunCMakeTest.cmake index 223a61c..8a9cc8c 100644 --- a/Tests/RunCMake/CTestCommandLine/RunCMakeTest.cmake +++ b/Tests/RunCMake/CTestCommandLine/RunCMakeTest.cmake @@ -229,19 +229,21 @@ function(run_TestLoad name load) file(MAKE_DIRECTORY "${RunCMake_TEST_BINARY_DIR}") file(WRITE "${RunCMake_TEST_BINARY_DIR}/CTestTestfile.cmake" " add_test(TestLoad1 \"${CMAKE_COMMAND}\" -E echo \"test of --test-load\") + set_tests_properties(TestLoad1 PROPERTIES PROCESSORS 2) add_test(TestLoad2 \"${CMAKE_COMMAND}\" -E echo \"test of --test-load\") + set_tests_properties(TestLoad2 PROPERTIES PROCESSORS 2) ") - run_cmake_command(${name} ${CMAKE_CTEST_COMMAND} -VV -j2 --test-load ${load}) + run_cmake_command(${name} ${CMAKE_CTEST_COMMAND} -VV -j8 --test-load ${load}) endfunction() # Tests for the --test-load feature of ctest # # Spoof a load average value to make these tests more reliable. -set(ENV{__CTEST_FAKE_LOAD_AVERAGE_FOR_TESTING} 5) +set(ENV{__CTEST_FAKE_LOAD_AVERAGE_FOR_TESTING} 7) # Verify that new tests are not started when the load average exceeds # our threshold and that they then run once the load average drops. -run_TestLoad(test-load-wait 3) +run_TestLoad(test-load-wait1 8) # Verify that warning message is displayed but tests still start when # an invalid argument is given. @@ -249,7 +251,7 @@ run_TestLoad(test-load-invalid 'two') # Verify that new tests are started when the load average falls below # our threshold. -run_TestLoad(test-load-pass 10) +run_TestLoad(test-load-pass 12) unset(ENV{__CTEST_FAKE_LOAD_AVERAGE_FOR_TESTING}) diff --git a/Tests/RunCMake/CTestCommandLine/test-load-wait-stdout.txt b/Tests/RunCMake/CTestCommandLine/test-load-wait-stdout.txt deleted file mode 100644 index db7d7f3..0000000 --- a/Tests/RunCMake/CTestCommandLine/test-load-wait-stdout.txt +++ /dev/null @@ -1,21 +0,0 @@ -Test project [^ -]*/Tests/RunCMake/CTestCommandLine/TestLoad( -[^*][^ -]*)* -\*\*\*\*\* WAITING, System Load: 5, Max Allowed Load: 3, Smallest test TestLoad[1-2] requires 1\*\*\*\*\* -test 1 - Start 1: TestLoad1 -+( -[^*][^ -]*)* -test 2 - Start 2: TestLoad2 -+( -[^*][^ -]*)* -1/2 Test #[1-2]: TestLoad[1-2] ........................ Passed +[0-9.]+ sec( -[^*][^ -]*)* -2/2 Test #[1-2]: TestLoad[1-2] ........................ Passed +[0-9.]+ sec -+ -100% tests passed, 0 tests failed out of 2 diff --git a/Tests/RunCMake/CTestCommandLine/test-load-wait1-stdout.txt b/Tests/RunCMake/CTestCommandLine/test-load-wait1-stdout.txt new file mode 100644 index 0000000..aa91950 --- /dev/null +++ b/Tests/RunCMake/CTestCommandLine/test-load-wait1-stdout.txt @@ -0,0 +1,21 @@ +Test project [^ +]*/Tests/RunCMake/CTestCommandLine/TestLoad( +[^*][^ +]*)* +\*\*\*\*\* WAITING, System Load: 7, Max Allowed Load: 8, Smallest test TestLoad[1-2] requires 2\*\*\*\*\* +test 1 + Start 1: TestLoad1 ++( +[^*][^ +]*)* +test 2 + Start 2: TestLoad2 ++( +[^*][^ +]*)* +1/2 Test #[1-2]: TestLoad[1-2] ........................ Passed +[0-9.]+ sec( +[^*][^ +]*)* +2/2 Test #[1-2]: TestLoad[1-2] ........................ Passed +[0-9.]+ sec ++ +100% tests passed, 0 tests failed out of 2 diff --git a/Tests/RunCMake/ctest_test/CTestTestLoadWait-stdout.txt b/Tests/RunCMake/ctest_test/CTestTestLoadWait-stdout.txt deleted file mode 100644 index 2f4468f..0000000 --- a/Tests/RunCMake/ctest_test/CTestTestLoadWait-stdout.txt +++ /dev/null @@ -1,15 +0,0 @@ -Test project [^ -]*/Tests/RunCMake/ctest_test/CTestTestLoadWait-build( -[^*][^ -]*)* -\*\*\*\*\* WAITING, System Load: 5, Max Allowed Load: 4, Smallest test RunCMakeVersion requires 1\*\*\*\*\* -test 1 - Start 1: RunCMakeVersion -+( -[^*][^ -]*)* -1/1 Test #1: RunCMakeVersion .................. Passed +[0-9.]+ sec -+ -100% tests passed, 0 tests failed out of 1 -+ -Total Test time \(real\) = +[0-9.]+ sec$ diff --git a/Tests/RunCMake/ctest_test/CTestTestLoadWait1-stdout.txt b/Tests/RunCMake/ctest_test/CTestTestLoadWait1-stdout.txt new file mode 100644 index 0000000..70d8d3e --- /dev/null +++ b/Tests/RunCMake/ctest_test/CTestTestLoadWait1-stdout.txt @@ -0,0 +1,15 @@ +Test project [^ +]*/Tests/RunCMake/ctest_test/CTestTestLoadWait1-build( +[^*][^ +]*)* +\*\*\*\*\* WAITING, System Load: 7, Max Allowed Load: 8, Smallest test RunCMakeVersion requires 2\*\*\*\*\* +test 1 + Start 1: RunCMakeVersion ++( +[^*][^ +]*)* +1/1 Test #1: RunCMakeVersion .................. Passed +[0-9.]+ sec ++ +100% tests passed, 0 tests failed out of 1 ++ +Total Test time \(real\) = +[0-9.]+ sec$ diff --git a/Tests/RunCMake/ctest_test/RunCMakeTest.cmake b/Tests/RunCMake/ctest_test/RunCMakeTest.cmake index d2f3da3..9570c77 100644 --- a/Tests/RunCMake/ctest_test/RunCMakeTest.cmake +++ b/Tests/RunCMake/ctest_test/RunCMakeTest.cmake @@ -14,16 +14,19 @@ run_ctest_test(TestQuiet QUIET) # Tests for the 'Test Load' feature of ctest # # Spoof a load average value to make these tests more reliable. -set(ENV{__CTEST_FAKE_LOAD_AVERAGE_FOR_TESTING} 5) +set(ENV{__CTEST_FAKE_LOAD_AVERAGE_FOR_TESTING} 7) set(RunCTest_VERBOSE_FLAG -VV) +set(CASE_CMAKELISTS_SUFFIX_CODE [[ +set_property(TEST RunCMakeVersion PROPERTY PROCESSORS 2) +]]) # Verify that new tests are started when the load average falls below # our threshold. -run_ctest_test(TestLoadPass TEST_LOAD 6) +run_ctest_test(TestLoadPass TEST_LOAD 8) # Verify that new tests are not started when the load average exceeds # our threshold and that they then run once the load average drops. -run_ctest_test(TestLoadWait TEST_LOAD 2) +run_ctest_test(TestLoadWait1 TEST_LOAD 8 PARALLEL_LEVEL 8) # Verify that when an invalid "TEST_LOAD" value is given, a warning # message is displayed and the value is ignored. @@ -31,13 +34,13 @@ run_ctest_test(TestLoadInvalid TEST_LOAD "ERR1") # Verify that new tests are started when the load average falls below # our threshold. -set(CASE_CTEST_TEST_LOAD 7) +set(CASE_CTEST_TEST_LOAD 9) run_ctest_test(CTestTestLoadPass) # Verify that new tests are not started when the load average exceeds # our threshold and that they then run once the load average drops. -set(CASE_CTEST_TEST_LOAD 4) -run_ctest_test(CTestTestLoadWait) +set(CASE_CTEST_TEST_LOAD 8) +run_ctest_test(CTestTestLoadWait1 PARALLEL_LEVEL 8) # Verify that when an invalid "CTEST_TEST_LOAD" value is given, # a warning message is displayed and the value is ignored. @@ -51,6 +54,7 @@ run_ctest_test(TestLoadOrder TEST_LOAD "ERR4") unset(ENV{__CTEST_FAKE_LOAD_AVERAGE_FOR_TESTING}) unset(CASE_CTEST_TEST_LOAD) +unset(CASE_CMAKELISTS_SUFFIX_CODE) unset(RunCTest_VERBOSE_FLAG) block() diff --git a/Tests/RunCMake/ctest_test/TestLoadWait-stdout.txt b/Tests/RunCMake/ctest_test/TestLoadWait-stdout.txt deleted file mode 100644 index fc32958..0000000 --- a/Tests/RunCMake/ctest_test/TestLoadWait-stdout.txt +++ /dev/null @@ -1,15 +0,0 @@ -Test project [^ -]*/Tests/RunCMake/ctest_test/TestLoadWait-build( -[^*][^ -]*)* -\*\*\*\*\* WAITING, System Load: 5, Max Allowed Load: 2, Smallest test RunCMakeVersion requires 1\*\*\*\*\* -test 1 - Start 1: RunCMakeVersion -+( -[^*][^ -]*)* -1/1 Test #1: RunCMakeVersion .................. Passed +[0-9.]+ sec -+ -100% tests passed, 0 tests failed out of 1 -+ -Total Test time \(real\) = +[0-9.]+ sec$ diff --git a/Tests/RunCMake/ctest_test/TestLoadWait1-stdout.txt b/Tests/RunCMake/ctest_test/TestLoadWait1-stdout.txt new file mode 100644 index 0000000..bca3e54 --- /dev/null +++ b/Tests/RunCMake/ctest_test/TestLoadWait1-stdout.txt @@ -0,0 +1,15 @@ +Test project [^ +]*/Tests/RunCMake/ctest_test/TestLoadWait1-build( +[^*][^ +]*)* +\*\*\*\*\* WAITING, System Load: 7, Max Allowed Load: 8, Smallest test RunCMakeVersion requires 2\*\*\*\*\* +test 1 + Start 1: RunCMakeVersion ++( +[^*][^ +]*)* +1/1 Test #1: RunCMakeVersion .................. Passed +[0-9.]+ sec ++ +100% tests passed, 0 tests failed out of 1 ++ +Total Test time \(real\) = +[0-9.]+ sec$ -- cgit v0.12 From 1f8f270f625d32486d1af7e7eac691bcafb5c2e4 Mon Sep 17 00:00:00 2001 From: Brad King Date: Wed, 15 Nov 2023 14:37:52 -0500 Subject: cmCTestRunTest: Consolidate initialization in constructor --- Source/CTest/cmCTestMultiProcessHandler.cxx | 12 +++--------- Source/CTest/cmCTestRunTest.cxx | 9 ++++++--- Source/CTest/cmCTestRunTest.h | 19 ++++++------------- 3 files changed, 15 insertions(+), 25 deletions(-) diff --git a/Source/CTest/cmCTestMultiProcessHandler.cxx b/Source/CTest/cmCTestMultiProcessHandler.cxx index ca07a08..24e8569 100644 --- a/Source/CTest/cmCTestMultiProcessHandler.cxx +++ b/Source/CTest/cmCTestMultiProcessHandler.cxx @@ -176,14 +176,12 @@ bool cmCTestMultiProcessHandler::StartTestProcess(int test) this->EraseTest(test); this->RunningCount += this->GetProcessorsUsed(test); - auto testRun = cm::make_unique(*this); + auto testRun = cm::make_unique(*this, test); if (this->RepeatMode != cmCTest::Repeat::Never) { testRun->SetRepeatMode(this->RepeatMode); testRun->SetNumberOfRuns(this->RepeatCount); } - testRun->SetIndex(test); - testRun->SetTestProperties(this->Properties[test]); if (this->UseResourceSpec) { testRun->SetUseAllocatedResources(true); testRun->SetAllocatedResources(this->AllocatedResources[test]); @@ -1259,9 +1257,7 @@ void cmCTestMultiProcessHandler::PrintOutputAsJson() // Don't worry if this fails, we are only showing the test list, not // running the tests cmWorkingDirectory workdir(p.Directory); - cmCTestRunTest testRun(*this); - testRun.SetIndex(p.Index); - testRun.SetTestProperties(&p); + cmCTestRunTest testRun(*this, p.Index); testRun.ComputeArguments(); // Skip tests not available in this configuration. @@ -1298,9 +1294,7 @@ void cmCTestMultiProcessHandler::PrintTestList() // running the tests cmWorkingDirectory workdir(p.Directory); - cmCTestRunTest testRun(*this); - testRun.SetIndex(p.Index); - testRun.SetTestProperties(&p); + cmCTestRunTest testRun(*this, p.Index); testRun.ComputeArguments(); // logs the command in verbose mode if (!p.Labels.empty()) // print the labels diff --git a/Source/CTest/cmCTestRunTest.cxx b/Source/CTest/cmCTestRunTest.cxx index 8ceb9db..d6b6d41 100644 --- a/Source/CTest/cmCTestRunTest.cxx +++ b/Source/CTest/cmCTestRunTest.cxx @@ -27,11 +27,14 @@ #include "cmSystemTools.h" #include "cmWorkingDirectory.h" -cmCTestRunTest::cmCTestRunTest(cmCTestMultiProcessHandler& multiHandler) +cmCTestRunTest::cmCTestRunTest(cmCTestMultiProcessHandler& multiHandler, + int index) : MultiTestHandler(multiHandler) + , Index(index) + , CTest(MultiTestHandler.CTest) + , TestHandler(MultiTestHandler.TestHandler) + , TestProperties(MultiTestHandler.Properties[Index]) { - this->CTest = multiHandler.CTest; - this->TestHandler = multiHandler.TestHandler; } void cmCTestRunTest::CheckOutput(std::string const& line) diff --git a/Source/CTest/cmCTestRunTest.h b/Source/CTest/cmCTestRunTest.h index 34f23c4..154abca 100644 --- a/Source/CTest/cmCTestRunTest.h +++ b/Source/CTest/cmCTestRunTest.h @@ -24,7 +24,7 @@ class cmCTestRunTest { public: - explicit cmCTestRunTest(cmCTestMultiProcessHandler& multiHandler); + explicit cmCTestRunTest(cmCTestMultiProcessHandler& multiHandler, int index); void SetNumberOfRuns(int n) { @@ -33,18 +33,12 @@ public: } void SetRepeatMode(cmCTest::Repeat r) { this->RepeatMode = r; } - void SetTestProperties(cmCTestTestHandler::cmCTestTestProperties* prop) - { - this->TestProperties = prop; - } cmCTestTestHandler::cmCTestTestProperties* GetTestProperties() { return this->TestProperties; } - void SetIndex(int i) { this->Index = i; } - int GetIndex() { return this->Index; } void AddFailedDependency(const std::string& failedTest) @@ -124,16 +118,15 @@ private: // Returns "completed/total Test #Index: " std::string GetTestPrefix(size_t completed, size_t total) const; - cmCTestTestHandler::cmCTestTestProperties* TestProperties; - // Pointer back to the "parent"; the handler that invoked this test run - cmCTestTestHandler* TestHandler; + cmCTestMultiProcessHandler& MultiTestHandler; + int Index; cmCTest* CTest; + cmCTestTestHandler* TestHandler; + cmCTestTestHandler::cmCTestTestProperties* TestProperties; + std::unique_ptr TestProcess; std::string ProcessOutput; - // The test results cmCTestTestHandler::cmCTestTestResult TestResult; - cmCTestMultiProcessHandler& MultiTestHandler; - int Index; std::set FailedDependencies; std::string StartTime; std::string ActualCommand; -- cgit v0.12 From 8c307ab5670877454572f9e0b3cdc4ae58cefd7b Mon Sep 17 00:00:00 2001 From: Chris Mahoney Date: Wed, 9 Aug 2023 18:28:09 +0000 Subject: cmCTestMultiProcessHandler: Replace false condition with opposite assert --- Source/CTest/cmCTestMultiProcessHandler.cxx | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/Source/CTest/cmCTestMultiProcessHandler.cxx b/Source/CTest/cmCTestMultiProcessHandler.cxx index 24e8569..b82e968 100644 --- a/Source/CTest/cmCTestMultiProcessHandler.cxx +++ b/Source/CTest/cmCTestMultiProcessHandler.cxx @@ -156,9 +156,7 @@ bool cmCTestMultiProcessHandler::StartTestProcess(int test) { if (this->HaveAffinity && this->Properties[test]->WantAffinity) { size_t needProcessors = this->GetProcessorsUsed(test); - if (needProcessors > this->ProcessorsAvailable.size()) { - return false; - } + assert(needProcessors <= this->ProcessorsAvailable.size()); std::vector affinity; affinity.reserve(needProcessors); for (size_t i = 0; i < needProcessors; ++i) { -- cgit v0.12 From 419443f68f8d1942cedab04777a5d2e7a0caccd8 Mon Sep 17 00:00:00 2001 From: John Parent Date: Thu, 19 Oct 2023 13:26:16 -0400 Subject: cmCTestMultiProcessHandler: Factor out helper to check for completion --- Source/CTest/cmCTestMultiProcessHandler.cxx | 7 ++++++- Source/CTest/cmCTestMultiProcessHandler.h | 1 + 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/Source/CTest/cmCTestMultiProcessHandler.cxx b/Source/CTest/cmCTestMultiProcessHandler.cxx index b82e968..d71a34d 100644 --- a/Source/CTest/cmCTestMultiProcessHandler.cxx +++ b/Source/CTest/cmCTestMultiProcessHandler.cxx @@ -126,6 +126,11 @@ void cmCTestMultiProcessHandler::SetTestLoad(unsigned long load) } } +bool cmCTestMultiProcessHandler::Complete() +{ + return this->Completed == this->Total; +} + void cmCTestMultiProcessHandler::RunTests() { this->CheckResume(); @@ -143,7 +148,7 @@ void cmCTestMultiProcessHandler::RunTests() uv_loop_close(&this->Loop); if (!this->StopTimePassed && !this->CheckStopOnFailure()) { - assert(this->Completed == this->Total); + assert(this->Complete()); assert(this->Tests.empty()); } assert(this->AllResourcesAvailable()); diff --git a/Source/CTest/cmCTestMultiProcessHandler.h b/Source/CTest/cmCTestMultiProcessHandler.h index 3b4e9c5..a002bcc 100644 --- a/Source/CTest/cmCTestMultiProcessHandler.h +++ b/Source/CTest/cmCTestMultiProcessHandler.h @@ -152,6 +152,7 @@ protected: InsufficientResources, }; + bool Complete(); bool AllocateResources(int index); bool TryAllocateResources( int index, -- cgit v0.12 From 451429e19c235cb2200be1c7aba3f6b29f389857 Mon Sep 17 00:00:00 2001 From: Brad King Date: Wed, 1 Nov 2023 16:48:03 -0400 Subject: cmCTestMultiProcessHandler: Use cm::uv_loop_ptr abstraction This ensures all loop resources are released. --- Source/CTest/cmCTestMultiProcessHandler.cxx | 8 ++++---- Source/CTest/cmCTestMultiProcessHandler.h | 2 +- Source/CTest/cmCTestRunTest.cxx | 3 ++- 3 files changed, 7 insertions(+), 6 deletions(-) diff --git a/Source/CTest/cmCTestMultiProcessHandler.cxx b/Source/CTest/cmCTestMultiProcessHandler.cxx index d71a34d..c4e3759 100644 --- a/Source/CTest/cmCTestMultiProcessHandler.cxx +++ b/Source/CTest/cmCTestMultiProcessHandler.cxx @@ -142,10 +142,10 @@ void cmCTestMultiProcessHandler::RunTests() #endif this->TestHandler->SetMaxIndex(this->FindMaxIndex()); - uv_loop_init(&this->Loop); + this->Loop.init(); this->StartNextTests(); - uv_run(&this->Loop, UV_RUN_DEFAULT); - uv_loop_close(&this->Loop); + uv_run(this->Loop, UV_RUN_DEFAULT); + this->Loop.reset(); if (!this->StopTimePassed && !this->CheckStopOnFailure()) { assert(this->Complete()); @@ -618,7 +618,7 @@ void cmCTestMultiProcessHandler::StartNextTests() milliseconds = 10; } if (this->TestLoadRetryTimer.get() == nullptr) { - this->TestLoadRetryTimer.init(this->Loop, this); + this->TestLoadRetryTimer.init(*this->Loop, this); } this->TestLoadRetryTimer.start( &cmCTestMultiProcessHandler::OnTestLoadRetryCB, milliseconds, 0); diff --git a/Source/CTest/cmCTestMultiProcessHandler.h b/Source/CTest/cmCTestMultiProcessHandler.h index a002bcc..8c118a6 100644 --- a/Source/CTest/cmCTestMultiProcessHandler.h +++ b/Source/CTest/cmCTestMultiProcessHandler.h @@ -201,7 +201,7 @@ protected: size_t ParallelLevel; // max number of process that can be run at once unsigned long TestLoad; unsigned long FakeLoadForTesting; - uv_loop_t Loop; + cm::uv_loop_ptr Loop; cm::uv_timer_ptr TestLoadRetryTimer; cmCTestTestHandler* TestHandler; cmCTest* CTest; diff --git a/Source/CTest/cmCTestRunTest.cxx b/Source/CTest/cmCTestRunTest.cxx index d6b6d41..4b97365 100644 --- a/Source/CTest/cmCTestRunTest.cxx +++ b/Source/CTest/cmCTestRunTest.cxx @@ -25,6 +25,7 @@ #include "cmProcess.h" #include "cmStringAlgorithms.h" #include "cmSystemTools.h" +#include "cmUVHandlePtr.h" #include "cmWorkingDirectory.h" cmCTestRunTest::cmCTestRunTest(cmCTestMultiProcessHandler& multiHandler, @@ -890,7 +891,7 @@ bool cmCTestRunTest::ForkProcess() this->TestResult.Environment.erase(this->TestResult.Environment.length() - 1); - return this->TestProcess->StartProcess(this->MultiTestHandler.Loop, + return this->TestProcess->StartProcess(*this->MultiTestHandler.Loop, &this->TestProperties->Affinity); } -- cgit v0.12 From d6d114f3e8ad7333b9dcf18c9c583e778d334659 Mon Sep 17 00:00:00 2001 From: Brad King Date: Mon, 13 Nov 2023 18:37:14 -0500 Subject: cmCTestMultiProcessHandler: Remove unused members The `TestRunningMap` and `TestFinishMap` members have not been used since commit 44017a4767 (CTest: handle dependent and non dependent test requirements equally, 2013-10-17, v3.0.0-rc1~451^2~7). --- Source/CTest/cmCTestMultiProcessHandler.cxx | 10 ---------- Source/CTest/cmCTestMultiProcessHandler.h | 2 -- 2 files changed, 12 deletions(-) diff --git a/Source/CTest/cmCTestMultiProcessHandler.cxx b/Source/CTest/cmCTestMultiProcessHandler.cxx index c4e3759..c80bfff 100644 --- a/Source/CTest/cmCTestMultiProcessHandler.cxx +++ b/Source/CTest/cmCTestMultiProcessHandler.cxx @@ -89,11 +89,6 @@ void cmCTestMultiProcessHandler::SetTests(TestMap& tests, this->Tests = tests; this->Properties = properties; this->Total = this->Tests.size(); - // set test run map to false for all - for (auto const& t : this->Tests) { - this->TestRunningMap[t.first] = false; - this->TestFinishMap[t.first] = false; - } if (!this->CTest->GetShowOnly()) { this->ReadCostData(); this->HasCycles = !this->CheckCycles(); @@ -174,7 +169,6 @@ bool cmCTestMultiProcessHandler::StartTestProcess(int test) cmCTestOptionalLog(this->CTest, HANDLER_VERBOSE_OUTPUT, "test " << test << "\n", this->Quiet); - this->TestRunningMap[test] = true; // mark the test as running // now remove the test itself this->EraseTest(test); this->RunningCount += this->GetProcessorsUsed(test); @@ -662,8 +656,6 @@ void cmCTestMultiProcessHandler::FinishTestProcess( t.second.erase(test); } - this->TestFinishMap[test] = true; - this->TestRunningMap[test] = false; this->WriteCheckpoint(test); this->DeallocateResources(test); this->UnlockResources(test); @@ -1393,8 +1385,6 @@ void cmCTestMultiProcessHandler::RemoveTest(int index) { this->EraseTest(index); this->Properties.erase(index); - this->TestRunningMap[index] = false; - this->TestFinishMap[index] = true; this->Completed++; } diff --git a/Source/CTest/cmCTestMultiProcessHandler.h b/Source/CTest/cmCTestMultiProcessHandler.h index 8c118a6..7b288cf 100644 --- a/Source/CTest/cmCTestMultiProcessHandler.h +++ b/Source/CTest/cmCTestMultiProcessHandler.h @@ -184,8 +184,6 @@ protected: bool StopTimePassed = false; // list of test properties (indices concurrent to the test map) PropertiesMap Properties; - std::map TestRunningMap; - std::map TestFinishMap; std::map TestOutput; std::vector* Passed; std::vector* Failed; -- cgit v0.12 From 7bca3f8c800510509be3468f57226daf5708e91c Mon Sep 17 00:00:00 2001 From: Brad King Date: Thu, 2 Nov 2023 15:50:49 -0400 Subject: cmCTestMultiProcessHandler: Avoid extra copy of test maps --- Source/CTest/cmCTestMultiProcessHandler.cxx | 8 ++++---- Source/CTest/cmCTestMultiProcessHandler.h | 2 +- Source/CTest/cmCTestTestHandler.cxx | 2 +- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/Source/CTest/cmCTestMultiProcessHandler.cxx b/Source/CTest/cmCTestMultiProcessHandler.cxx index c80bfff..f2c89f8 100644 --- a/Source/CTest/cmCTestMultiProcessHandler.cxx +++ b/Source/CTest/cmCTestMultiProcessHandler.cxx @@ -83,11 +83,11 @@ cmCTestMultiProcessHandler::cmCTestMultiProcessHandler() cmCTestMultiProcessHandler::~cmCTestMultiProcessHandler() = default; // Set the tests -void cmCTestMultiProcessHandler::SetTests(TestMap& tests, - PropertiesMap& properties) +void cmCTestMultiProcessHandler::SetTests(TestMap tests, + PropertiesMap properties) { - this->Tests = tests; - this->Properties = properties; + this->Tests = std::move(tests); + this->Properties = std::move(properties); this->Total = this->Tests.size(); if (!this->CTest->GetShowOnly()) { this->ReadCostData(); diff --git a/Source/CTest/cmCTestMultiProcessHandler.h b/Source/CTest/cmCTestMultiProcessHandler.h index 7b288cf..4bf0844 100644 --- a/Source/CTest/cmCTestMultiProcessHandler.h +++ b/Source/CTest/cmCTestMultiProcessHandler.h @@ -57,7 +57,7 @@ public: cmCTestMultiProcessHandler(); virtual ~cmCTestMultiProcessHandler(); // Set the tests - void SetTests(TestMap& tests, PropertiesMap& properties); + void SetTests(TestMap tests, PropertiesMap properties); // Set the max number of tests that can be run at the same time. void SetParallelLevel(size_t); void SetTestLoad(unsigned long load); diff --git a/Source/CTest/cmCTestTestHandler.cxx b/Source/CTest/cmCTestTestHandler.cxx index eb3b4dd..2fcef90 100644 --- a/Source/CTest/cmCTestTestHandler.cxx +++ b/Source/CTest/cmCTestTestHandler.cxx @@ -1385,7 +1385,7 @@ bool cmCTestTestHandler::ProcessDirectory(std::vector& passed, properties[p.Index] = &p; } parallel->SetResourceSpecFile(this->ResourceSpecFile); - parallel->SetTests(tests, properties); + parallel->SetTests(std::move(tests), std::move(properties)); parallel->SetPassFailVectors(&passed, &failed); this->TestResults.clear(); parallel->SetTestResults(&this->TestResults); -- cgit v0.12 From b27969c89d4c14c1938825d30fcf70ddab7c0cd1 Mon Sep 17 00:00:00 2001 From: Brad King Date: Thu, 2 Nov 2023 13:09:47 -0400 Subject: cmCTestMultiProcessHandler: Clarify representation of pending tests --- Source/CTest/cmCTestMultiProcessHandler.cxx | 36 ++++++++++++++--------------- Source/CTest/cmCTestMultiProcessHandler.h | 12 ++++++---- Source/CTest/cmCTestTestHandler.cxx | 2 +- 3 files changed, 27 insertions(+), 23 deletions(-) diff --git a/Source/CTest/cmCTestMultiProcessHandler.cxx b/Source/CTest/cmCTestMultiProcessHandler.cxx index f2c89f8..2777b2a 100644 --- a/Source/CTest/cmCTestMultiProcessHandler.cxx +++ b/Source/CTest/cmCTestMultiProcessHandler.cxx @@ -86,9 +86,9 @@ cmCTestMultiProcessHandler::~cmCTestMultiProcessHandler() = default; void cmCTestMultiProcessHandler::SetTests(TestMap tests, PropertiesMap properties) { - this->Tests = std::move(tests); + this->PendingTests = std::move(tests); this->Properties = std::move(properties); - this->Total = this->Tests.size(); + this->Total = this->PendingTests.size(); if (!this->CTest->GetShowOnly()) { this->ReadCostData(); this->HasCycles = !this->CheckCycles(); @@ -144,7 +144,7 @@ void cmCTestMultiProcessHandler::RunTests() if (!this->StopTimePassed && !this->CheckStopOnFailure()) { assert(this->Complete()); - assert(this->Tests.empty()); + assert(this->PendingTests.empty()); } assert(this->AllResourcesAvailable()); @@ -170,7 +170,7 @@ bool cmCTestMultiProcessHandler::StartTestProcess(int test) cmCTestOptionalLog(this->CTest, HANDLER_VERBOSE_OUTPUT, "test " << test << "\n", this->Quiet); // now remove the test itself - this->EraseTest(test); + this->ErasePendingTest(test); this->RunningCount += this->GetProcessorsUsed(test); auto testRun = cm::make_unique(*this, test); @@ -417,9 +417,9 @@ void cmCTestMultiProcessHandler::UnlockResources(int index) } } -void cmCTestMultiProcessHandler::EraseTest(int test) +void cmCTestMultiProcessHandler::ErasePendingTest(int test) { - this->Tests.erase(test); + this->PendingTests.erase(test); this->SortedTests.erase( std::find(this->SortedTests.begin(), this->SortedTests.end(), test)); } @@ -462,7 +462,7 @@ bool cmCTestMultiProcessHandler::StartTest(int test) } // if there are no depends left then run this test - if (this->Tests[test].empty()) { + if (this->PendingTests[test].Depends.empty()) { return this->StartTestProcess(test); } // This test was not able to start because it is waiting @@ -479,7 +479,7 @@ void cmCTestMultiProcessHandler::StartNextTests() uv_timer_stop(this->TestLoadRetryTimer); } - if (this->Tests.empty()) { + if (this->PendingTests.empty()) { this->TestLoadRetryTimer.reset(); return; } @@ -652,8 +652,8 @@ void cmCTestMultiProcessHandler::FinishTestProcess( this->Failed->push_back(properties->Name); } - for (auto& t : this->Tests) { - t.second.erase(test); + for (auto& t : this->PendingTests) { + t.second.Depends.erase(test); } this->WriteCheckpoint(test); @@ -808,7 +808,7 @@ void cmCTestMultiProcessHandler::CreateParallelTestCostList() // In parallel test runs add previously failed tests to the front // of the cost list and queue other tests for further sorting - for (auto const& t : this->Tests) { + for (auto const& t : this->PendingTests) { if (cm::contains(this->LastTestsFailed, this->Properties[t.first]->Name)) { // If the test failed last time, it should be run first. this->SortedTests.push_back(t.first); @@ -827,7 +827,7 @@ void cmCTestMultiProcessHandler::CreateParallelTestCostList() TestSet& currentSet = priorityStack.back(); for (auto const& i : previousSet) { - TestSet const& dependencies = this->Tests[i]; + TestSet const& dependencies = this->PendingTests[i].Depends; currentSet.insert(dependencies.begin(), dependencies.end()); } @@ -859,7 +859,7 @@ void cmCTestMultiProcessHandler::CreateParallelTestCostList() void cmCTestMultiProcessHandler::GetAllTestDependencies(int test, TestList& dependencies) { - TestSet const& dependencySet = this->Tests[test]; + TestSet const& dependencySet = this->PendingTests[test].Depends; for (int i : dependencySet) { this->GetAllTestDependencies(i, dependencies); dependencies.push_back(i); @@ -870,7 +870,7 @@ void cmCTestMultiProcessHandler::CreateSerialTestCostList() { TestList presortedList; - for (auto const& i : this->Tests) { + for (auto const& i : this->PendingTests) { presortedList.push_back(i.first); } @@ -1383,7 +1383,7 @@ void cmCTestMultiProcessHandler::CheckResume() void cmCTestMultiProcessHandler::RemoveTest(int index) { - this->EraseTest(index); + this->ErasePendingTest(index); this->Properties.erase(index); this->Completed++; } @@ -1391,7 +1391,7 @@ void cmCTestMultiProcessHandler::RemoveTest(int index) int cmCTestMultiProcessHandler::FindMaxIndex() { int max = 0; - for (auto const& i : this->Tests) { + for (auto const& i : this->PendingTests) { if (i.first > max) { max = i.first; } @@ -1405,7 +1405,7 @@ bool cmCTestMultiProcessHandler::CheckCycles() cmCTestOptionalLog(this->CTest, HANDLER_VERBOSE_OUTPUT, "Checking test dependency graph..." << std::endl, this->Quiet); - for (auto const& it : this->Tests) { + for (auto const& it : this->PendingTests) { // DFS from each element to itself int root = it.first; std::set visited; @@ -1415,7 +1415,7 @@ bool cmCTestMultiProcessHandler::CheckCycles() int test = s.top(); s.pop(); if (visited.insert(test).second) { - for (auto const& d : this->Tests[test]) { + for (auto const& d : this->PendingTests[test].Depends) { if (d == root) { // cycle exists cmCTestLog( diff --git a/Source/CTest/cmCTestMultiProcessHandler.h b/Source/CTest/cmCTestMultiProcessHandler.h index 4bf0844..0877de2 100644 --- a/Source/CTest/cmCTestMultiProcessHandler.h +++ b/Source/CTest/cmCTestMultiProcessHandler.h @@ -38,7 +38,11 @@ public: struct TestSet : public std::set { }; - struct TestMap : public std::map + struct TestInfo + { + TestSet Depends; + }; + struct TestMap : public std::map { }; struct TestList : public std::vector @@ -124,7 +128,7 @@ protected: // Removes the checkpoint file void MarkFinished(); - void EraseTest(int index); + void ErasePendingTest(int index); void FinishTestProcess(std::unique_ptr runner, bool started); static void OnTestLoadRetryCB(uv_timer_t* timer); @@ -171,8 +175,8 @@ protected: cm::optional ResourceSpecSetupTest; bool HasInvalidGeneratedResourceSpec; - // map from test number to set of depend tests - TestMap Tests; + // Tests pending selection to start. They may have dependencies. + TestMap PendingTests; TestList SortedTests; // Total number of tests we'll be running size_t Total; diff --git a/Source/CTest/cmCTestTestHandler.cxx b/Source/CTest/cmCTestTestHandler.cxx index 2fcef90..f8a8f9f 100644 --- a/Source/CTest/cmCTestTestHandler.cxx +++ b/Source/CTest/cmCTestTestHandler.cxx @@ -1381,7 +1381,7 @@ bool cmCTestTestHandler::ProcessDirectory(std::vector& passed, } } } - tests[p.Index] = depends; + tests[p.Index].Depends = depends; properties[p.Index] = &p; } parallel->SetResourceSpecFile(this->ResourceSpecFile); -- cgit v0.12 From 3bc3f6cbb57753926438c297d8838a5f9e589f23 Mon Sep 17 00:00:00 2001 From: Brad King Date: Thu, 2 Nov 2023 10:00:13 -0400 Subject: cmCTestMultiProcessHandler: Clarify representation of pending test order Also avoid copying the entire list each time we start a batch of tests. --- Source/CTest/cmCTestMultiProcessHandler.cxx | 43 ++++++++++++++++------------- Source/CTest/cmCTestMultiProcessHandler.h | 4 ++- 2 files changed, 27 insertions(+), 20 deletions(-) diff --git a/Source/CTest/cmCTestMultiProcessHandler.cxx b/Source/CTest/cmCTestMultiProcessHandler.cxx index 2777b2a..4034941 100644 --- a/Source/CTest/cmCTestMultiProcessHandler.cxx +++ b/Source/CTest/cmCTestMultiProcessHandler.cxx @@ -358,7 +358,7 @@ bool cmCTestMultiProcessHandler::AllResourcesAvailable() void cmCTestMultiProcessHandler::CheckResourcesAvailable() { if (this->UseResourceSpec) { - for (auto test : this->SortedTests) { + for (auto test : this->OrderedTests) { std::map> allocations; this->TryAllocateResources(test, allocations, @@ -420,8 +420,8 @@ void cmCTestMultiProcessHandler::UnlockResources(int index) void cmCTestMultiProcessHandler::ErasePendingTest(int test) { this->PendingTests.erase(test); - this->SortedTests.erase( - std::find(this->SortedTests.begin(), this->SortedTests.end(), test)); + this->OrderedTests.erase( + std::find(this->OrderedTests.begin(), this->OrderedTests.end(), test)); } inline size_t cmCTestMultiProcessHandler::GetProcessorsUsed(int test) @@ -540,8 +540,13 @@ void cmCTestMultiProcessHandler::StartNextTests() } } - TestList copy = this->SortedTests; - for (auto const& test : copy) { + // Start tests in the preferred order, each subject to readiness checks. + auto ti = this->OrderedTests.begin(); + while (ti != this->OrderedTests.end()) { + // Increment the test iterator now because the current list + // entry may be deleted below. + int test = *ti++; + // Take a nap if we're currently performing a RUN_SERIAL test. if (this->SerialTestRunning) { break; @@ -582,7 +587,7 @@ void cmCTestMultiProcessHandler::StartNextTests() // Find out whether there are any non RUN_SERIAL tests left, so that the // correct warning may be displayed. bool onlyRunSerialTestsLeft = true; - for (auto const& test : copy) { + for (auto const& test : this->OrderedTests) { if (!this->Properties[test]->RunSerial) { onlyRunSerialTestsLeft = false; } @@ -800,7 +805,7 @@ void cmCTestMultiProcessHandler::CreateTestCostList() void cmCTestMultiProcessHandler::CreateParallelTestCostList() { - TestSet alreadySortedTests; + TestSet alreadyOrderedTests; std::list priorityStack; priorityStack.emplace_back(); @@ -811,8 +816,8 @@ void cmCTestMultiProcessHandler::CreateParallelTestCostList() for (auto const& t : this->PendingTests) { if (cm::contains(this->LastTestsFailed, this->Properties[t.first]->Name)) { // If the test failed last time, it should be run first. - this->SortedTests.push_back(t.first); - alreadySortedTests.insert(t.first); + this->OrderedTests.push_back(t.first); + alreadyOrderedTests.insert(t.first); } else { topLevel.insert(t.first); } @@ -848,9 +853,9 @@ void cmCTestMultiProcessHandler::CreateParallelTestCostList() TestComparator(this)); for (auto const& j : sortedCopy) { - if (!cm::contains(alreadySortedTests, j)) { - this->SortedTests.push_back(j); - alreadySortedTests.insert(j); + if (!cm::contains(alreadyOrderedTests, j)) { + this->OrderedTests.push_back(j); + alreadyOrderedTests.insert(j); } } } @@ -877,10 +882,10 @@ void cmCTestMultiProcessHandler::CreateSerialTestCostList() std::stable_sort(presortedList.begin(), presortedList.end(), TestComparator(this)); - TestSet alreadySortedTests; + TestSet alreadyOrderedTests; for (int test : presortedList) { - if (cm::contains(alreadySortedTests, test)) { + if (cm::contains(alreadyOrderedTests, test)) { continue; } @@ -888,14 +893,14 @@ void cmCTestMultiProcessHandler::CreateSerialTestCostList() this->GetAllTestDependencies(test, dependencies); for (int testDependency : dependencies) { - if (!cm::contains(alreadySortedTests, testDependency)) { - alreadySortedTests.insert(testDependency); - this->SortedTests.push_back(testDependency); + if (!cm::contains(alreadyOrderedTests, testDependency)) { + alreadyOrderedTests.insert(testDependency); + this->OrderedTests.push_back(testDependency); } } - alreadySortedTests.insert(test); - this->SortedTests.push_back(test); + alreadyOrderedTests.insert(test); + this->OrderedTests.push_back(test); } } diff --git a/Source/CTest/cmCTestMultiProcessHandler.h b/Source/CTest/cmCTestMultiProcessHandler.h index 0877de2..f161793 100644 --- a/Source/CTest/cmCTestMultiProcessHandler.h +++ b/Source/CTest/cmCTestMultiProcessHandler.h @@ -5,6 +5,7 @@ #include "cmConfigure.h" // IWYU pragma: keep #include +#include #include #include #include @@ -177,7 +178,8 @@ protected: // Tests pending selection to start. They may have dependencies. TestMap PendingTests; - TestList SortedTests; + // List of pending test indexes, ordered by cost. + std::list OrderedTests; // Total number of tests we'll be running size_t Total; // Number of tests that are complete -- cgit v0.12 From afd185881f3b138389ab79024e397e52cfd945c3 Mon Sep 17 00:00:00 2001 From: Brad King Date: Thu, 2 Nov 2023 15:39:52 -0400 Subject: cmCTestMultiProcessHandler: Make loops over all pending tests more consistent Some loops were using the ordered list unnecessarily while others used the main map of pending test. Update all to use the latter. --- Source/CTest/cmCTestMultiProcessHandler.cxx | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/Source/CTest/cmCTestMultiProcessHandler.cxx b/Source/CTest/cmCTestMultiProcessHandler.cxx index 4034941..e6466ad 100644 --- a/Source/CTest/cmCTestMultiProcessHandler.cxx +++ b/Source/CTest/cmCTestMultiProcessHandler.cxx @@ -358,11 +358,11 @@ bool cmCTestMultiProcessHandler::AllResourcesAvailable() void cmCTestMultiProcessHandler::CheckResourcesAvailable() { if (this->UseResourceSpec) { - for (auto test : this->OrderedTests) { + for (auto const& t : this->PendingTests) { std::map> allocations; - this->TryAllocateResources(test, allocations, - &this->ResourceAllocationErrors[test]); + this->TryAllocateResources(t.first, allocations, + &this->ResourceAllocationErrors[t.first]); } } } @@ -587,8 +587,8 @@ void cmCTestMultiProcessHandler::StartNextTests() // Find out whether there are any non RUN_SERIAL tests left, so that the // correct warning may be displayed. bool onlyRunSerialTestsLeft = true; - for (auto const& test : this->OrderedTests) { - if (!this->Properties[test]->RunSerial) { + for (auto const& t : this->PendingTests) { + if (!this->Properties[t.first]->RunSerial) { onlyRunSerialTestsLeft = false; } } -- cgit v0.12 From ee321dc85fcc99389c372f7ba1453edf27b27663 Mon Sep 17 00:00:00 2001 From: Brad King Date: Fri, 20 Oct 2023 15:17:39 -0400 Subject: cmCTestMultiProcessHandler: Clarify search for tests <= spare load Move code into conditions where it is needed and comment its purpose. --- Source/CTest/cmCTestMultiProcessHandler.cxx | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/Source/CTest/cmCTestMultiProcessHandler.cxx b/Source/CTest/cmCTestMultiProcessHandler.cxx index e6466ad..02f1d83 100644 --- a/Source/CTest/cmCTestMultiProcessHandler.cxx +++ b/Source/CTest/cmCTestMultiProcessHandler.cxx @@ -558,25 +558,26 @@ void cmCTestMultiProcessHandler::StartNextTests() } size_t processors = this->GetProcessorsUsed(test); - bool testLoadOk = true; if (this->TestLoad > 0) { + // Exclude tests that are too big to fit in the spare load. if (processors <= spareLoad) { + // We found a test that fits in the spare load. cmCTestLog(this->CTest, DEBUG, "OK to run " << this->GetName(test) << ", it requires " << processors << " procs & system load is: " << systemLoad << std::endl); allTestsFailedTestLoadCheck = false; } else { - testLoadOk = false; + // Keep track of the smallest excluded test to report in message below. + if (processors <= minProcessorsRequired) { + minProcessorsRequired = processors; + testWithMinProcessors = this->GetName(test); + } + continue; } } - if (processors <= minProcessorsRequired) { - minProcessorsRequired = processors; - testWithMinProcessors = this->GetName(test); - } - - if (testLoadOk && processors <= numToStart && this->StartTest(test)) { + if (processors <= numToStart && this->StartTest(test)) { numToStart -= processors; } else if (numToStart == 0) { break; -- cgit v0.12 From 9b548139fd63c22325330b58a00c4d610a500447 Mon Sep 17 00:00:00 2001 From: Brad King Date: Fri, 20 Oct 2023 15:24:18 -0400 Subject: cmCTestMultiProcessHandler: Clarify search for tests <= concurrency limit --- Source/CTest/cmCTestMultiProcessHandler.cxx | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/Source/CTest/cmCTestMultiProcessHandler.cxx b/Source/CTest/cmCTestMultiProcessHandler.cxx index 02f1d83..0691008 100644 --- a/Source/CTest/cmCTestMultiProcessHandler.cxx +++ b/Source/CTest/cmCTestMultiProcessHandler.cxx @@ -577,7 +577,12 @@ void cmCTestMultiProcessHandler::StartNextTests() } } - if (processors <= numToStart && this->StartTest(test)) { + // Exclude tests that are too big to fit in the concurrency limit. + if (processors > numToStart) { + continue; + } + + if (this->StartTest(test)) { numToStart -= processors; } else if (numToStart == 0) { break; -- cgit v0.12 From bd0b4ca867c297a42b3128e0193c6ab01f1b53ec Mon Sep 17 00:00:00 2001 From: Brad King Date: Fri, 20 Oct 2023 15:20:41 -0400 Subject: cmCTestMultiProcessHandler: Invert spare load condition Switch then/else blocks to reduce indentation. --- Source/CTest/cmCTestMultiProcessHandler.cxx | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/Source/CTest/cmCTestMultiProcessHandler.cxx b/Source/CTest/cmCTestMultiProcessHandler.cxx index 0691008..d4cda42 100644 --- a/Source/CTest/cmCTestMultiProcessHandler.cxx +++ b/Source/CTest/cmCTestMultiProcessHandler.cxx @@ -560,14 +560,7 @@ void cmCTestMultiProcessHandler::StartNextTests() size_t processors = this->GetProcessorsUsed(test); if (this->TestLoad > 0) { // Exclude tests that are too big to fit in the spare load. - if (processors <= spareLoad) { - // We found a test that fits in the spare load. - cmCTestLog(this->CTest, DEBUG, - "OK to run " << this->GetName(test) << ", it requires " - << processors << " procs & system load is: " - << systemLoad << std::endl); - allTestsFailedTestLoadCheck = false; - } else { + if (processors > spareLoad) { // Keep track of the smallest excluded test to report in message below. if (processors <= minProcessorsRequired) { minProcessorsRequired = processors; @@ -575,6 +568,13 @@ void cmCTestMultiProcessHandler::StartNextTests() } continue; } + + // We found a test that fits in the spare load. + allTestsFailedTestLoadCheck = false; + cmCTestLog(this->CTest, DEBUG, + "OK to run " + << this->GetName(test) << ", it requires " << processors + << " procs & system load is: " << systemLoad << std::endl); } // Exclude tests that are too big to fit in the concurrency limit. -- cgit v0.12 From 8f1e8af0ccec06b69f25d01d1f27d967a3501f14 Mon Sep 17 00:00:00 2001 From: Brad King Date: Fri, 20 Oct 2023 15:26:10 -0400 Subject: cmCTestMultiProcessHandler: Stop searching for tests when limit is reached Avoid an extra loop iteration if we have no room for more tests. --- Source/CTest/cmCTestMultiProcessHandler.cxx | 12 ++++++++---- Tests/RunCMake/CTestCommandLine/RunCMakeTest.cmake | 1 + .../CTestCommandLine/test-load-wait0-stdout.txt | 21 +++++++++++++++++++++ .../ctest_test/CTestTestLoadWait0-stdout.txt | 15 +++++++++++++++ Tests/RunCMake/ctest_test/RunCMakeTest.cmake | 3 +++ Tests/RunCMake/ctest_test/TestLoadWait0-stdout.txt | 15 +++++++++++++++ 6 files changed, 63 insertions(+), 4 deletions(-) create mode 100644 Tests/RunCMake/CTestCommandLine/test-load-wait0-stdout.txt create mode 100644 Tests/RunCMake/ctest_test/CTestTestLoadWait0-stdout.txt create mode 100644 Tests/RunCMake/ctest_test/TestLoadWait0-stdout.txt diff --git a/Source/CTest/cmCTestMultiProcessHandler.cxx b/Source/CTest/cmCTestMultiProcessHandler.cxx index d4cda42..31b7422 100644 --- a/Source/CTest/cmCTestMultiProcessHandler.cxx +++ b/Source/CTest/cmCTestMultiProcessHandler.cxx @@ -542,7 +542,7 @@ void cmCTestMultiProcessHandler::StartNextTests() // Start tests in the preferred order, each subject to readiness checks. auto ti = this->OrderedTests.begin(); - while (ti != this->OrderedTests.end()) { + while (numToStart > 0 && ti != this->OrderedTests.end()) { // Increment the test iterator now because the current list // entry may be deleted below. int test = *ti++; @@ -584,8 +584,6 @@ void cmCTestMultiProcessHandler::StartNextTests() if (this->StartTest(test)) { numToStart -= processors; - } else if (numToStart == 0) { - break; } } @@ -606,7 +604,7 @@ void cmCTestMultiProcessHandler::StartNextTests() } else if (onlyRunSerialTestsLeft) { cmCTestLog(this->CTest, HANDLER_VERBOSE_OUTPUT, "Only RUN_SERIAL tests remain, awaiting available slot."); - } else { + } else if (!testWithMinProcessors.empty()) { /* clang-format off */ cmCTestLog(this->CTest, HANDLER_VERBOSE_OUTPUT, "System Load: " << systemLoad << ", " @@ -614,6 +612,12 @@ void cmCTestMultiProcessHandler::StartNextTests() "Smallest test " << testWithMinProcessors << " requires " << minProcessorsRequired); /* clang-format on */ + } else { + /* clang-format off */ + cmCTestLog(this->CTest, HANDLER_VERBOSE_OUTPUT, + "System Load: " << systemLoad << ", " + "Max Allowed Load: " << this->TestLoad); + /* clang-format on */ } cmCTestLog(this->CTest, HANDLER_VERBOSE_OUTPUT, "*****" << std::endl); diff --git a/Tests/RunCMake/CTestCommandLine/RunCMakeTest.cmake b/Tests/RunCMake/CTestCommandLine/RunCMakeTest.cmake index 8a9cc8c..1b8d0d9 100644 --- a/Tests/RunCMake/CTestCommandLine/RunCMakeTest.cmake +++ b/Tests/RunCMake/CTestCommandLine/RunCMakeTest.cmake @@ -243,6 +243,7 @@ set(ENV{__CTEST_FAKE_LOAD_AVERAGE_FOR_TESTING} 7) # Verify that new tests are not started when the load average exceeds # our threshold and that they then run once the load average drops. +run_TestLoad(test-load-wait0 5) run_TestLoad(test-load-wait1 8) # Verify that warning message is displayed but tests still start when diff --git a/Tests/RunCMake/CTestCommandLine/test-load-wait0-stdout.txt b/Tests/RunCMake/CTestCommandLine/test-load-wait0-stdout.txt new file mode 100644 index 0000000..d112fde --- /dev/null +++ b/Tests/RunCMake/CTestCommandLine/test-load-wait0-stdout.txt @@ -0,0 +1,21 @@ +Test project [^ +]*/Tests/RunCMake/CTestCommandLine/TestLoad( +[^*][^ +]*)* +\*\*\*\*\* WAITING, System Load: 7, Max Allowed Load: 5\*\*\*\*\* +test 1 + Start 1: TestLoad1 ++( +[^*][^ +]*)* +test 2 + Start 2: TestLoad2 ++( +[^*][^ +]*)* +1/2 Test #[1-2]: TestLoad[1-2] ........................ Passed +[0-9.]+ sec( +[^*][^ +]*)* +2/2 Test #[1-2]: TestLoad[1-2] ........................ Passed +[0-9.]+ sec ++ +100% tests passed, 0 tests failed out of 2 diff --git a/Tests/RunCMake/ctest_test/CTestTestLoadWait0-stdout.txt b/Tests/RunCMake/ctest_test/CTestTestLoadWait0-stdout.txt new file mode 100644 index 0000000..60d70c9 --- /dev/null +++ b/Tests/RunCMake/ctest_test/CTestTestLoadWait0-stdout.txt @@ -0,0 +1,15 @@ +Test project [^ +]*/Tests/RunCMake/ctest_test/CTestTestLoadWait0-build( +[^*][^ +]*)* +\*\*\*\*\* WAITING, System Load: 7, Max Allowed Load: 6\*\*\*\*\* +test 1 + Start 1: RunCMakeVersion ++( +[^*][^ +]*)* +1/1 Test #1: RunCMakeVersion .................. Passed +[0-9.]+ sec ++ +100% tests passed, 0 tests failed out of 1 ++ +Total Test time \(real\) = +[0-9.]+ sec$ diff --git a/Tests/RunCMake/ctest_test/RunCMakeTest.cmake b/Tests/RunCMake/ctest_test/RunCMakeTest.cmake index 9570c77..b1ec9ad 100644 --- a/Tests/RunCMake/ctest_test/RunCMakeTest.cmake +++ b/Tests/RunCMake/ctest_test/RunCMakeTest.cmake @@ -26,6 +26,7 @@ run_ctest_test(TestLoadPass TEST_LOAD 8) # Verify that new tests are not started when the load average exceeds # our threshold and that they then run once the load average drops. +run_ctest_test(TestLoadWait0 TEST_LOAD 4 PARALLEL_LEVEL 8) run_ctest_test(TestLoadWait1 TEST_LOAD 8 PARALLEL_LEVEL 8) # Verify that when an invalid "TEST_LOAD" value is given, a warning @@ -39,6 +40,8 @@ run_ctest_test(CTestTestLoadPass) # Verify that new tests are not started when the load average exceeds # our threshold and that they then run once the load average drops. +set(CASE_CTEST_TEST_LOAD 6) +run_ctest_test(CTestTestLoadWait0 PARALLEL_LEVEL 8) set(CASE_CTEST_TEST_LOAD 8) run_ctest_test(CTestTestLoadWait1 PARALLEL_LEVEL 8) diff --git a/Tests/RunCMake/ctest_test/TestLoadWait0-stdout.txt b/Tests/RunCMake/ctest_test/TestLoadWait0-stdout.txt new file mode 100644 index 0000000..c7172aa --- /dev/null +++ b/Tests/RunCMake/ctest_test/TestLoadWait0-stdout.txt @@ -0,0 +1,15 @@ +Test project [^ +]*/Tests/RunCMake/ctest_test/TestLoadWait0-build( +[^*][^ +]*)* +\*\*\*\*\* WAITING, System Load: 7, Max Allowed Load: 4\*\*\*\*\* +test 1 + Start 1: RunCMakeVersion ++( +[^*][^ +]*)* +1/1 Test #1: RunCMakeVersion .................. Passed +[0-9.]+ sec ++ +100% tests passed, 0 tests failed out of 1 ++ +Total Test time \(real\) = +[0-9.]+ sec$ -- cgit v0.12 From b02b628ad9b7c59f703c42fdc238c9f11d277937 Mon Sep 17 00:00:00 2001 From: Brad King Date: Thu, 2 Nov 2023 14:26:54 -0400 Subject: cmCTestMultiProcessHandler: Simplify loop termination on serial test --- Source/CTest/cmCTestMultiProcessHandler.cxx | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/Source/CTest/cmCTestMultiProcessHandler.cxx b/Source/CTest/cmCTestMultiProcessHandler.cxx index 31b7422..61582d1 100644 --- a/Source/CTest/cmCTestMultiProcessHandler.cxx +++ b/Source/CTest/cmCTestMultiProcessHandler.cxx @@ -542,15 +542,12 @@ void cmCTestMultiProcessHandler::StartNextTests() // Start tests in the preferred order, each subject to readiness checks. auto ti = this->OrderedTests.begin(); - while (numToStart > 0 && ti != this->OrderedTests.end()) { + while (numToStart > 0 && !this->SerialTestRunning && + ti != this->OrderedTests.end()) { // Increment the test iterator now because the current list // entry may be deleted below. int test = *ti++; - // Take a nap if we're currently performing a RUN_SERIAL test. - if (this->SerialTestRunning) { - break; - } // We can only start a RUN_SERIAL test if no other tests are also // running. if (this->Properties[test]->RunSerial && this->RunningCount > 0) { -- cgit v0.12 From 1487e540aa2bc069a7d156a3ad884e98edfb8815 Mon Sep 17 00:00:00 2001 From: Brad King Date: Wed, 15 Nov 2023 14:58:52 -0500 Subject: cmCTestMultiProcessHandler: Reduce repeat test property map lookups --- Source/CTest/cmCTestMultiProcessHandler.cxx | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/Source/CTest/cmCTestMultiProcessHandler.cxx b/Source/CTest/cmCTestMultiProcessHandler.cxx index 61582d1..bee8735 100644 --- a/Source/CTest/cmCTestMultiProcessHandler.cxx +++ b/Source/CTest/cmCTestMultiProcessHandler.cxx @@ -398,21 +398,21 @@ void cmCTestMultiProcessHandler::SetStopTimePassed() void cmCTestMultiProcessHandler::LockResources(int index) { - this->LockedResources.insert( - this->Properties[index]->LockedResources.begin(), - this->Properties[index]->LockedResources.end()); - - if (this->Properties[index]->RunSerial) { + auto* properties = this->Properties[index]; + this->LockedResources.insert(properties->LockedResources.begin(), + properties->LockedResources.end()); + if (properties->RunSerial) { this->SerialTestRunning = true; } } void cmCTestMultiProcessHandler::UnlockResources(int index) { - for (std::string const& i : this->Properties[index]->LockedResources) { + auto* properties = this->Properties[index]; + for (std::string const& i : properties->LockedResources) { this->LockedResources.erase(i); } - if (this->Properties[index]->RunSerial) { + if (properties->RunSerial) { this->SerialTestRunning = false; } } -- cgit v0.12 From a4b061a035c782974642bf2ca2d8d5e1a2f94553 Mon Sep 17 00:00:00 2001 From: Brad King Date: Wed, 15 Nov 2023 16:01:09 -0500 Subject: cmCTestMultiProcessHandler: Clarify resource availability error member names The members are about the availability of sufficient resources, not allocation of them. --- Source/CTest/cmCTestMultiProcessHandler.cxx | 20 ++++++++++---------- Source/CTest/cmCTestMultiProcessHandler.h | 10 +++++----- Source/CTest/cmCTestRunTest.cxx | 2 +- Source/CTest/cmCTestTestHandler.cxx | 2 +- 4 files changed, 17 insertions(+), 17 deletions(-) diff --git a/Source/CTest/cmCTestMultiProcessHandler.cxx b/Source/CTest/cmCTestMultiProcessHandler.cxx index bee8735..67bbb4a 100644 --- a/Source/CTest/cmCTestMultiProcessHandler.cxx +++ b/Source/CTest/cmCTestMultiProcessHandler.cxx @@ -196,18 +196,18 @@ bool cmCTestMultiProcessHandler::StartTestProcess(int test) // working directory because FinishTestProcess() will try to unlock them this->LockResources(test); - if (!this->ResourceAllocationErrors[test].empty()) { + if (!this->ResourceAvailabilityErrors[test].empty()) { std::ostringstream e; e << "Insufficient resources for test " << this->Properties[test]->Name << ":\n\n"; - for (auto const& it : this->ResourceAllocationErrors[test]) { + for (auto const& it : this->ResourceAvailabilityErrors[test]) { switch (it.second) { - case ResourceAllocationError::NoResourceType: + case ResourceAvailabilityError::NoResourceType: e << " Test requested resources of type '" << it.first << "' which does not exist\n"; break; - case ResourceAllocationError::InsufficientResources: + case ResourceAvailabilityError::InsufficientResources: e << " Test requested resources of type '" << it.first << "' in the following amounts:\n"; for (auto const& group : this->Properties[test]->ResourceGroups) { @@ -280,7 +280,7 @@ bool cmCTestMultiProcessHandler::AllocateResources(int index) bool cmCTestMultiProcessHandler::TryAllocateResources( int index, std::map>& allocations, - std::map* errors) + std::map* errors) { allocations.clear(); @@ -300,7 +300,7 @@ bool cmCTestMultiProcessHandler::TryAllocateResources( for (auto& it : allocations) { if (!availableResources.count(it.first)) { if (errors) { - (*errors)[it.first] = ResourceAllocationError::NoResourceType; + (*errors)[it.first] = ResourceAvailabilityError::NoResourceType; result = false; } else { return false; @@ -308,7 +308,7 @@ bool cmCTestMultiProcessHandler::TryAllocateResources( } else if (!cmAllocateCTestResourcesRoundRobin( availableResources.at(it.first), it.second)) { if (errors) { - (*errors)[it.first] = ResourceAllocationError::InsufficientResources; + (*errors)[it.first] = ResourceAvailabilityError::InsufficientResources; result = false; } else { return false; @@ -355,14 +355,14 @@ bool cmCTestMultiProcessHandler::AllResourcesAvailable() return true; } -void cmCTestMultiProcessHandler::CheckResourcesAvailable() +void cmCTestMultiProcessHandler::CheckResourceAvailability() { if (this->UseResourceSpec) { for (auto const& t : this->PendingTests) { std::map> allocations; this->TryAllocateResources(t.first, allocations, - &this->ResourceAllocationErrors[t.first]); + &this->ResourceAvailabilityErrors[t.first]); } } } @@ -455,7 +455,7 @@ bool cmCTestMultiProcessHandler::StartTest(int test) } // Allocate resources - if (this->ResourceAllocationErrors[test].empty() && + if (this->ResourceAvailabilityErrors[test].empty() && !this->AllocateResources(test)) { this->DeallocateResources(test); return false; diff --git a/Source/CTest/cmCTestMultiProcessHandler.h b/Source/CTest/cmCTestMultiProcessHandler.h index f161793..c01089c 100644 --- a/Source/CTest/cmCTestMultiProcessHandler.h +++ b/Source/CTest/cmCTestMultiProcessHandler.h @@ -104,7 +104,7 @@ public: void SetQuiet(bool b) { this->Quiet = b; } - void CheckResourcesAvailable(); + void CheckResourceAvailability(); protected: // Start the next test or tests as many as are allowed by @@ -151,7 +151,7 @@ protected: void LockResources(int index); void UnlockResources(int index); - enum class ResourceAllocationError + enum class ResourceAvailabilityError { NoResourceType, InsufficientResources, @@ -163,7 +163,7 @@ protected: int index, std::map>& allocations, - std::map* errors = nullptr); + std::map* errors = nullptr); void DeallocateResources(int index); bool AllResourcesAvailable(); bool InitResourceAllocator(std::string& error); @@ -198,8 +198,8 @@ protected: std::map>>> AllocatedResources; - std::map> - ResourceAllocationErrors; + std::map> + ResourceAvailabilityErrors; cmCTestResourceAllocator ResourceAllocator; std::vector* TestResults; size_t ParallelLevel; // max number of process that can be run at once diff --git a/Source/CTest/cmCTestRunTest.cxx b/Source/CTest/cmCTestRunTest.cxx index 4b97365..e96dc1f 100644 --- a/Source/CTest/cmCTestRunTest.cxx +++ b/Source/CTest/cmCTestRunTest.cxx @@ -165,7 +165,7 @@ cmCTestRunTest::EndTestResult cmCTestRunTest::EndTest(size_t completed, reason = "Invalid resource spec file"; forceFail = true; } else { - this->MultiTestHandler.CheckResourcesAvailable(); + this->MultiTestHandler.CheckResourceAvailability(); } } std::ostringstream outputStream; diff --git a/Source/CTest/cmCTestTestHandler.cxx b/Source/CTest/cmCTestTestHandler.cxx index f8a8f9f..ba15366 100644 --- a/Source/CTest/cmCTestTestHandler.cxx +++ b/Source/CTest/cmCTestTestHandler.cxx @@ -1389,7 +1389,7 @@ bool cmCTestTestHandler::ProcessDirectory(std::vector& passed, parallel->SetPassFailVectors(&passed, &failed); this->TestResults.clear(); parallel->SetTestResults(&this->TestResults); - parallel->CheckResourcesAvailable(); + parallel->CheckResourceAvailability(); if (this->CTest->ShouldPrintLabels()) { parallel->PrintLabels(); -- cgit v0.12 From 5d2e93f9e8e775c210285e63deb5377f2114b851 Mon Sep 17 00:00:00 2001 From: Brad King Date: Wed, 15 Nov 2023 15:36:46 -0500 Subject: cmCTestMultiProcessHandler: Simplify logic on unavailable resources Deallocation is not necessary after allocation fails. --- Source/CTest/cmCTestMultiProcessHandler.cxx | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/Source/CTest/cmCTestMultiProcessHandler.cxx b/Source/CTest/cmCTestMultiProcessHandler.cxx index 67bbb4a..b91f07c 100644 --- a/Source/CTest/cmCTestMultiProcessHandler.cxx +++ b/Source/CTest/cmCTestMultiProcessHandler.cxx @@ -256,6 +256,12 @@ bool cmCTestMultiProcessHandler::AllocateResources(int index) return true; } + // If the test needs unavailable resources then do not allocate anything + // because it will never run. We will issue the recorded errors instead. + if (!this->ResourceAvailabilityErrors[index].empty()) { + return true; + } + std::map> allocations; if (!this->TryAllocateResources(index, allocations)) { return false; @@ -454,10 +460,7 @@ bool cmCTestMultiProcessHandler::StartTest(int test) } } - // Allocate resources - if (this->ResourceAvailabilityErrors[test].empty() && - !this->AllocateResources(test)) { - this->DeallocateResources(test); + if (!this->AllocateResources(test)) { return false; } -- cgit v0.12