[clang][deps] Avoid unnecessary work for seen dependencies

[jansvoboda11]

Summary

Modular dependencies the client has already seen don't need to be reported again. This patch leverages that to skip somewhat expensive computation: generating the full command line and collecting the full set of file dependencies. Everything else is necessary for computation of the canonical context hash, which we cannot skip.

clang/include/clang/Tooling/DependencyScanning/DependencyScanningTool.h

public:
   void handleFileDependency(StringRef File) override {
     Dependencies.push_back(std::string(File));
   }
 
   void handlePrebuiltModuleDependency(PrebuiltModuleDep PMD) override {
     PrebuiltModuleDeps.emplace_back(std::move(PMD));
   }
 
+  bool alreadySeenModuleDependency(ModuleID ID) override {
+    return AlreadySeen.contains(ID);
+  }
+
   void handleModuleDependency(ModuleDeps MD) override {
     ClangModuleDeps[MD.ID] = std::move(MD);
   }
 
   void handleDirectModuleDependency(ModuleID ID) override {
     DirectModuleDeps.push_back(ID);
   }
 

clang/include/clang/Tooling/DependencyScanning/DependencyScanningWorker.h

public:
 
   virtual void
   handleDependencyOutputOpts(const DependencyOutputOptions &Opts) = 0;
 
   virtual void handleFileDependency(StringRef Filename) = 0;
 
   virtual void handlePrebuiltModuleDependency(PrebuiltModuleDep PMD) = 0;
 
+  virtual bool alreadySeenModuleDependency(ModuleID ID) { return false; }
+
   virtual void handleModuleDependency(ModuleDeps MD) = 0;
 
   virtual void handleDirectModuleDependency(ModuleID MD) = 0;
 
   virtual void handleContextHash(std::string Hash) = 0;
 };
 
 /// Dependency scanner callbacks that are used during scanning to influence the

clang/lib/Tooling/DependencyScanning/DependencyScanningTool.cpp

 
 TranslationUnitDeps FullDependencyConsumer::takeTranslationUnitDeps() {
   TranslationUnitDeps TU;
 
   TU.ID.ContextHash = std::move(ContextHash);
   TU.FileDeps = std::move(Dependencies);
   TU.PrebuiltModuleDeps = std::move(PrebuiltModuleDeps);
   TU.Commands = std::move(Commands);
-
-  for (auto &&M : ClangModuleDeps) {
-    auto &MD = M.second;
-    // TODO: Avoid handleModuleDependency even being called for modules
-    //   we've already seen.
-    if (AlreadySeen.count(M.first))
-      continue;
-    TU.ModuleGraph.push_back(std::move(MD));
-  }
+  TU.ModuleGraph = takeModuleGraphDeps();
   TU.ClangModuleDeps = std::move(DirectModuleDeps);
 
   return TU;
 }
 
 ModuleDepsGraph FullDependencyConsumer::takeModuleGraphDeps() {
   ModuleDepsGraph ModuleGraph;
 
-  for (auto &&M : ClangModuleDeps) {
-    auto &MD = M.second;
-    // TODO: Avoid handleModuleDependency even being called for modules
-    //   we've already seen.
-    if (AlreadySeen.count(M.first))
-      continue;
+  for (auto &&[ID, MD] : ClangModuleDeps)
     ModuleGraph.push_back(std::move(MD));
-  }
 
   return ModuleGraph;
 }
 
 CallbackActionController::~CallbackActionController() {}

clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp

