From 39e5f9d963c6580f95316f9b5f76a3a8f660d6cc Mon Sep 17 00:00:00 2001 From: Zach Mullen Date: Tue, 8 Sep 2009 13:39:13 -0400 Subject: [PATCH] ENH: Replaced the EXPENSIVE test property with a COST test property taking a floating point value. Tests are now started in descending order of their cost, which defaults to 0 if none is specified. --- Source/CTest/cmCTestMultiProcessHandler.cxx | 109 +++++++++++--------- Source/CTest/cmCTestMultiProcessHandler.h | 10 +- Source/CTest/cmCTestTestHandler.cxx | 15 ++- Source/CTest/cmCTestTestHandler.h | 2 +- Source/cmSetTestsPropertiesCommand.h | 5 +- 5 files changed, 74 insertions(+), 67 deletions(-) diff --git a/Source/CTest/cmCTestMultiProcessHandler.cxx b/Source/CTest/cmCTestMultiProcessHandler.cxx index fadf164d5..41b764d58 100644 --- a/Source/CTest/cmCTestMultiProcessHandler.cxx +++ b/Source/CTest/cmCTestMultiProcessHandler.cxx @@ -30,25 +30,20 @@ cmCTestMultiProcessHandler::cmCTestMultiProcessHandler() // Set the tests void cmCTestMultiProcessHandler::SetTests(TestMap& tests, - TestMap& expensiveTests, + TestCostMap& testCosts, PropertiesMap& properties) { + this->Tests = tests; + this->TestCosts = testCosts; + this->Properties = properties; + this->Total = this->Tests.size(); // set test run map to false for all for(TestMap::iterator i = this->Tests.begin(); i != this->Tests.end(); ++i) { this->TestRunningMap[i->first] = false; this->TestFinishMap[i->first] = false; - - if(this->Properties[i->first]->Expensive) - { - this->ExpensiveTests[i->first] = i->second; - } } - this->Tests = tests; - this->ExpensiveTests = expensiveTests; - this->Properties = properties; - this->Total = this->Tests.size(); } // Set the max number of tests that can be run at the same time. @@ -57,6 +52,7 @@ void cmCTestMultiProcessHandler::SetParallelLevel(size_t level) this->ParallelLevel = level < 1 ? 1 : level; } +//--------------------------------------------------------- void cmCTestMultiProcessHandler::RunTests() { if(this->CTest->GetBatchJobs()) @@ -67,7 +63,7 @@ void cmCTestMultiProcessHandler::RunTests() this->CheckResume(); this->TestHandler->SetMaxIndex(this->FindMaxIndex()); this->StartNextTests(); - while(this->Tests.size() != 0 || this->ExpensiveTests.size() != 0) + while(this->Tests.size() != 0) { this->CheckOutput(); this->StartNextTests(); @@ -79,6 +75,7 @@ void cmCTestMultiProcessHandler::RunTests() this->MarkFinished(); } +//--------------------------------------------------------- void cmCTestMultiProcessHandler::SubmitBatchTests() { for(cmCTest::CTestConfigurationMap::iterator i = @@ -90,17 +87,14 @@ void cmCTestMultiProcessHandler::SubmitBatchTests() } } +//--------------------------------------------------------- void cmCTestMultiProcessHandler::StartTestProcess(int test) { - cmCTestLog(this->CTest, HANDLER_VERBOSE_OUTPUT, test << ": " - << " test " << test << "\n"); + cmCTestLog(this->CTest, HANDLER_VERBOSE_OUTPUT, " test " << test << "\n"); this->TestRunningMap[test] = true; // mark the test as running // now remove the test itself - if(this->ExpensiveTests.size() > 0) - { - this->ExpensiveTests.erase(test); - } - this->Tests.erase(test); + this->EraseTest(test); + cmCTestRunTest* testRun = new cmCTestRunTest; testRun->SetCTest(this->CTest); testRun->SetTestHandler(this->TestHandler); @@ -118,6 +112,22 @@ void cmCTestMultiProcessHandler::StartTestProcess(int test) } } +//--------------------------------------------------------- +void cmCTestMultiProcessHandler::EraseTest(int test) +{ + this->Tests.erase(test); + for(TestCostMap::iterator i = this->TestCosts.begin(); + i != this->TestCosts.end(); ++i) + { + if(i->second.find(test) != i->second.end()) + { + i->second.erase(test); + return; + } + } +} + +//--------------------------------------------------------- inline size_t cmCTestMultiProcessHandler::GetProcessorsUsed(int test) { size_t processors = @@ -130,10 +140,10 @@ inline size_t cmCTestMultiProcessHandler::GetProcessorsUsed(int test) { processors = this->ParallelLevel; } - return processors; } +//--------------------------------------------------------- bool cmCTestMultiProcessHandler::StartTest(int test) { // copy the depend tests locally because when @@ -176,6 +186,7 @@ bool cmCTestMultiProcessHandler::StartTest(int test) return false; } +//--------------------------------------------------------- void cmCTestMultiProcessHandler::StartNextTests() { size_t numToStart = this->ParallelLevel - this->RunningCount; @@ -183,36 +194,39 @@ void cmCTestMultiProcessHandler::StartNextTests() { return; } - TestMap tests = this->ExpensiveTests.size() > 0 ? - this->ExpensiveTests : this->Tests; - for(TestMap::iterator i = tests.begin(); - i != tests.end(); ++i) + for(TestCostMap::reverse_iterator i = this->TestCosts.rbegin(); + i != this->TestCosts.rend(); ++i) { - size_t processors = GetProcessorsUsed(i->first); - if(processors > numToStart) + TestSet tests = i->second; //copy the test set + for(TestSet::iterator test = tests.begin(); + test != tests.end(); ++test) { - return; - } - // start test should start only one test - if(this->StartTest(i->first)) - { - numToStart -= processors; - this->RunningCount += processors; - } - else - { - cmCTestLog(this->CTest, HANDLER_VERBOSE_OUTPUT, std::endl - << "Test did not start waiting on depends to finish: " - << i->first << "\n"); - } - if(numToStart == 0 ) - { - return; + size_t processors = GetProcessorsUsed(*test); + if(processors > numToStart) + { + return; + } + if(this->StartTest(*test)) + { + numToStart -= processors; + this->RunningCount += processors; + } + else + { + cmCTestLog(this->CTest, HANDLER_VERBOSE_OUTPUT, std::endl + << "Test did not start waiting on depends to finish: " + << *test << "\n"); + } + if(numToStart == 0) + { + return; + } } } } +//--------------------------------------------------------- bool cmCTestMultiProcessHandler::CheckOutput() { // no more output we are done @@ -248,11 +262,6 @@ bool cmCTestMultiProcessHandler::CheckOutput() { this->Failed->push_back(p->GetTestProperties()->Name); } - for(TestMap::iterator j = this->ExpensiveTests.begin(); - j != this->ExpensiveTests.end(); ++j) - { - j->second.erase(test); - } for(TestMap::iterator j = this->Tests.begin(); j != this->Tests.end(); ++j) { @@ -268,6 +277,7 @@ bool cmCTestMultiProcessHandler::CheckOutput() return true; } +//--------------------------------------------------------- void cmCTestMultiProcessHandler::WriteCheckpoint(int index) { std::string fname = this->CTest->GetBinaryDir() @@ -354,16 +364,17 @@ void cmCTestMultiProcessHandler::CheckResume() } } +//--------------------------------------------------------- void cmCTestMultiProcessHandler::RemoveTest(int index) { - this->Tests.erase(index); + this->EraseTest(index); this->Properties.erase(index); - this->ExpensiveTests.erase(index); this->TestRunningMap[index] = false; this->TestFinishMap[index] = true; this->Completed++; } +//--------------------------------------------------------- int cmCTestMultiProcessHandler::FindMaxIndex() { int max = 0; diff --git a/Source/CTest/cmCTestMultiProcessHandler.h b/Source/CTest/cmCTestMultiProcessHandler.h index 5d0ae46e8..97af2f3ec 100644 --- a/Source/CTest/cmCTestMultiProcessHandler.h +++ b/Source/CTest/cmCTestMultiProcessHandler.h @@ -31,12 +31,13 @@ class cmCTestMultiProcessHandler public: struct TestSet : public std::set {}; struct TestMap : public std::map {}; + struct TestCostMap : public std::map {}; struct PropertiesMap : public std::map {}; cmCTestMultiProcessHandler(); // Set the tests - void SetTests(TestMap& tests, TestMap& expensiveTests, + void SetTests(TestMap& tests, TestCostMap& testCosts, PropertiesMap& properties); // Set the max number of tests that can be run at the same time. void SetParallelLevel(size_t); @@ -51,9 +52,7 @@ public: this->Failed = failed; } void SetTestResults(std::vector* r) - { - this->TestResults = r; - } + { this->TestResults = r; } void SetCTest(cmCTest* ctest) { this->CTest = ctest;} @@ -73,6 +72,7 @@ protected: void WriteCheckpoint(int index); // Removes the checkpoint file void MarkFinished(); + void EraseTest(int index); // Return true if there are still tests running // check all running processes for output and exit case bool CheckOutput(); @@ -83,7 +83,7 @@ protected: inline size_t GetProcessorsUsed(int index); // map from test number to set of depend tests TestMap Tests; - TestMap ExpensiveTests; + TestCostMap TestCosts; //Total number of tests we'll be running size_t Total; //Number of tests that are complete diff --git a/Source/CTest/cmCTestTestHandler.cxx b/Source/CTest/cmCTestTestHandler.cxx index 6ae227b5d..3cf2d124d 100644 --- a/Source/CTest/cmCTestTestHandler.cxx +++ b/Source/CTest/cmCTestTestHandler.cxx @@ -1010,7 +1010,7 @@ void cmCTestTestHandler::ProcessDirectory(std::vector &passed, << std::endl; cmCTestMultiProcessHandler::TestMap tests; - cmCTestMultiProcessHandler::TestMap expensiveTests; + cmCTestMultiProcessHandler::TestCostMap testCosts; cmCTestMultiProcessHandler::PropertiesMap properties; for (ListOfTests::iterator it = this->TestList.begin(); @@ -1037,12 +1037,9 @@ void cmCTestTestHandler::ProcessDirectory(std::vector &passed, } tests[it->Index] = depends; properties[it->Index] = &*it; - if(it->Expensive) - { - expensiveTests[it->Index] = depends; - } + testCosts[p.Cost].insert(p.Index); } - parallel.SetTests(tests, expensiveTests, properties); + parallel.SetTests(tests, testCosts, properties); parallel.SetPassFailVectors(&passed, &failed); this->TestResults.clear(); parallel.SetTestResults(&this->TestResults); @@ -1975,9 +1972,9 @@ bool cmCTestTestHandler::SetTestsProperties( { rtit->Timeout = atof(val.c_str()); } - if ( key == "EXPENSIVE" ) + if ( key == "COST" ) { - rtit->Expensive = cmSystemTools::IsOn(val.c_str()); + rtit->Cost = atof(val.c_str()); } if ( key == "RUN_SERIAL" ) { @@ -2130,9 +2127,9 @@ bool cmCTestTestHandler::AddTest(const std::vector& args) test.IsInBasedOnREOptions = true; test.WillFail = false; - test.Expensive = false; test.RunSerial = false; test.Timeout = 0; + test.Cost = 0; test.Processors = 1; if (this->UseIncludeRegExpFlag && !this->IncludeTestsRegularExpression.find(testname.c_str())) diff --git a/Source/CTest/cmCTestTestHandler.h b/Source/CTest/cmCTestTestHandler.h index 113c1c9c8..ac46e57cb 100644 --- a/Source/CTest/cmCTestTestHandler.h +++ b/Source/CTest/cmCTestTestHandler.h @@ -96,7 +96,7 @@ public: std::map Measurements; bool IsInBasedOnREOptions; bool WillFail; - bool Expensive; + float Cost; bool RunSerial; double Timeout; int Index; diff --git a/Source/cmSetTestsPropertiesCommand.h b/Source/cmSetTestsPropertiesCommand.h index c975011eb..89f03e97f 100644 --- a/Source/cmSetTestsPropertiesCommand.h +++ b/Source/cmSetTestsPropertiesCommand.h @@ -72,9 +72,8 @@ public: "PROCESSORS: Denotes the number of processors that this test will " "require. This is typically used for MPI tests, and should be used in " "conjunction with the ctest_test PARALLEL_LEVEL option.\n" - "EXPENSIVE: If set to true, this test will be run before tests that " - "are not marked as expensive. This should be used in conjunction with " - "the ctest_test PARALLEL_LEVEL option.\n" + "COST: Set this to a floating point value. Tests in a test set will be " + "run in descending order of cost.\n" "RUN_SERIAL: If set to true, this test will not run in parallel with " "any other tests. This should be used in conjunction with " "the ctest_test PARALLEL_LEVEL option.\n";