Merge pull request #60830 from artemcm/NoGlobalClangDepCache

[Dependency Scanning] Do not persist cached Clang module dependencies between scans.

Author: artemcm

include/swift/AST/ModuleDependencies.h

@@ -527,7 +527,6 @@ class GlobalModuleDependenciesCache {
   GlobalModuleDependenciesCache(const GlobalModuleDependenciesCache &) = delete;
   GlobalModuleDependenciesCache &
   operator=(const GlobalModuleDependenciesCache &) = delete;
-
   virtual ~GlobalModuleDependenciesCache() {}
 
   void configureForTriple(std::string triple);
@@ -603,24 +602,18 @@ class ModuleDependenciesCache {
 private:
   GlobalModuleDependenciesCache &globalCache;
 
-  /// References to data in `globalCache` for dependencies accimulated during
+  /// References to data in `globalCache` for Swift dependencies and
+  /// `clangModuleDependencies` for Clang dependencies accimulated during
   /// the current scanning action.
   ModuleDependenciesKindRefMap ModuleDependenciesMap;
 
-  /// Additional information needed for Clang dependency scanning.
-  ClangModuleDependenciesCacheImpl *clangImpl = nullptr;
-
-  /// Name of the main Swift module of this scan
-  StringRef mainModuleName;
-  /// Underlying Clang module is seen differently by the main
-  /// module and by module clients. For this reason, we do not wish subsequent
-  /// scans to be able to re-use this dependency info and therefore we avoid
-  /// adding it to the global cache. The dependency storage is therefore tied
-  /// to this, local, cache.
-  std::unique_ptr<ModuleDependencies> underlyingClangModuleDependency = nullptr;
+  /// Discovered Clang modules are only cached locally.
+  llvm::StringMap<ModuleDependenciesVector> clangModuleDependencies;
 
   /// Function that will delete \c clangImpl properly.
   void (*clangImplDeleter)(ClangModuleDependenciesCacheImpl *) = nullptr;
+  /// Additional information needed for Clang dependency scanning.
+  ClangModuleDependenciesCacheImpl *clangImpl = nullptr;
 
   /// Free up the storage associated with the Clang implementation.
   void destroyClangImpl() {
@@ -647,8 +640,7 @@ class ModuleDependenciesCache {
                    Optional<ModuleDependenciesKind> kind) const;
 
 public:
-  ModuleDependenciesCache(GlobalModuleDependenciesCache &globalCache,
-                          StringRef mainModuleName);
+  ModuleDependenciesCache(GlobalModuleDependenciesCache &globalCache);
   ModuleDependenciesCache(const ModuleDependenciesCache &) = delete;
   ModuleDependenciesCache &operator=(const ModuleDependenciesCache &) = delete;
   virtual ~ModuleDependenciesCache() { destroyClangImpl(); }

include/swift/AST/ModuleLoader.h

@@ -19,13 +19,13 @@
 
 #include "swift/AST/Identifier.h"
 #include "swift/AST/Import.h"
-#include "swift/AST/ModuleDependencies.h"
 #include "swift/Basic/ArrayRefView.h"
 #include "swift/Basic/Fingerprint.h"
 #include "swift/Basic/LLVM.h"
 #include "swift/Basic/Located.h"
 #include "swift/Basic/SourceLoc.h"
 #include "llvm/ADT/SetVector.h"
+#include "llvm/ADT/StringSet.h"
 #include "llvm/ADT/TinyPtrVector.h"
 #include "llvm/Support/VersionTuple.h"
 #include <system_error>
@@ -52,6 +52,7 @@ class NominalTypeDecl;
 class SourceFile;
 class TypeDecl;
 class CompilerInstance;
+enum class ModuleDependenciesKind : int8_t;
 
 enum class KnownProtocolKind : uint8_t;
 

include/swift/Sema/SourceLoader.h

@@ -13,7 +13,6 @@
 #ifndef SWIFT_SEMA_SOURCELOADER_H
 #define SWIFT_SEMA_SOURCELOADER_H
 
-#include "swift/AST/ModuleDependencies.h"
 #include "swift/AST/ModuleLoader.h"
 
 namespace swift {
@@ -94,14 +93,10 @@ class SourceLoader : public ModuleLoader {
     // Parsing populates the Objective-C method tables.
   }
 
-  Optional<ModuleDependencies> getModuleDependencies(
-      StringRef moduleName, ModuleDependenciesCache &cache,
-      InterfaceSubContextDelegate &delegate) override {
-    // FIXME: Implement?
-    return None;
-  }
+  Optional<ModuleDependencies>
+  getModuleDependencies(StringRef moduleName, ModuleDependenciesCache &cache,
+                        InterfaceSubContextDelegate &delegate) override;
 };
-
 }
 
 #endif

lib/AST/ModuleDependencies.cpp

@@ -388,6 +388,9 @@ GlobalModuleDependenciesCache::findSourceModuleDependency(
 
 bool GlobalModuleDependenciesCache::hasDependencies(
     StringRef moduleName, ModuleLookupSpecifics details) const {
+  assert(details.kind != ModuleDependenciesKind::Clang &&
+         "Attempting to query Clang dependency in persistent Dependency "
+         "Scanner Cache.");
   return findDependencies(moduleName, details).hasValue();
 }
 
@@ -397,6 +400,9 @@ GlobalModuleDependenciesCache::findAllDependenciesIrrespectiveOfSearchPaths(
   if (!kind) {
     for (auto kind = ModuleDependenciesKind::FirstKind;
          kind != ModuleDependenciesKind::LastKind; ++kind) {
+      if (kind == ModuleDependenciesKind::Clang)
+        continue;
+
       auto deps =
           findAllDependenciesIrrespectiveOfSearchPaths(moduleName, kind);
       if (deps.hasValue())
@@ -446,6 +452,10 @@ static std::string modulePathForVerification(const ModuleDependencies &module) {
 const ModuleDependencies *GlobalModuleDependenciesCache::recordDependencies(
     StringRef moduleName, ModuleDependencies dependencies) {
   auto kind = dependencies.getKind();
+  assert(kind != ModuleDependenciesKind::Clang &&
+         "Attempting to cache Clang dependency in persistent Dependency "
+         "Scanner Cache.");
+
   // Source-based dependencies are recorded independently of the invocation's
   // target triple.
   if (kind == swift::ModuleDependenciesKind::SwiftSource) {
@@ -482,6 +492,10 @@ const ModuleDependencies *GlobalModuleDependenciesCache::recordDependencies(
 const ModuleDependencies *GlobalModuleDependenciesCache::updateDependencies(
     ModuleDependencyID moduleID, ModuleDependencies dependencies) {
   auto kind = dependencies.getKind();
+  assert(kind != ModuleDependenciesKind::Clang &&
+         "Attempting to update Clang dependency in persistent Dependency "
+         "Scanner Cache.");
+
   // Source-based dependencies
   if (kind == swift::ModuleDependenciesKind::SwiftSource) {
     assert(SwiftSourceModuleDependenciesMap.count(moduleID.first) == 1 &&
@@ -517,9 +531,8 @@ ModuleDependenciesCache::getDependencyReferencesMap(
 }
 
 ModuleDependenciesCache::ModuleDependenciesCache(
-    GlobalModuleDependenciesCache &globalCache,
-    StringRef mainModuleName)
-    : globalCache(globalCache), mainModuleName(mainModuleName) {
+    GlobalModuleDependenciesCache &globalCache)
+    : globalCache(globalCache) {
   for (auto kind = ModuleDependenciesKind::FirstKind;
        kind != ModuleDependenciesKind::LastKind; ++kind) {
     ModuleDependenciesMap.insert(
@@ -572,12 +585,29 @@ void ModuleDependenciesCache::recordDependencies(
   // The underlying Clang module needs to be cached in this invocation,
   // but should not make it to the global cache since it will look slightly
   // differently for clients of this module than it does for the module itself.
-  const ModuleDependencies* recordedDependencies;
-  if (moduleName == mainModuleName && dependencies.isClangModule()) {
-    underlyingClangModuleDependency = std::make_unique<ModuleDependencies>(std::move(dependencies));
-    recordedDependencies = underlyingClangModuleDependency.get();
+  const ModuleDependencies *recordedDependencies;
+  if (dependencies.getKind() == ModuleDependenciesKind::Clang) {
+    auto *clangDep = dependencies.getAsClangModule();
+    assert(clangDep && "Unexpected NULL Clang dependency.");
+    // Cache may already have a dependency for this module
+    if (clangModuleDependencies.count(moduleName) != 0) {
+      // Do not record duplicate dependencies.
+      auto newModulePath = clangDep->moduleMapFile;
+      for (auto &existingDeps : clangModuleDependencies[moduleName]) {
+        if (modulePathForVerification(existingDeps) == newModulePath)
+          return;
+      }
+      clangModuleDependencies[moduleName].emplace_back(std::move(dependencies));
+      recordedDependencies = clangModuleDependencies[moduleName].end() - 1;
+    } else {
+      clangModuleDependencies.insert(
+          {moduleName, ModuleDependenciesVector{std::move(dependencies)}});
+      recordedDependencies = &(clangModuleDependencies[moduleName].front());
+    }
+
   } else
-    recordedDependencies = globalCache.recordDependencies(moduleName, dependencies);
+    recordedDependencies =
+        globalCache.recordDependencies(moduleName, dependencies);
 
   auto &map = getDependencyReferencesMap(dependenciesKind);
   assert(map.count(moduleName) == 0 && "Already added to map");

lib/AST/ModuleLoader.cpp

@@ -18,6 +18,7 @@
 #include "swift/AST/DiagnosticsCommon.h"
 #include "swift/AST/FileUnit.h"
 #include "swift/AST/ModuleLoader.h"
+#include "swift/AST/ModuleDependencies.h"
 #include "swift/Basic/FileTypes.h"
 #include "swift/Basic/Platform.h"
 #include "swift/Basic/SourceManager.h"

lib/ClangImporter/ImporterImpl.h

@@ -53,6 +53,7 @@
 #include <functional>
 #include <set>
 #include <unordered_set>
+#include <unordered_map>
 #include <vector>
 
 namespace llvm {

lib/DependencyScan/DependencyScanningTool.cpp

@@ -73,8 +73,7 @@ DependencyScanningTool::getDependencies(
   auto Instance = std::move(*InstanceOrErr);
 
   // Local scan cache instance, wrapping the shared global cache.
-  ModuleDependenciesCache cache(*SharedCache,
-                                Instance->getMainModule()->getNameStr());
+  ModuleDependenciesCache cache(*SharedCache);
   // Execute the scanning action, retrieving the in-memory result
   auto DependenciesOrErr = performModuleScan(*Instance.get(), cache);
   if (DependenciesOrErr.getError())
@@ -114,8 +113,7 @@ DependencyScanningTool::getDependencies(
   auto Instance = std::move(*InstanceOrErr);
 
   // Local scan cache instance, wrapping the shared global cache.
-  ModuleDependenciesCache cache(*SharedCache,
-                                Instance->getMainModule()->getNameStr());
+  ModuleDependenciesCache cache(*SharedCache);
   auto BatchScanResults = performBatchModuleScan(
       *Instance.get(), cache, VersionedPCMInstanceCacheCache.get(),
       Saver, BatchInput);

lib/DependencyScan/ScanDependencies.cpp

@@ -1109,7 +1109,7 @@ forEachBatchEntry(CompilerInstance &invocationInstance,
       newGlobalCache->configureForTriple(
           newInstance->getInvocation().getLangOptions().Target.str());
       auto newLocalCache = std::make_unique<ModuleDependenciesCache>(
-          *newGlobalCache, newInstance->getMainModule()->getNameStr());
+          *newGlobalCache);
       pInstance = newInstance.get();
       pCache = newLocalCache.get();
       subInstanceMap->insert(
@@ -1253,8 +1253,7 @@ bool swift::dependencies::scanDependencies(CompilerInstance &instance) {
   if (opts.ReuseDependencyScannerCache)
     deserializeDependencyCache(instance, globalCache);
 
-  ModuleDependenciesCache cache(globalCache,
-                                instance.getMainModule()->getNameStr());
+  ModuleDependenciesCache cache(globalCache);
 
   // Execute scan
   auto dependenciesOrErr = performModuleScan(instance, cache);
@@ -1289,8 +1288,7 @@ bool swift::dependencies::prescanDependencies(CompilerInstance &instance) {
   GlobalModuleDependenciesCache singleUseGlobalCache;
   singleUseGlobalCache.configureForTriple(instance.getInvocation()
                                                   .getLangOptions().Target.str());
-  ModuleDependenciesCache cache(singleUseGlobalCache,
-                                instance.getMainModule()->getNameStr());
+  ModuleDependenciesCache cache(singleUseGlobalCache);
   if (out.has_error() || EC) {
     Context.Diags.diagnose(SourceLoc(), diag::error_opening_output, path,
                            EC.message());
@@ -1321,8 +1319,7 @@ bool swift::dependencies::batchScanDependencies(
   GlobalModuleDependenciesCache singleUseGlobalCache;
   singleUseGlobalCache.configureForTriple(instance.getInvocation()
                                                   .getLangOptions().Target.str());
-  ModuleDependenciesCache cache(singleUseGlobalCache,
-                                instance.getMainModule()->getNameStr());
+  ModuleDependenciesCache cache(singleUseGlobalCache);
   (void)instance.getMainModule();
   llvm::BumpPtrAllocator alloc;
   llvm::StringSaver saver(alloc);
@@ -1357,8 +1354,7 @@ bool swift::dependencies::batchPrescanDependencies(
   GlobalModuleDependenciesCache singleUseGlobalCache;
   singleUseGlobalCache.configureForTriple(instance.getInvocation()
                                                   .getLangOptions().Target.str());
-  ModuleDependenciesCache cache(singleUseGlobalCache,
-                                instance.getMainModule()->getNameStr());
+  ModuleDependenciesCache cache(singleUseGlobalCache);
   (void)instance.getMainModule();
   llvm::BumpPtrAllocator alloc;
   llvm::StringSaver saver(alloc);

lib/Frontend/Frontend.cpp

@@ -21,6 +21,7 @@
 #include "swift/AST/DiagnosticsSema.h"
 #include "swift/AST/FileSystem.h"
 #include "swift/AST/Module.h"
+#include "swift/AST/ModuleDependencies.h"
 #include "swift/AST/TypeCheckRequests.h"
 #include "swift/Basic/FileTypes.h"
 #include "swift/Basic/SourceManager.h"

lib/Sema/SourceLoader.cpp

@@ -20,6 +20,7 @@
 #include "swift/AST/ASTContext.h"
 #include "swift/AST/DiagnosticsSema.h"
 #include "swift/AST/Module.h"
+#include "swift/AST/ModuleDependencies.h"
 #include "swift/AST/SourceFile.h"
 #include "swift/Parse/PersistentParserState.h"
 #include "swift/Basic/SourceManager.h"
@@ -149,3 +150,11 @@ void SourceLoader::loadExtensions(NominalTypeDecl *nominal,
   // Type-checking the source automatically loads all extensions; there's
   // nothing to do here.
 }
+
+Optional<ModuleDependencies>
+SourceLoader::getModuleDependencies(StringRef moduleName,
+                                    ModuleDependenciesCache &cache,
+                                    InterfaceSubContextDelegate &delegate) {
+  // FIXME: Implement?
+  return None;
+}