void ModuleDepCollectorPP::EndOfMainFile() {
 
   MDC.Consumer.handleDependencyOutputOpts(*MDC.Opts);
 
   if (MDC.IsStdModuleP1689Format)
     MDC.Consumer.handleProvidedAndRequiredStdCXXModules(
         MDC.ProvidedStdCXXModule, MDC.RequiredStdCXXModules);
 
   for (auto &&I : MDC.ModularDeps)
+    if (!MDC.Consumer.alreadySeenModuleDependency(I.second->ID))
       MDC.Consumer.handleModuleDependency(*I.second);
 
   for (const Module *M : MDC.DirectModularDeps) {
     auto It = MDC.ModularDeps.find(M);
     // Only report direct dependencies that were successfully handled.
     if (It != MDC.ModularDeps.end())
       MDC.Consumer.handleDirectModuleDependency(MDC.ModularDeps[M]->ID);
   }
 
if (ModuleMap) {
     SmallString<128> Path = ModuleMap->getNameAsRequested();
     ModMapInfo.canonicalizeModuleMapPath(Path);
     MD.ClangModuleMapFile = std::string(Path);
   }
 
   serialization::ModuleFile *MF =
       MDC.ScanInstance.getASTReader()->getModuleManager().lookup(
           M->getASTFile());
-  MDC.ScanInstance.getASTReader()->visitInputFiles(
-      *MF, true, true, [&](const serialization::InputFile &IF, bool isSystem) {
-        // __inferred_module.map is the result of the way in which an implicit
-        // module build handles inferred modules. It adds an overlay VFS with
-        // this file in the proper directory and relies on the rest of Clang to
-        // handle it like normal. With explicitly built modules we don't need
-        // to play VFS tricks, so replace it with the correct module map.
-        if (IF.getFile()->getName().endswith("__inferred_module.map")) {
-          MDC.addFileDep(MD, ModuleMap->getName());
-          return;
-        }
-        MDC.addFileDep(MD, IF.getFile()->getName());
-      });
 
   llvm::DenseSet<const Module *> SeenDeps;
   addAllSubmodulePrebuiltDeps(M, MD, SeenDeps);
   addAllSubmoduleDeps(M, MD, SeenDeps);
   addAllAffectingClangModules(M, MD, SeenDeps);
 
   MDC.ScanInstance.getASTReader()->visitTopLevelModuleMaps(
       *MF, [&](FileEntryRef FE) {
         if (FE.getNameAsRequested().endswith("__inferred_module.map"))
           return;
         MD.ModuleMapFileDeps.emplace_back(FE.getNameAsRequested());
       });
 
   CompilerInvocation CI = MDC.makeInvocationForModuleBuildWithoutOutputs(
       MD, [&](CompilerInvocation &BuildInvocation) {
         if (MDC.OptimizeArgs)
           optimizeHeaderSearchOpts(BuildInvocation.getHeaderSearchOpts(),
                                    *MDC.ScanInstance.getASTReader(), *MF);
       });
 
   MDC.associateWithContextHash(CI, MD);
 
+  if (MDC.Consumer.alreadySeenModuleDependency(MD.ID))
+    return MD.ID;
+
   // Finish the compiler invocation. Requires dependencies and the context hash.
   MDC.addOutputPaths(CI, MD);
 
   MD.BuildArguments = CI.getCC1CommandLine();
 
+  MDC.ScanInstance.getASTReader()->visitInputFiles(
+      *MF, true, true, [&](const serialization::InputFile &IF, bool isSystem) {
+        // __inferred_module.map is the result of the way in which an implicit
+        // module build handles inferred modules. It adds an overlay VFS with
+        // this file in the proper directory and relies on the rest of Clang to
+        // handle it like normal. With explicitly built modules we don't need
+        // to play VFS tricks, so replace it with the correct module map.
+        if (IF.getFile()->getName().endswith("__inferred_module.map")) {
+          MDC.addFileDep(MD, ModuleMap->getName());
+          return;
+        }
+        MDC.addFileDep(MD, IF.getFile()->getName());
+      });
+
   return MD.ID;
 }
 
 static void forEachSubmoduleSorted(const Module *M,
                                    llvm::function_ref<void(const Module *)> F) {
   // Submodule order depends on order of header includes for inferred submodules
   // we don't care about the exact order, so sort so that it's consistent across
   // TUs to improve sharing.