cmGlobalNinjaGenerator: Optimize target depends closure

Rewrite AppendTargetDependsClosure method to only cache local target
outputs, not including outputs from dependencies.

Caching all recursive target outputs causes much time to be spent
merging sets that have many elements in common (from targets that are
included through multiple dependency paths).
It is faster to always iterate over all dependencies instead.

Source/cmGlobalNinjaGenerator.cxx

@@ -8,6 +8,7 @@
 #include <cstdio>
 #include <functional>
 #include <sstream>
+#include <tuple>
 #include <utility>
 
 #include <cm/iterator>
@@ -584,7 +585,7 @@ void cmGlobalNinjaGenerator::Generate()
   }
 
   for (auto& it : this->Configs) {
-    it.second.TargetDependsClosures.clear();
+    it.second.TargetDependsClosureLocalOutputs.clear();
   }
 
   this->InitOutputPathPrefix();
@@ -1360,70 +1361,85 @@ void cmGlobalNinjaGenerator::AppendTargetDependsClosure(
   cmGeneratorTarget const* target, cmNinjaDeps& outputs,
   const std::string& config, const std::string& fileConfig, bool genexOutput)
 {
-  cmNinjaOuts outs;
+  struct Entry
-  this->AppendTargetDependsClosure(target, outs, config, fileConfig,
+  {
-                                   genexOutput, true);
+    Entry(cmGeneratorTarget const* target_, std::string config_,
-  cm::append(outputs, outs);
+          std::string fileConfig_)
-}
+      : target(target_)
+      , config(std::move(config_))
+      , fileConfig(std::move(fileConfig_))
+    {
+    }
 
-void cmGlobalNinjaGenerator::AppendTargetDependsClosure(
+    bool operator<(Entry const& other) const
-  cmGeneratorTarget const* target, cmNinjaOuts& outputs,
+    {
-  const std::string& config, const std::string& fileConfig, bool genexOutput,
+      return std::tie(target, config, fileConfig) <
-  bool omit_self)
+        std::tie(other.target, other.config, other.fileConfig);
-{
+    }
 
-  // try to locate the target in the cache
+    cmGeneratorTarget const* target;
-  ByConfig::TargetDependsClosureKey key{
+    std::string config;
-    target,
+    std::string fileConfig;
-    config,
-    genexOutput,
   };
-  auto find = this->Configs[fileConfig].TargetDependsClosures.lower_bound(key);
 
-  if (find == this->Configs[fileConfig].TargetDependsClosures.end() ||
+  cmNinjaOuts outputSet;
-      find->first != key) {
+  std::vector<Entry> stack;
-    // We now calculate the closure outputs by inspecting the dependent
+  stack.emplace_back(target, config, fileConfig);
-    // targets recursively.
+  std::set<Entry> seen = { stack.back() };
-    // For that we have to distinguish between a local result set that is only
-    // relevant for filling the cache entries properly isolated and a global
-    // result set that is relevant for the result of the top level call to
-    // AppendTargetDependsClosure.
-    cmNinjaOuts this_outs; // this will be the new cache entry
 
-    for (auto const& dep_target : this->GetTargetDirectDepends(target)) {
+  do {
+    Entry entry = std::move(stack.back());
+    stack.pop_back();
+
+    // generate the outputs of the target itself, if applicable
+    if (entry.target != target) {
+      // try to locate the target in the cache
+      ByConfig::TargetDependsClosureKey localCacheKey{
+        entry.target,
+        entry.config,
+        genexOutput,
+      };
+      auto& configs = this->Configs[entry.fileConfig];
+      auto lb =
+        configs.TargetDependsClosureLocalOutputs.lower_bound(localCacheKey);
+
+      if (lb == configs.TargetDependsClosureLocalOutputs.end() ||
+          lb->first != localCacheKey) {
+        cmNinjaDeps outs;
+        this->AppendTargetOutputs(entry.target, outs, entry.config,
+                                  DependOnTargetArtifact);
+        configs.TargetDependsClosureLocalOutputs.emplace_hint(
+          lb, localCacheKey, outs);
+        for (auto& value : outs) {
+          outputSet.emplace(std::move(value));
+        }
+      } else {
+        outputSet.insert(lb->second.begin(), lb->second.end());
+      }
+    }
+
+    // push next dependencies
+    for (const auto& dep_target : this->GetTargetDirectDepends(entry.target)) {
       if (!dep_target->IsInBuildSystem()) {
         continue;
       }
 
-      if (!this->IsSingleConfigUtility(target) &&
+      if (!this->IsSingleConfigUtility(entry.target) &&
           !this->IsSingleConfigUtility(dep_target) &&
           this->EnableCrossConfigBuild() && !dep_target.IsCross() &&
           !genexOutput) {
         continue;
       }
 
-      if (dep_target.IsCross()) {
+      auto emplaceRes = seen.emplace(
-        this->AppendTargetDependsClosure(dep_target, this_outs, fileConfig,
+        dep_target, dep_target.IsCross() ? entry.fileConfig : entry.config,
-                                         fileConfig, genexOutput, false);
+        entry.fileConfig);
-      } else {
+      if (emplaceRes.second) {
-        this->AppendTargetDependsClosure(dep_target, this_outs, config,
+        stack.emplace_back(*emplaceRes.first);
-                                         fileConfig, genexOutput, false);
       }
     }
-    find = this->Configs[fileConfig].TargetDependsClosures.emplace_hint(
+  } while (!stack.empty());
-      find, key, std::move(this_outs));
+  cm::append(outputs, outputSet);
-  }
-
-  // now fill the outputs of the final result from the newly generated cache
-  // entry
-  outputs.insert(find->second.begin(), find->second.end());
-
-  // finally generate the outputs of the target itself, if applicable
-  cmNinjaDeps outs;
-  if (!omit_self) {
-    this->AppendTargetOutputs(target, outs, config, DependOnTargetArtifact);
-  }
-  outputs.insert(outs.begin(), outs.end());
 }
 
 void cmGlobalNinjaGenerator::AddTargetAlias(const std::string& alias,

Source/cmGlobalNinjaGenerator.h

@@ -354,11 +354,6 @@ public:
                                   const std::string& config,
                                   const std::string& fileConfig,
                                   bool genexOutput);
-  void AppendTargetDependsClosure(cmGeneratorTarget const* target,
-                                  cmNinjaOuts& outputs,
-                                  const std::string& config,
-                                  const std::string& fileConfig,
-                                  bool genexOutput, bool omit_self);
 
   void AppendDirectoryForConfig(const std::string& prefix,
                                 const std::string& config,
@@ -615,7 +610,8 @@ private:
       bool GenexOutput;
     };
 
-    std::map<TargetDependsClosureKey, cmNinjaOuts> TargetDependsClosures;
+    std::map<TargetDependsClosureKey, cmNinjaDeps>
+      TargetDependsClosureLocalOutputs;
 
     TargetAliasMap TargetAliases;