Merge topic 'optimize-source-file-processing'

a4e6bf8e cmTarget: Make GetSourceFiles string overload private.
92e2fbe1 cmGeneratorTarget: Trace cmSourceFile objects instead of strings.
c5b26f3b cmTarget: Cache the cmSourceFiles in GetSourceFiles.
eb163f37 cmTarget: Extract a ProcessSourceItemCMP0049 method.
19b7c22d Ninja: Query custom commands once per target, not once per file.

5 files changed, 100 insertions, 47 deletions

diff --git a/Source/cmGeneratorTarget.cxx b/Source/cmGeneratorTarget.cxx
index 01fad26..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<std::string> SourceQueue;
-  std::set<std::string> SourcesQueued;
+  std::queue<cmSourceFile*> SourceQueue;
+  std::set<cmSourceFile*> SourcesQueued;
   typedef std::map<std::string, cmSourceFile*> NameMapType;
   NameMapType NameMap;
 
-  void QueueSource(std::string const& name);
+  void QueueSource(cmSourceFile* sf);
   void FollowName(std::string const& name);
   void FollowNames(std::vector<std::string> const& names);
   bool IsUtility(std::string const& dep);
@@ -636,26 +636,26 @@ cmTargetTraceDependencies
   // Queue all the source files already specified for the target.
   if (this->Target->GetType() != cmTarget::INTERFACE_LIBRARY)
     {
-    std::vector<std::string> sources;
     std::vector<std::string> configs;
     this->Makefile->GetConfigurations(configs);
     if (configs.empty())
       {
       configs.push_back("");
       }
+    std::set<cmSourceFile*> emitted;
     for(std::vector<std::string>::const_iterator ci = configs.begin();
         ci != configs.end(); ++ci)
       {
+      std::vector<cmSourceFile*> sources;
       this->Target->GetSourceFiles(sources, *ci);
-      }
-    std::set<std::string> emitted;
-    for(std::vector<std::string>::const_iterator si = sources.begin();
-        si != sources.end(); ++si)
-      {
-      if(emitted.insert(*si).second && this->SourcesQueued.insert(*si).second)
+      for(std::vector<cmSourceFile*>::const_iterator si = sources.begin();
+          si != sources.end(); ++si)
         {
-        this->SourceQueue.push(*si);
-        this->Makefile->GetOrCreateSource(*si);
+        cmSourceFile* sf = *si;
+        if(emitted.insert(sf).second && this->SourcesQueued.insert(sf).second)
+          {
+          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);
     }
 }
 
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<cmSourceFile const*> headerSources;
   this->GeneratorTarget->GetHeaderSources(headerSources, config);
@@ -565,14 +568,11 @@ cmNinjaTargetGenerator
   }
 
   // Add order-only dependencies on custom command outputs.
-  std::vector<cmSourceFile const*> customCommands;
-  std::string config = this->Makefile->GetSafeDefinition("CMAKE_BUILD_TYPE");
-  this->GeneratorTarget->GetCustomCommands(customCommands, config);
-  for(std::vector<cmSourceFile const*>::const_iterator
-        si = customCommands.begin();
-      si != customCommands.end(); ++si)
+  for(std::vector<cmCustomCommand const*>::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<std::string>& 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<cmCustomCommand const*> CustomCommands;
 
   typedef std::map<std::string, std::string> LanguageFlagMap;
   LanguageFlagMap LanguageFlags;
diff --git a/Source/cmTarget.cxx b/Source/cmTarget.cxx
index b6bb2d3..d28ac3f 100644
--- a/Source/cmTarget.cxx
+++ b/Source/cmTarget.cxx
@@ -138,6 +138,10 @@ public:
                                                           LinkClosureMapType;
   LinkClosureMapType LinkClosureMap;
 
+  typedef std::map<TargetConfigPair, std::vector<cmSourceFile*> >
+                                                          SourceFilesMapType;
+  SourceFilesMapType SourceFilesMap;
+
   struct TargetPropertyEntry {
     TargetPropertyEntry(cmsys::auto_ptr<cmCompiledGeneratorExpression> cge,
       const std::string &targetName = std::string())
@@ -793,42 +797,75 @@ void cmTarget::GetSourceFiles(std::vector<cmSourceFile*> &files,
                               const std::string& config,
                               cmTarget const* head) const
 {
-  std::vector<std::string> srcs;
-  this->GetSourceFiles(srcs, config, head);
 
-  std::set<cmSourceFile*> emitted;
+  // Lookup any existing link implementation for this configuration.
+  TargetConfigPair key(head, cmSystemTools::UpperCase(config));
 
-  for(std::vector<std::string>::const_iterator i = srcs.begin();
-      i != srcs.end(); ++i)
+  cmTargetInternals::SourceFilesMapType::iterator
+    it = this->Internal->SourceFilesMap.find(key);
+  if(it != this->Internal->SourceFilesMap.end())
     {
-    cmSourceFile* sf = this->Makefile->GetOrCreateSource(*i);
-    if (emitted.insert(sf).second)
+    files = it->second;
+    }
+  else
+    {
+    std::vector<std::string> srcs;
+    this->GetSourceFiles(srcs, config, head);
+
+    std::set<cmSourceFile*> emitted;
+
+    for(std::vector<std::string>::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;
     }
 }
 
 //----------------------------------------------------------------------------
 void cmTarget::AddSources(std::vector<std::string> const& srcs)
 {
+  std::string srcFiles;
+  const char* sep = "";
   for(std::vector<std::string>::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())
+    {
+    this->Internal->SourceFilesMap.clear();
+    cmListFileBacktrace lfbt;
+    this->Makefile->GetBacktrace(lfbt);
+    cmGeneratorExpression ge(lfbt);
+    cmsys::auto_ptr<cmCompiledGeneratorExpression> 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 +900,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);
 }
 
@@ -939,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);
@@ -1708,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);
@@ -1794,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);
diff --git a/Source/cmTarget.h b/Source/cmTarget.h
index da032a5..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<std::string> &files,
-                      const std::string& config,
-                      cmTarget const* head = 0) const;
   void GetSourceFiles(std::vector<cmSourceFile*> &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<std::string> &files,
+                      const std::string& config,
+                      cmTarget const* head = 0) const;
 private:
   std::string Name;
   std::vector<cmCustomCommand> PreBuildCommands;
@@ -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);