From 19b7c22d020a3b1bf26065beb33b0f2f499cd1ad Mon Sep 17 00:00:00 2001 From: Stephen Kelly Date: Fri, 4 Apr 2014 17:09:56 +0200 Subject: Ninja: Query custom commands once per target, not once per file. Computing the source files is now more expensive, so the Ninja generator became very slow with a large number of files. --- Source/cmNinjaTargetGenerator.cxx | 14 +++++++------- Source/cmNinjaTargetGenerator.h | 1 + 2 files changed, 8 insertions(+), 7 deletions(-) diff --git a/Source/cmNinjaTargetGenerator.cxx b/Source/cmNinjaTargetGenerator.cxx index 56155ef..cb6eb90 100644 --- a/Source/cmNinjaTargetGenerator.cxx +++ b/Source/cmNinjaTargetGenerator.cxx @@ -494,6 +494,9 @@ cmNinjaTargetGenerator { cmCustomCommand const* cc = (*si)->GetCustomCommand(); this->GetLocalGenerator()->AddCustomCommandTarget(cc, this->GetTarget()); + // Record the custom commands for this target. The container is used + // in WriteObjectBuildStatement when called in a loop below. + this->CustomCommands.push_back((*si)->GetCustomCommand()); } std::vector headerSources; this->GeneratorTarget->GetHeaderSources(headerSources, config); @@ -565,14 +568,11 @@ cmNinjaTargetGenerator } // Add order-only dependencies on custom command outputs. - std::vector customCommands; - std::string config = this->Makefile->GetSafeDefinition("CMAKE_BUILD_TYPE"); - this->GeneratorTarget->GetCustomCommands(customCommands, config); - for(std::vector::const_iterator - si = customCommands.begin(); - si != customCommands.end(); ++si) + for(std::vector::const_iterator + cci = this->CustomCommands.begin(); + cci != this->CustomCommands.end(); ++cci) { - cmCustomCommand const* cc = (*si)->GetCustomCommand(); + cmCustomCommand const* cc = *cci; cmCustomCommandGenerator ccg(*cc, this->GetConfigName(), this->GetMakefile()); const std::vector& ccoutputs = ccg.GetOutputs(); diff --git a/Source/cmNinjaTargetGenerator.h b/Source/cmNinjaTargetGenerator.h index 8669e6e..8073af2 100644 --- a/Source/cmNinjaTargetGenerator.h +++ b/Source/cmNinjaTargetGenerator.h @@ -153,6 +153,7 @@ private: cmLocalNinjaGenerator* LocalGenerator; /// List of object files for this target. cmNinjaDeps Objects; + std::vector CustomCommands; typedef std::map LanguageFlagMap; LanguageFlagMap LanguageFlags; -- cgit v0.12 From eb163f37d449af27fe8446c5d8d632aa295b6428 Mon Sep 17 00:00:00 2001 From: Stephen Kelly Date: Fri, 4 Apr 2014 17:45:28 +0200 Subject: cmTarget: Extract a ProcessSourceItemCMP0049 method. Avoid calling AddSource for each src filename. That involves checking each entry for uniqueness and creating a separate generator expression for each one. Instead, add a single entry for the list of sources. The source files are passed through a uniqueness filter at generate-time, so duplicates don't matter so much. --- Source/cmTarget.cxx | 48 +++++++++++++++++++++++++++++++++++++++--------- Source/cmTarget.h | 2 ++ 2 files changed, 41 insertions(+), 9 deletions(-) diff --git a/Source/cmTarget.cxx b/Source/cmTarget.cxx index b6bb2d3..22be57f 100644 --- a/Source/cmTarget.cxx +++ b/Source/cmTarget.cxx @@ -812,23 +812,41 @@ void cmTarget::GetSourceFiles(std::vector &files, //---------------------------------------------------------------------------- void cmTarget::AddSources(std::vector const& srcs) { + std::string srcFiles; + const char* sep = ""; for(std::vector::const_iterator i = srcs.begin(); i != srcs.end(); ++i) { - const char* src = i->c_str(); - if(src[0] == '$' && src[1] == '<') - { - this->AddSource(src); - } - else + std::string filename = *i; + const char* src = filename.c_str(); + + if(!(src[0] == '$' && src[1] == '<')) { - this->AddSourceCMP0049(src); + filename = this->ProcessSourceItemCMP0049(filename); + if (cmSystemTools::GetErrorOccuredFlag()) + { + return; + } + this->Makefile->GetOrCreateSource(filename); } + srcFiles += sep; + srcFiles += filename; + sep = ";"; + } + if (!srcFiles.empty()) + { + cmListFileBacktrace lfbt; + this->Makefile->GetBacktrace(lfbt); + cmGeneratorExpression ge(lfbt); + cmsys::auto_ptr cge = ge.Parse(srcFiles); + cge->SetEvaluateForBuildsystem(true); + this->Internal->SourceEntries.push_back( + new cmTargetInternals::TargetPropertyEntry(cge)); } } //---------------------------------------------------------------------------- -cmSourceFile* cmTarget::AddSourceCMP0049(const std::string& s) +std::string cmTarget::ProcessSourceItemCMP0049(const std::string& s) { std::string src = s; @@ -863,10 +881,22 @@ cmSourceFile* cmTarget::AddSourceCMP0049(const std::string& s) this->Makefile->IssueMessage(messageType, e.str()); if (messageType == cmake::FATAL_ERROR) { - return 0; + return ""; } } } + return src; +} + +//---------------------------------------------------------------------------- +cmSourceFile* cmTarget::AddSourceCMP0049(const std::string& s) +{ + std::string src = this->ProcessSourceItemCMP0049(s); + + if (cmSystemTools::GetErrorOccuredFlag()) + { + return 0; + } return this->AddSource(src); } diff --git a/Source/cmTarget.h b/Source/cmTarget.h index da032a5..a8ac57f 100644 --- a/Source/cmTarget.h +++ b/Source/cmTarget.h @@ -752,6 +752,8 @@ private: void ComputeLinkClosure(const std::string& config, LinkClosure& lc, cmTarget const* head) const; + std::string ProcessSourceItemCMP0049(const std::string& s); + void ClearLinkMaps(); void MaybeInvalidatePropertyCache(const std::string& prop); -- cgit v0.12 From c5b26f3bec0e6e18255802da53886eae30a74021 Mon Sep 17 00:00:00 2001 From: Stephen Kelly Date: Sat, 5 Apr 2014 11:40:38 +0200 Subject: cmTarget: Cache the cmSourceFiles in GetSourceFiles. Avoid calling GetSourceFiles with the same result container multiple times when tracing target dependencies. The result from the previous configuration is cached and used later otherwise. --- Source/cmGeneratorTarget.cxx | 18 +++++++++--------- Source/cmTarget.cxx | 39 ++++++++++++++++++++++++++++++--------- 2 files changed, 39 insertions(+), 18 deletions(-) diff --git a/Source/cmGeneratorTarget.cxx b/Source/cmGeneratorTarget.cxx index 01fad26..69a2977 100644 --- a/Source/cmGeneratorTarget.cxx +++ b/Source/cmGeneratorTarget.cxx @@ -636,26 +636,26 @@ cmTargetTraceDependencies // Queue all the source files already specified for the target. if (this->Target->GetType() != cmTarget::INTERFACE_LIBRARY) { - std::vector sources; std::vector configs; this->Makefile->GetConfigurations(configs); if (configs.empty()) { configs.push_back(""); } + std::set emitted; for(std::vector::const_iterator ci = configs.begin(); ci != configs.end(); ++ci) { + std::vector sources; this->Target->GetSourceFiles(sources, *ci); - } - std::set emitted; - for(std::vector::const_iterator si = sources.begin(); - si != sources.end(); ++si) - { - if(emitted.insert(*si).second && this->SourcesQueued.insert(*si).second) + for(std::vector::const_iterator si = sources.begin(); + si != sources.end(); ++si) { - this->SourceQueue.push(*si); - this->Makefile->GetOrCreateSource(*si); + if(emitted.insert(*si).second && this->SourcesQueued.insert(*si).second) + { + this->SourceQueue.push(*si); + this->Makefile->GetOrCreateSource(*si); + } } } } diff --git a/Source/cmTarget.cxx b/Source/cmTarget.cxx index 22be57f..d28ac3f 100644 --- a/Source/cmTarget.cxx +++ b/Source/cmTarget.cxx @@ -138,6 +138,10 @@ public: LinkClosureMapType; LinkClosureMapType LinkClosureMap; + typedef std::map > + SourceFilesMapType; + SourceFilesMapType SourceFilesMap; + struct TargetPropertyEntry { TargetPropertyEntry(cmsys::auto_ptr cge, const std::string &targetName = std::string()) @@ -793,19 +797,33 @@ void cmTarget::GetSourceFiles(std::vector &files, const std::string& config, cmTarget const* head) const { - std::vector srcs; - this->GetSourceFiles(srcs, config, head); - std::set emitted; + // Lookup any existing link implementation for this configuration. + TargetConfigPair key(head, cmSystemTools::UpperCase(config)); - for(std::vector::const_iterator i = srcs.begin(); - i != srcs.end(); ++i) + cmTargetInternals::SourceFilesMapType::iterator + it = this->Internal->SourceFilesMap.find(key); + if(it != this->Internal->SourceFilesMap.end()) + { + files = it->second; + } + else { - cmSourceFile* sf = this->Makefile->GetOrCreateSource(*i); - if (emitted.insert(sf).second) + std::vector srcs; + this->GetSourceFiles(srcs, config, head); + + std::set emitted; + + for(std::vector::const_iterator i = srcs.begin(); + i != srcs.end(); ++i) { - files.push_back(sf); + cmSourceFile* sf = this->Makefile->GetOrCreateSource(*i); + if (emitted.insert(sf).second) + { + files.push_back(sf); + } } + this->Internal->SourceFilesMap[key] = files; } } @@ -835,6 +853,7 @@ void cmTarget::AddSources(std::vector const& srcs) } if (!srcFiles.empty()) { + this->Internal->SourceFilesMap.clear(); cmListFileBacktrace lfbt; this->Makefile->GetBacktrace(lfbt); cmGeneratorExpression ge(lfbt); @@ -969,6 +988,7 @@ cmSourceFile* cmTarget::AddSource(const std::string& src) TargetPropertyEntryFinder(sfl)) == this->Internal->SourceEntries.end()) { + this->Internal->SourceFilesMap.clear(); cmListFileBacktrace lfbt; this->Makefile->GetBacktrace(lfbt); cmGeneratorExpression ge(lfbt); @@ -1738,6 +1758,7 @@ void cmTarget::SetProperty(const std::string& prop, const char* value) this->Makefile->IssueMessage(cmake::FATAL_ERROR, e.str()); return; } + this->Internal->SourceFilesMap.clear(); cmListFileBacktrace lfbt; this->Makefile->GetBacktrace(lfbt); cmGeneratorExpression ge(lfbt); @@ -1824,7 +1845,7 @@ void cmTarget::AppendProperty(const std::string& prop, const char* value, this->Makefile->IssueMessage(cmake::FATAL_ERROR, e.str()); return; } - + this->Internal->SourceFilesMap.clear(); cmListFileBacktrace lfbt; this->Makefile->GetBacktrace(lfbt); cmGeneratorExpression ge(lfbt); -- cgit v0.12 From 92e2fbe103c6ffc3af1189d0450493488c65baaa Mon Sep 17 00:00:00 2001 From: Stephen Kelly Date: Sat, 5 Apr 2014 11:41:58 +0200 Subject: cmGeneratorTarget: Trace cmSourceFile objects instead of strings. This reverses the decision in commit d38423ec (cmTarget: Add a method to obtain list of filenames for sources., 2014-03-17). The cmSourceFile based API is preferred because that avoids creation of many cmSourceFileLocation objects for matching strings, and the result is cached by cmTarget. --- Source/cmGeneratorTarget.cxx | 31 +++++++++++++++---------------- 1 file changed, 15 insertions(+), 16 deletions(-) diff --git a/Source/cmGeneratorTarget.cxx b/Source/cmGeneratorTarget.cxx index 69a2977..0d25a00 100644 --- a/Source/cmGeneratorTarget.cxx +++ b/Source/cmGeneratorTarget.cxx @@ -606,12 +606,12 @@ private: cmGlobalGenerator const* GlobalGenerator; typedef cmGeneratorTarget::SourceEntry SourceEntry; SourceEntry* CurrentEntry; - std::queue SourceQueue; - std::set SourcesQueued; + std::queue SourceQueue; + std::set SourcesQueued; typedef std::map NameMapType; NameMapType NameMap; - void QueueSource(std::string const& name); + void QueueSource(cmSourceFile* sf); void FollowName(std::string const& name); void FollowNames(std::vector const& names); bool IsUtility(std::string const& dep); @@ -642,19 +642,19 @@ cmTargetTraceDependencies { configs.push_back(""); } - std::set emitted; + std::set emitted; for(std::vector::const_iterator ci = configs.begin(); ci != configs.end(); ++ci) { - std::vector sources; + std::vector sources; this->Target->GetSourceFiles(sources, *ci); - for(std::vector::const_iterator si = sources.begin(); + for(std::vector::const_iterator si = sources.begin(); si != sources.end(); ++si) { - if(emitted.insert(*si).second && this->SourcesQueued.insert(*si).second) + cmSourceFile* sf = *si; + if(emitted.insert(sf).second && this->SourcesQueued.insert(sf).second) { - this->SourceQueue.push(*si); - this->Makefile->GetOrCreateSource(*si); + this->SourceQueue.push(sf); } } } @@ -673,8 +673,7 @@ void cmTargetTraceDependencies::Trace() while(!this->SourceQueue.empty()) { // Get the next source from the queue. - std::string src = this->SourceQueue.front(); - cmSourceFile* sf = this->Makefile->GetSource(src); + cmSourceFile* sf = this->SourceQueue.front(); this->SourceQueue.pop(); this->CurrentEntry = &this->GeneratorTarget->SourceEntries[sf]; @@ -702,14 +701,14 @@ void cmTargetTraceDependencies::Trace() } //---------------------------------------------------------------------------- -void cmTargetTraceDependencies::QueueSource(std::string const& name) +void cmTargetTraceDependencies::QueueSource(cmSourceFile* sf) { - if(this->SourcesQueued.insert(name).second) + if(this->SourcesQueued.insert(sf).second) { - this->SourceQueue.push(name); + this->SourceQueue.push(sf); // Make sure this file is in the target. - this->Target->AddSource(name); + this->Target->AddSource(sf->GetFullPath()); } } @@ -731,7 +730,7 @@ void cmTargetTraceDependencies::FollowName(std::string const& name) { this->CurrentEntry->Depends.push_back(sf); } - this->QueueSource(sf->GetFullPath()); + this->QueueSource(sf); } } -- cgit v0.12 From a4e6bf8e36e6fa38bb40dc6df2345922de86bad4 Mon Sep 17 00:00:00 2001 From: Stephen Kelly Date: Sat, 5 Apr 2014 12:35:43 +0200 Subject: cmTarget: Make GetSourceFiles string overload private. Consumers should use the cmSourceFile overload, which is now always the case. --- Source/cmTarget.h | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Source/cmTarget.h b/Source/cmTarget.h index a8ac57f..da9d0a1 100644 --- a/Source/cmTarget.h +++ b/Source/cmTarget.h @@ -135,9 +135,6 @@ public: /** * Get the list of the source files used by this target */ - void GetSourceFiles(std::vector &files, - const std::string& config, - cmTarget const* head = 0) const; void GetSourceFiles(std::vector &files, const std::string& config, cmTarget const* head = 0) const; @@ -683,6 +680,9 @@ private: const std::string& config, bool contentOnly) const; + void GetSourceFiles(std::vector &files, + const std::string& config, + cmTarget const* head = 0) const; private: std::string Name; std::vector PreBuildCommands; -- cgit v0.12