Commit 6cbf6a51 authored by Brad King's avatar Brad King
Browse files

Fix internal target lookup performance regression



Refactoring in commit v3.5.0-rc1~272^2~13 (cmGlobalGenerator: Remove
direct storage of targets, 2015-10-25) replaced an efficient data
structure mapping from target name to cmTarget instance with a linear
search.  Lookups through cmGlobalGenerator::FindTarget are done a lot.
Restore the efficient mapping structure with a name indicating its
purpose.
Reported-by: Bartosz's avatarBartosz Kosiorek <gang65@poczta.onet.pl>
parent a5a5a685
...@@ -1649,6 +1649,7 @@ void cmGlobalGenerator::ClearGeneratorMembers() ...@@ -1649,6 +1649,7 @@ void cmGlobalGenerator::ClearGeneratorMembers()
this->ExportSets.clear(); this->ExportSets.clear();
this->TargetDependencies.clear(); this->TargetDependencies.clear();
this->TargetSearchIndex.clear();
this->ProjectMap.clear(); this->ProjectMap.clear();
this->RuleHashes.clear(); this->RuleHashes.clear();
this->DirectoryContentMap.clear(); this->DirectoryContentMap.clear();
...@@ -2177,18 +2178,20 @@ bool cmGlobalGenerator::IsAlias(const std::string& name) const ...@@ -2177,18 +2178,20 @@ bool cmGlobalGenerator::IsAlias(const std::string& name) const
return this->AliasTargets.find(name) != this->AliasTargets.end(); return this->AliasTargets.find(name) != this->AliasTargets.end();
} }
void cmGlobalGenerator::IndexTarget(cmTarget* t)
{
if (!t->IsImported() || t->IsImportedGloballyVisible())
{
this->TargetSearchIndex[t->GetName()] = t;
}
}
cmTarget* cmGlobalGenerator::FindTargetImpl(std::string const& name) const 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(); return i->second;
for (cmTargets::iterator it = tgts.begin(); it != tgts.end(); ++it)
{
if (it->second.GetName() == name)
{
return &it->second;
}
}
} }
return 0; return 0;
} }
...@@ -2212,25 +2215,6 @@ cmGlobalGenerator::FindGeneratorTargetImpl(std::string const& name) const ...@@ -2212,25 +2215,6 @@ cmGlobalGenerator::FindGeneratorTargetImpl(std::string const& name) const
return 0; 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( cmGeneratorTarget* cmGlobalGenerator::FindImportedGeneratorTargetImpl(
std::string const& name) const std::string const& name) const
{ {
...@@ -2264,11 +2248,7 @@ cmGlobalGenerator::FindTarget(const std::string& name, ...@@ -2264,11 +2248,7 @@ cmGlobalGenerator::FindTarget(const std::string& name,
return this->FindTargetImpl(ai->second); return this->FindTargetImpl(ai->second);
} }
} }
if (cmTarget* tgt = this->FindTargetImpl(name)) return this->FindTargetImpl(name);
{
return tgt;
}
return this->FindImportedTargetImpl(name);
} }
cmGeneratorTarget* cmGeneratorTarget*
......
...@@ -278,6 +278,8 @@ public: ...@@ -278,6 +278,8 @@ public:
std::set<std::string> const& GetDirectoryContent(std::string const& dir, std::set<std::string> const& GetDirectoryContent(std::string const& dir,
bool needDisk = true); bool needDisk = true);
void IndexTarget(cmTarget* t);
static bool IsReservedTarget(std::string const& name); static bool IsReservedTarget(std::string const& name);
virtual const char* GetAllTargetName() const { return "ALL_BUILD"; } virtual const char* GetAllTargetName() const { return "ALL_BUILD"; }
...@@ -420,7 +422,6 @@ protected: ...@@ -420,7 +422,6 @@ protected:
std::map<std::string, std::string> AliasTargets; std::map<std::string, std::string> AliasTargets;
cmTarget* FindTargetImpl(std::string const& name) const; cmTarget* FindTargetImpl(std::string const& name) const;
cmTarget* FindImportedTargetImpl(std::string const& name) const;
cmGeneratorTarget* FindGeneratorTargetImpl(std::string const& name) const; cmGeneratorTarget* FindGeneratorTargetImpl(std::string const& name) const;
cmGeneratorTarget* cmGeneratorTarget*
...@@ -430,6 +431,21 @@ protected: ...@@ -430,6 +431,21 @@ protected:
virtual bool UseFolderProperty(); virtual bool UseFolderProperty();
private: private:
#if defined(CMAKE_BUILD_WITH_CMAKE)
# ifdef CMake_HAVE_CXX11_UNORDERED_MAP
typedef std::unordered_map<std::string, cmTarget*> TargetMap;
# else
typedef cmsys::hash_map<std::string, cmTarget*> TargetMap;
# endif
#else
typedef std::map<std::string,cmTarget *> TargetMap;
#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;
cmMakefile* TryCompileOuterMakefile; cmMakefile* TryCompileOuterMakefile;
// If you add a new map here, make sure it is copied // If you add a new map here, make sure it is copied
// in EnableLanguagesFromGenerator // in EnableLanguagesFromGenerator
......
...@@ -2128,6 +2128,7 @@ cmMakefile::AddNewTarget(cmState::TargetType type, const std::string& name) ...@@ -2128,6 +2128,7 @@ cmMakefile::AddNewTarget(cmState::TargetType type, const std::string& name)
cmTarget& target = it->second; cmTarget& target = it->second;
target.SetType(type, name); target.SetType(type, name);
target.SetMakefile(this); target.SetMakefile(this);
this->GetGlobalGenerator()->IndexTarget(&it->second);
return &it->second; return &it->second;
} }
...@@ -4218,6 +4219,7 @@ cmMakefile::AddImportedTarget(const std::string& name, ...@@ -4218,6 +4219,7 @@ cmMakefile::AddImportedTarget(const std::string& name,
// Add to the set of available imported targets. // Add to the set of available imported targets.
this->ImportedTargets[name] = target.get(); this->ImportedTargets[name] = target.get();
this->GetGlobalGenerator()->IndexTarget(target.get());
// Transfer ownership to this cmMakefile object. // Transfer ownership to this cmMakefile object.
this->ImportedTargetsOwned.push_back(target.get()); this->ImportedTargetsOwned.push_back(target.get());
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment