Merge topic 'fix-target-lookup-performance-regression'

9b7d5871 Improve internal generator target structure lookup
6cbf6a51 Fix internal target lookup performance regression

Source/cmGlobalGenerator.cxx

@@ -1649,6 +1649,8 @@ void cmGlobalGenerator::ClearGeneratorMembers()
 
   this->ExportSets.clear();
   this->TargetDependencies.clear();
+  this->TargetSearchIndex.clear();
+  this->GeneratorTargetSearchIndex.clear();
   this->ProjectMap.clear();
   this->RuleHashes.clear();
   this->DirectoryContentMap.clear();
@@ -2177,18 +2179,28 @@ bool cmGlobalGenerator::IsAlias(const std::string& name) const
   return this->AliasTargets.find(name) != this->AliasTargets.end();
 }
 
+void cmGlobalGenerator::IndexTarget(cmTarget* t)
+{
+  if (!t->IsImported() || t->IsImportedGloballyVisible())
+    {
+    this->TargetSearchIndex[t->GetName()] = t;
+    }
+}
+
+void cmGlobalGenerator::IndexGeneratorTarget(cmGeneratorTarget* gt)
+{
+  if (!gt->IsImported() || gt->IsImportedGloballyVisible())
+    {
+    this->GeneratorTargetSearchIndex[gt->GetName()] = gt;
+    }
+}
+
 cmTarget* cmGlobalGenerator::FindTargetImpl(std::string const& name) const
 {
-  for (unsigned int i = 0; i < this->Makefiles.size(); ++i)
+  TargetMap::const_iterator i = this->TargetSearchIndex.find(name);
+  if (i != this->TargetSearchIndex.end())
     {
-    cmTargets& tgts = this->Makefiles[i]->GetTargets();
-    for (cmTargets::iterator it = tgts.begin(); it != tgts.end(); ++it)
-      {
-      if (it->second.GetName() == name)
-        {
-        return &it->second;
-        }
-      }
+    return i->second;
     }
   return 0;
 }
@@ -2196,56 +2208,11 @@ cmTarget* cmGlobalGenerator::FindTargetImpl(std::string const& name) const
 cmGeneratorTarget*
 cmGlobalGenerator::FindGeneratorTargetImpl(std::string const& name) const
 {
-  for (unsigned int i = 0; i < this->LocalGenerators.size(); ++i)
+  GeneratorTargetMap::const_iterator i =
+    this->GeneratorTargetSearchIndex.find(name);
+  if (i != this->GeneratorTargetSearchIndex.end())
     {
-    const std::vector<cmGeneratorTarget*>& tgts =
-        this->LocalGenerators[i]->GetGeneratorTargets();
-    for (std::vector<cmGeneratorTarget*>::const_iterator it = tgts.begin();
-         it != tgts.end(); ++it)
-      {
-      if ((*it)->GetName() == name)
-        {
-        return *it;
-        }
-      }
-    }
-  return 0;
-}
-
-cmTarget*
-cmGlobalGenerator::FindImportedTargetImpl(std::string const& name) const
-{
-  for (unsigned int i = 0; i < this->Makefiles.size(); ++i)
-    {
-    const std::vector<cmTarget*>& tgts =
-        this->Makefiles[i]->GetOwnedImportedTargets();
-    for (std::vector<cmTarget*>::const_iterator it = tgts.begin();
-         it != tgts.end(); ++it)
-      {
-      if ((*it)->GetName() == name && (*it)->IsImportedGloballyVisible())
-        {
-        return *it;
-        }
-      }
-    }
-  return 0;
-}
-
-cmGeneratorTarget* cmGlobalGenerator::FindImportedGeneratorTargetImpl(
-    std::string const& name) const
-{
-  for (unsigned int i = 0; i < this->LocalGenerators.size(); ++i)
-    {
-    const std::vector<cmGeneratorTarget*>& tgts =
-        this->LocalGenerators[i]->GetImportedGeneratorTargets();
-    for (std::vector<cmGeneratorTarget*>::const_iterator it = tgts.begin();
-         it != tgts.end(); ++it)
-      {
-      if ((*it)->IsImportedGloballyVisible() && (*it)->GetName() == name)
-        {
-        return *it;
-        }
-      }
+    return i->second;
     }
   return 0;
 }
@@ -2264,11 +2231,7 @@ cmGlobalGenerator::FindTarget(const std::string& name,
       return this->FindTargetImpl(ai->second);
       }
     }
