Merge branch 'improve-test-cost-sorting'

Source/CTest/cmCTestMultiProcessHandler.cxx

@@ -18,6 +18,23 @@
 #include <stack>
 #include <float.h>
 
+class TestComparator
+{
+public:
+  TestComparator(cmCTestMultiProcessHandler* handler) : Handler(handler) {}
+  ~TestComparator() {}
+
+  // Sorts tests in descending order of cost
+  bool operator() (int index1, int index2) const
+    {
+    return Handler->Properties[index1]->Cost >
+      Handler->Properties[index2]->Cost;
+    }
+
+private:
+  cmCTestMultiProcessHandler* Handler;
+};
+
 cmCTestMultiProcessHandler::cmCTestMultiProcessHandler()
 {
   this->ParallelLevel = 1;
@@ -154,15 +171,8 @@ void cmCTestMultiProcessHandler::UnlockResources(int index)
 void cmCTestMultiProcessHandler::EraseTest(int test)
 {
   this->Tests.erase(test);
-  for(TestCostMap::iterator i = this->TestCosts.begin();
+  this->SortedTests.erase(
-      i != this->TestCosts.end(); ++i)
+    std::find(this->SortedTests.begin(), this->SortedTests.end(), test));
-    {
-    if(i->second.find(test) != i->second.end())
-      {
-      i->second.erase(test);
-      return;
-      }
-    }
 }
 
 //---------------------------------------------------------
@@ -244,12 +254,8 @@ void cmCTestMultiProcessHandler::StartNextTests()
     return;
     }
 
-  for(TestCostMap::reverse_iterator i = this->TestCosts.rbegin();
+  TestList copy = this->SortedTests;
-      i != this->TestCosts.rend(); ++i)
+  for(TestList::iterator test = copy.begin(); test != copy.end(); ++test)
-    {
-    TestSet tests = i->second; //copy the test set
-    for(TestSet::iterator test = tests.begin();
-        test != tests.end(); ++test)
     {
     //in case this test has already been started due to dependency
     if(this->TestRunningMap[*test] || this->TestFinishMap[*test])
@@ -281,7 +287,6 @@ void cmCTestMultiProcessHandler::StartNextTests()
       }
     }
 }
-}
 
 //---------------------------------------------------------
 bool cmCTestMultiProcessHandler::CheckOutput()
@@ -468,26 +473,21 @@ void cmCTestMultiProcessHandler::CreateTestCostList()
   for(TestMap::iterator i = this->Tests.begin();
       i != this->Tests.end(); ++i)
     {
-    //We only want to schedule them by cost in a parallel situation
+    SortedTests.push_back(i->first);
+
+    //If the test failed last time, it should be run first, so max the cost
+    if(std::find(this->LastTestsFailed.begin(),
+                 this->LastTestsFailed.end(),
+                 this->Properties[i->first]->Name)
+       != this->LastTestsFailed.end())
+      {
+      this->Properties[i->first]->Cost = FLT_MAX;
+      }
+    }
   if(this->ParallelLevel > 1)
     {
-      std::string name = this->Properties[i->first]->Name;
+    TestComparator comp(this);
-      if(std::find(this->LastTestsFailed.begin(), this->LastTestsFailed.end(),
+    std::sort(SortedTests.begin(), SortedTests.end(), comp);
-         name) != this->LastTestsFailed.end())
-        {
-        this->TestCosts[FLT_MAX].insert(i->first);
-        }
-      else
-        {
-        this->TestCosts[this->Properties[i->first]->Cost].insert(i->first);
-        }
-      }
-    else //we ignore their cost
-      {
-      size_t index = this->Tests.size()
-        - static_cast<size_t>(this->Properties[i->first]->Index);
-      this->TestCosts[index].insert(i->first);
-      }
     }
 }
 

Source/CTest/cmCTestMultiProcessHandler.h

@@ -23,10 +23,11 @@
  */
 class cmCTestMultiProcessHandler 
 {
+  friend class TestComparator;
 public:
   struct TestSet : public std::set<int> {};
   struct TestMap : public std::map<int, TestSet> {};
-  struct TestCostMap : public std::map<float, TestSet> {};
+  struct TestList : public std::vector<int> {};
   struct PropertiesMap : public 
      std::map<int, cmCTestTestHandler::cmCTestTestProperties*> {};
 
@@ -88,7 +89,7 @@ protected:
   void UnlockResources(int index);
   // map from test number to set of depend tests
   TestMap Tests;
-  TestCostMap TestCosts;
+  TestList SortedTests;
   //Total number of tests we'll be running
   size_t Total;
   //Number of tests that are complete