ENH: Improve test property speed with a map

Previously we stored a vector of tests to preserve their order.
Property set/get operations would do a linear search for matching tests.
This uses a map to efficiently look up tests while keeping the original
order with a vector for test file generation.

Source/cmGetPropertyCommand.cxx

@@ -328,16 +328,10 @@ bool cmGetPropertyCommand::HandleTestMode()
     }
 
   // Loop over all tests looking for matching names.
-  std::vector<cmTest*> const& tests = *this->Makefile->GetTests();
-  for(std::vector<cmTest*>::const_iterator ti = tests.begin();
-      ti != tests.end(); ++ti)
-    {
-    cmTest* test = *ti;
-    if(test->GetName() == this->Name)
+  if(cmTest* test = this->Makefile->GetTest(this->Name.c_str()))
     {
     return this->StoreResult(test->GetProperty(this->PropertyName.c_str()));
     }
-    }
 
   // If not found it is an error.
   cmOStringStream e;

Source/cmMakefile.cxx

@@ -108,6 +108,7 @@ cmMakefile::cmMakefile(const cmMakefile& mf)
   this->Targets = mf.Targets;
   this->SourceFiles = mf.SourceFiles;
   this->Tests = mf.Tests;
+  this->OrderedTests = mf.OrderedTests;
   this->IncludeDirectories = mf.IncludeDirectories;
   this->LinkDirectories = mf.LinkDirectories;
   this->SystemIncludeDirectories = mf.SystemIncludeDirectories;
@@ -182,10 +183,10 @@ cmMakefile::~cmMakefile()
     {
     delete *i;
     }
-  for(std::vector<cmTest*>::iterator i = this->Tests.begin();
+  for(std::map<cmStdString, cmTest*>::iterator i = this->Tests.begin();
       i != this->Tests.end(); ++i)
     {
-    delete *i;
+    delete i->second;
     }
   for(std::vector<cmTarget*>::iterator
         i = this->ImportedTargetsOwned.begin();
@@ -3117,6 +3118,7 @@ cmTarget* cmMakefile::FindTarget(const char* name)
   return 0;
 }
 
+//----------------------------------------------------------------------------
 cmTest* cmMakefile::CreateTest(const char* testName)
 {
   if ( !testName )
@@ -3131,35 +3133,30 @@ cmTest* cmMakefile::CreateTest(const char* testName)
   test = new cmTest;
   test->SetName(testName);
   test->SetMakefile(this);
-  this->Tests.push_back(test);
+  this->Tests[testName] = test;
+  this->OrderedTests.push_back(test);
   return test;
 }
 
+//----------------------------------------------------------------------------
 cmTest* cmMakefile::GetTest(const char* testName) const
 {
-  if ( !testName )
+  if(testName)
     {
-    return 0;
-    }
-  std::vector<cmTest*>::const_iterator it;
-  for ( it = this->Tests.begin(); it != this->Tests.end(); ++ it )
+    std::map<cmStdString, cmTest*>::const_iterator
+      mi = this->Tests.find(testName);
+    if(mi != this->Tests.end())
       {
-    if ( strcmp((*it)->GetName(), testName) == 0 )
-      {
-      return *it;
+      return mi->second;
       }
     }
   return 0;
 }
 
+//----------------------------------------------------------------------------
 const std::vector<cmTest*> *cmMakefile::GetTests() const
 {
-  return &this->Tests;
-}
-
-std::vector<cmTest*> *cmMakefile::GetTests()
-{
-  return &this->Tests;
+  return &this->OrderedTests;
 }
 
 std::string cmMakefile::GetListFileStack()

Source/cmMakefile.h

@@ -737,7 +737,6 @@ public:
    */
   cmTest* GetTest(const char* testName) const;
   const std::vector<cmTest*> *GetTests() const;
-  std::vector<cmTest*> *GetTests();
 
   /**
    * Get a list of macros as a ; separated string
@@ -807,7 +806,8 @@ protected:
   std::vector<cmSourceFile*> SourceFiles;
 
   // Tests
-  std::vector<cmTest*> Tests;
+  std::map<cmStdString, cmTest*> Tests;
+  std::vector<cmTest*> OrderedTests;
   
   // The include and link-library paths.  These may have order
   // dependency, so they must be vectors (not set).

Source/cmSetPropertyCommand.cxx

@@ -327,15 +327,14 @@ bool cmSetPropertyCommand::HandleSource(cmSourceFile* sf)
 //----------------------------------------------------------------------------
 bool cmSetPropertyCommand::HandleTestMode()
 {
-  // Loop over all tests looking for matching names.
-  std::vector<cmTest*> const& tests = *this->Makefile->GetTests();
-  for(std::vector<cmTest*>::const_iterator ti = tests.begin();
-      ti != tests.end(); ++ti)
+  // Look for tests with all names given.
+  std::set<cmStdString>::iterator next;
+  for(std::set<cmStdString>::iterator ni = this->Names.begin();
+      ni != this->Names.end(); ni = next)
     {
-    cmTest* test = *ti;
-    std::set<cmStdString>::iterator ni =
-      this->Names.find(test->GetName());
-    if(ni != this->Names.end())
+    next = ni;
+    ++next;
+    if(cmTest* test = this->Makefile->GetTest(ni->c_str()))
       {
       if(this->HandleTest(test))
         {

Source/cmSetTestsPropertiesCommand.cxx

@@ -100,30 +100,17 @@ bool cmSetTestsPropertiesCommand
              std::vector<std::string> &propertyPairs,
              cmMakefile *mf, std::string &errors)
 {
-  std::vector<cmTest*> &tests = *mf->GetTests();
-  // now loop over all the targets
-  unsigned int k;
-  bool found = false;
-  // if the file is already in the makefile just set properites on it
-  std::vector<cmTest*>::iterator it;
-  for ( it = tests.begin(); it != tests.end(); ++ it )
-    {
-    cmTest* test = *it;
-    if ( !strcmp(test->GetName(),tname ))
+  if(cmTest* test = mf->GetTest(tname))
     {
     // now loop through all the props and set them
+    unsigned int k;
     for (k = 0; k < propertyPairs.size(); k = k + 2)
       {
       test->SetProperty(propertyPairs[k].c_str(),
                         propertyPairs[k+1].c_str());
       }
-      found = true;
-      break;
     }
-    }
-  
-  // if file is not already in the makefile, then add it
-  if ( ! found )
+  else
     { 
     errors = "Can not find test to add properties to: ";
     errors += tname;