-  if (cmTarget* tgt = this->FindTargetImpl(name))
-    {
-    return tgt;
-    }
-  return this->FindImportedTargetImpl(name);
+  return this->FindTargetImpl(name);
 }
 
 cmGeneratorTarget*
@@ -2280,11 +2243,7 @@ cmGlobalGenerator::FindGeneratorTarget(const std::string& name) const
     {
     return this->FindGeneratorTargetImpl(ai->second);
     }
-  if (cmGeneratorTarget* tgt = this->FindGeneratorTargetImpl(name))
-    {
-    return tgt;
-    }
-  return this->FindImportedGeneratorTargetImpl(name);
+  return this->FindGeneratorTargetImpl(name);
 }
 
 //----------------------------------------------------------------------------

Source/cmGlobalGenerator.h

@@ -278,6 +278,9 @@ public:
   std::set<std::string> const& GetDirectoryContent(std::string const& dir,
                                                    bool needDisk = true);
 
+  void IndexTarget(cmTarget* t);
+  void IndexGeneratorTarget(cmGeneratorTarget* gt);
+
   static bool IsReservedTarget(std::string const& name);
 
   virtual const char* GetAllTargetName()         const { return "ALL_BUILD"; }
@@ -420,7 +423,6 @@ protected:
   std::map<std::string, std::string> AliasTargets;
 
   cmTarget* FindTargetImpl(std::string const& name) const;
-  cmTarget* FindImportedTargetImpl(std::string const& name) const;
 
   cmGeneratorTarget* FindGeneratorTargetImpl(std::string const& name) const;
   cmGeneratorTarget*
@@ -430,6 +432,26 @@ protected:
   virtual bool UseFolderProperty();
 
 private:
+
+#if defined(CMAKE_BUILD_WITH_CMAKE)
+# ifdef CMake_HAVE_CXX11_UNORDERED_MAP
+  typedef std::unordered_map<std::string, cmTarget*> TargetMap;
+  typedef std::unordered_map<std::string, cmGeneratorTarget*>
+    GeneratorTargetMap;
+# else
+  typedef cmsys::hash_map<std::string, cmTarget*> TargetMap;
+  typedef cmsys::hash_map<std::string, cmGeneratorTarget*> GeneratorTargetMap;
+# endif
+#else
+  typedef std::map<std::string,cmTarget *> TargetMap;
+  typedef std::map<std::string,cmGeneratorTarget *> GeneratorTargetMap;
+#endif
+  // Map efficiently from target name to cmTarget instance.
+  // Do not use this structure for looping over all targets.
+  // It contains both normal and globally visible imported targets.
+  TargetMap TargetSearchIndex;
+  GeneratorTargetMap GeneratorTargetSearchIndex;
+
   cmMakefile* TryCompileOuterMakefile;
   // If you add a new map here, make sure it is copied
   // in EnableLanguagesFromGenerator

Source/cmLocalGenerator.cxx

@@ -455,11 +455,13 @@ void cmLocalGenerator::GenerateInstallRules()
 void cmLocalGenerator::AddGeneratorTarget(cmGeneratorTarget* gt)
 {
   this->GeneratorTargets.push_back(gt);
+  this->GlobalGenerator->IndexGeneratorTarget(gt);
 }
 
 void cmLocalGenerator::AddImportedGeneratorTarget(cmGeneratorTarget* gt)
 {
   this->ImportedGeneratorTargets.push_back(gt);
+  this->GlobalGenerator->IndexGeneratorTarget(gt);
 }
 
 void cmLocalGenerator::AddOwnedImportedGeneratorTarget(cmGeneratorTarget* gt)

Source/cmMakefile.cxx

@@ -2128,6 +2128,7 @@ cmMakefile::AddNewTarget(cmState::TargetType type, const std::string& name)
   cmTarget& target = it->second;
   target.SetType(type, name);
   target.SetMakefile(this);
+  this->GetGlobalGenerator()->IndexTarget(&it->second);
   return &it->second;
 }
 
@@ -4218,6 +4219,7 @@ cmMakefile::AddImportedTarget(const std::string& name,
 
   // Add to the set of available imported targets.
   this->ImportedTargets[name] = target.get();
+  this->GetGlobalGenerator()->IndexTarget(target.get());
 
   // Transfer ownership to this cmMakefile object.
   this->ImportedTargetsOwned.push_back(target.get());