[Dependency Scanning] Split the ModuleDependencyCache into two: current scan cache & global.

This change causes the cache to be layered with a local "cache" that wraps the global cache, which will serve as the source of truth. The local cache persists only for the duration of a given scanning action, and has a store of references to dependencies resolved as a part of the current scanning action only, while the global cache is the one that persists across scanning actions (e.g. in `DependencyScanningTool`) and stores actual module dependency info values.

Only the local cache can answer dependency lookup queries, checking current scanning action results first, before falling back to querying the global cache, with queries disambiguated by the current scannning action's search paths, ensuring we never resolve a dependency lookup query with a module info that could not be found in the current action's search paths.

This change is required because search-path disambiguation can lead to false-negatives: for example, the Clang dependency scanner may find modules relative to the compiler's path that are not on the compiler's direct search paths. While such false-negative query responses should be functionally safe, we rely on the current scanning action's results being always-present-in-the-cache for the scanner's functionality. This layering ensures that the cache use-sites remain unchanged and that we get both: preserved global state which can be queried disambiguated with the search path details, and an always-consistent local (current action) cache state.

Author: artemcm

include/swift/AST/ModuleDependencies.h

@@ -25,6 +25,7 @@
 #include "llvm/ADT/StringSet.h"
 #include <string>
 #include <vector>
+#include <unordered_map>
 
 namespace swift {
 
@@ -35,7 +36,8 @@ class Identifier;
 
 /// Which kind of module dependencies we are looking for.
 enum class ModuleDependenciesKind : int8_t {
-  SwiftTextual,
+  FirstKind,
+  SwiftTextual = FirstKind,
   SwiftBinary,
   // Placeholder dependencies are a kind of dependencies used only by the
   // dependency scanner. They are swift modules that the scanner will not be
@@ -61,6 +63,14 @@ enum class ModuleDependenciesKind : int8_t {
   // of all targets, individually, have been computed.
   SwiftPlaceholder,
   Clang,
+  LastKind = Clang + 1
+};
+
+struct ModuleDependenciesKindHash {
+  std::size_t operator()(ModuleDependenciesKind k) const {
+    using UnderlyingType = std::underlying_type<ModuleDependenciesKind>::type;
+    return std::hash<UnderlyingType>{}(static_cast<UnderlyingType>(k));
+  }
 };
 
 /// Details of a given module used for dependency scanner cache queries.
@@ -409,29 +419,30 @@ class ModuleDependencies {
 };
 
 using ModuleDependencyID = std::pair<std::string, ModuleDependenciesKind>;
+using ModuleDependenciesVector = llvm::SmallVector<ModuleDependencies, 1>;
 
 /// A cache describing the set of module dependencies that has been queried
-/// thus far.
-class ModuleDependenciesCache {
-  using ModuleDependenciesVector = llvm::SmallVector<ModuleDependencies, 1>;
-
+/// thus far. This cache records/stores the actual Dependency values and can be
+/// preserved across different scanning actions (e.g. in
+/// `DependencyScanningTool`). It is not to be queried directly, but is rather
+/// meant to be wrapped in an instance of `ModuleDependenciesCache`, responsible
+/// for recording new dependencies and answering cache queries in a given scan.
+/// Queries to this cache must be disambiguated with a set of search paths to
+/// ensure that the returned cached dependency was one that can be found in the
+/// current scanning action's filesystem view.
+class GlobalModuleDependenciesCache {
   /// All cached module dependencies, in the order in which they were
   /// encountered.
   std::vector<ModuleDependencyID> AllModules;
 
-  /// Dependencies for Textual Swift modules that have already been computed.
-  /// This maps a module's id (name, kind) to a vector of Dependency objects, which correspond
-  /// to instances of the same module that may have been found in different sets of search paths.
-  llvm::StringMap<ModuleDependenciesVector> SwiftTextualModuleDependencies;
-
-  /// Dependencies for Binary Swift modules that have already been computed.
-  llvm::StringMap<ModuleDependenciesVector> SwiftBinaryModuleDependencies;
-
-  /// Dependencies for Swift placeholder dependency modules that have already been computed.
-  llvm::StringMap<ModuleDependenciesVector> SwiftPlaceholderModuleDependencies;
-
-  /// Dependencies for Clang modules that have already been computed.
-  llvm::StringMap<ModuleDependenciesVector> ClangModuleDependencies;
+  /// Dependencies for modules that have already been computed.
+  /// This maps a dependency kind to a map of a module's name to a vector of Dependency objects,
+  /// which correspond to instances of the same module that may have been found
+  /// in different sets of search paths.
+  std::unordered_map<ModuleDependenciesKind,
+                     llvm::StringMap<ModuleDependenciesVector>,
+                     ModuleDependenciesKindHash>
+      ModuleDependenciesKindMap;
 
   /// Additional information needed for Clang dependency scanning.
   ClangModuleDependenciesCacheImpl *clangImpl = nullptr;
@@ -447,55 +458,148 @@ class ModuleDependenciesCache {
 
   /// Retrieve the dependencies map that corresponds to the given dependency
   /// kind.
-  llvm::StringMap<ModuleDependenciesVector> &getDependenciesMap(
-      ModuleDependenciesKind kind);
-  const llvm::StringMap<ModuleDependenciesVector> &getDependenciesMap(
-      ModuleDependenciesKind kind) const;
+  llvm::StringMap<ModuleDependenciesVector> &
+  getDependenciesMap(ModuleDependenciesKind kind);
+  const llvm::StringMap<ModuleDependenciesVector> &
+  getDependenciesMap(ModuleDependenciesKind kind) const;
 
 public:
-  ModuleDependenciesCache() { }
+  GlobalModuleDependenciesCache();
+  GlobalModuleDependenciesCache(const GlobalModuleDependenciesCache &) = delete;
+  GlobalModuleDependenciesCache &
+  operator=(const GlobalModuleDependenciesCache &) = delete;
 
-  ModuleDependenciesCache(const ModuleDependenciesCache &) = delete;
-  ModuleDependenciesCache &operator=(const ModuleDependenciesCache &) = delete;
+  virtual ~GlobalModuleDependenciesCache() { destroyClangImpl(); }
 
-  ~ModuleDependenciesCache() {
-    destroyClangImpl();
-  }
+private:
+  /// Enforce clients not being allowed to query this cache directly, it must be
+  /// wrapped in an instance of `ModuleDependenciesCache`.
+  friend class ModuleDependenciesCache;
 
   /// Set the Clang-specific implementation data.
-  void setClangImpl(
-      ClangModuleDependenciesCacheImpl *clangImpl,
-      void (*clangImplDeleter)(ClangModuleDependenciesCacheImpl *)) {
+  virtual void
+  setClangImpl(ClangModuleDependenciesCacheImpl *clangImpl,
+               void (*clangImplDeleter)(ClangModuleDependenciesCacheImpl *)) {
     destroyClangImpl();
 
     this->clangImpl = clangImpl;
     this->clangImplDeleter = clangImplDeleter;
   }
 
+  /// Retrieve the Clang-specific implementation data;
+  ClangModuleDependenciesCacheImpl *getClangImpl() const { return clangImpl; }
+
+  /// Whether we have cached dependency information for the given module.
+  bool hasDependencies(StringRef moduleName,
+                       ModuleLookupSpecifics details) const;
+
+  /// Look for module dependencies for a module with the given name given
+  /// current search paths.
+  ///
+  /// \returns the cached result, or \c None if there is no cached entry.
+  Optional<ModuleDependencies>
+  findDependencies(StringRef moduleName, ModuleLookupSpecifics details) const;
+
+public:
+  /// Look for module dependencies for a module with the given name.
+  /// This method has a deliberately-obtuse name to indicate that it is not to
+  /// be used for general queries.
+  ///
+  /// \returns the cached result, or \c None if there is no cached entry.
+  Optional<ModuleDependenciesVector>
+  findAllDependenciesIrrespectiveOfSearchPaths(
+      StringRef moduleName, Optional<ModuleDependenciesKind> kind) const;
+
+  /// Record dependencies for the given module.
+  const ModuleDependencies *recordDependencies(StringRef moduleName,
+                                               ModuleDependencies dependencies);
+
+  /// Update stored dependencies for the given module.
+  const ModuleDependencies *updateDependencies(ModuleDependencyID moduleID,
+                                               ModuleDependencies dependencies);
+
+  /// Reference the list of all module dependencies.
+  const std::vector<ModuleDependencyID> &getAllModules() const {
+    return AllModules;
+  }
+};
+
+/// This "local" dependencies cache persists only for the duration of a given
+/// scanning action, and wraps an instance of a `GlobalModuleDependenciesCache`
+/// which may carry cached scanning information from prior scanning actions.
+/// This cache maintains a store of references to all dependencies found within
+/// the current scanning action (with their values stored in the global Cache),
+/// since these do not require clients to disambiguate them with search paths.
+class ModuleDependenciesCache {
+private:
+  GlobalModuleDependenciesCache &globalCache;
+
+  /// References to data in `globalCache` for dependencies accimulated during
+  /// the current scanning action.
+  std::unordered_map<ModuleDependenciesKind,
+                     llvm::StringMap<const ModuleDependencies *>,
+                     ModuleDependenciesKindHash>
+      ModuleDependenciesKindMap;
+
+  /// Retrieve the dependencies map that corresponds to the given dependency
+  /// kind.
+  llvm::StringMap<const ModuleDependencies *> &
+  getDependencyReferencesMap(ModuleDependenciesKind kind);
+  const llvm::StringMap<const ModuleDependencies *> &
+  getDependencyReferencesMap(ModuleDependenciesKind kind) const;
+
+  /// Local cache results lookup, only for modules which were discovered during
+  /// the current scanner invocation.
+  bool hasDependencies(StringRef moduleName,
+                       Optional<ModuleDependenciesKind> kind) const;
+
+  /// Local cache results lookup, only for modules which were discovered during
+  /// the current scanner invocation.
+  Optional<const ModuleDependencies *>
+  findDependencies(StringRef moduleName,
+                   Optional<ModuleDependenciesKind> kind) const;
+
+public:
+  ModuleDependenciesCache(GlobalModuleDependenciesCache &globalCache);
+  ModuleDependenciesCache(const ModuleDependenciesCache &) = delete;
+  ModuleDependenciesCache &operator=(const ModuleDependenciesCache &) = delete;
+  virtual ~ModuleDependenciesCache() {}
+
+public:
+  /// Set the Clang-specific implementation data.
+  void
+  setClangImpl(ClangModuleDependenciesCacheImpl *clangImpl,
+               void (*clangImplDeleter)(ClangModuleDependenciesCacheImpl *)) {
+    globalCache.setClangImpl(clangImpl, clangImplDeleter);
+  }
+
   /// Retrieve the Clang-specific implementation data;
   ClangModuleDependenciesCacheImpl *getClangImpl() const {
-    return clangImpl;
+    return globalCache.getClangImpl();
   }
 
   /// Whether we have cached dependency information for the given module.
   bool hasDependencies(StringRef moduleName,
                        ModuleLookupSpecifics details) const;
 
-  /// Look for module dependencies for a module with the given name given current search paths.
+  /// Look for module dependencies for a module with the given name given
+  /// current search paths.
   ///
   /// \returns the cached result, or \c None if there is no cached entry.
-  Optional<ModuleDependencies> findDependencies(
-      StringRef moduleName,
-      ModuleLookupSpecifics details) const;
+  Optional<ModuleDependencies>
+  findDependencies(StringRef moduleName, ModuleLookupSpecifics details) const;
 
   /// Look for module dependencies for a module with the given name.
-  /// This method has a deliberately-obtuse name to indicate that it is not to be used for general
-  /// queries.
+  /// This method has a deliberately-obtuse name to indicate that it is not to
+  /// be used for general queries.
   ///
   /// \returns the cached result, or \c None if there is no cached entry.
-  Optional<ModuleDependenciesVector> findAllDependenciesIrrespectiveOfSearchPaths(
-      StringRef moduleName,
-      Optional<ModuleDependenciesKind> kind) const;
+  Optional<ModuleDependenciesVector>
+  findAllDependenciesIrrespectiveOfSearchPaths(
+      StringRef moduleName, Optional<ModuleDependenciesKind> kind) const {
+    return globalCache.findAllDependenciesIrrespectiveOfSearchPaths(moduleName,
+                                                                    kind);
+  }
 
   /// Record dependencies for the given module.
   void recordDependencies(StringRef moduleName,
@@ -507,10 +611,10 @@ class ModuleDependenciesCache {
 
   /// Reference the list of all module dependencies.
   const std::vector<ModuleDependencyID> &getAllModules() const {
-    return AllModules;
+    return globalCache.getAllModules();
   }
 };
 
-}
+} // namespace swift
 
 #endif /* SWIFT_AST_MODULE_DEPENDENCIES_H */

include/swift/DependencyScan/DependencyScanningTool.h

@@ -59,7 +59,7 @@ class DependencyScanningTool {
 
   /// Writes the current `SharedCache` instance to a specified FileSystem path.
   void serializeCache(llvm::StringRef path);
-  /// Loads an instance of a `ModuleDependenciesCache` to serve as the `SharedCache`
+  /// Loads an instance of a `GlobalModuleDependenciesCache` to serve as the `SharedCache`
   /// from a specified FileSystem path.
   bool loadCache(llvm::StringRef path);
   /// Discard the tool's current `SharedCache` and start anew.
@@ -73,7 +73,7 @@ class DependencyScanningTool {
 
   /// Shared cache of module dependencies, re-used by individual full-scan queries
   /// during the lifetime of this Tool.
-  std::unique_ptr<ModuleDependenciesCache> SharedCache;
+  std::unique_ptr<GlobalModuleDependenciesCache> SharedCache;
 
   /// Shared cache of compiler instances created during batch scanning, corresponding to
   /// command-line options specified in the batch scan input entry.

include/swift/DependencyScan/ScanDependencies.h

@@ -27,12 +27,18 @@ namespace swift {
 class CompilerInvocation;
 class CompilerInstance;
 class ModuleDependenciesCache;
+class GlobalModuleDependenciesCache;
 
 namespace dependencies {
 
+//using CompilerArgInstanceCacheMap =
+//    llvm::StringMap<std::pair<std::unique_ptr<CompilerInstance>,
+//                              std::unique_ptr<ModuleDependenciesCache>>>;
+
 using CompilerArgInstanceCacheMap =
-    llvm::StringMap<std::pair<std::unique_ptr<CompilerInstance>,
-                              std::unique_ptr<ModuleDependenciesCache>>>;
+    llvm::StringMap<std::tuple<std::unique_ptr<CompilerInstance>,
+                               std::unique_ptr<GlobalModuleDependenciesCache>,
+                               std::unique_ptr<ModuleDependenciesCache>>>;
 
 struct BatchScanInput {
   llvm::StringRef moduleName;

include/swift/DependencyScan/SerializedModuleDependencyCacheFormat.h

@@ -23,7 +23,7 @@ class MemoryBuffer;
 namespace swift {
 
 class DiagnosticEngine;
-class ModuleDependenciesCache;
+class GlobalModuleDependenciesCache;
 
 namespace dependencies {
 namespace module_dependency_cache_serialization {
@@ -160,23 +160,23 @@ using ClangModuleDetailsLayout =
 /// Tries to read the dependency graph from the given buffer.
 /// Returns \c true if there was an error.
 bool readInterModuleDependenciesCache(llvm::MemoryBuffer &buffer,
-                                      ModuleDependenciesCache &cache);
+                                      GlobalModuleDependenciesCache &cache);
 
 /// Tries to read the dependency graph from the given path name.
 /// Returns true if there was an error.
 bool readInterModuleDependenciesCache(llvm::StringRef path,
-                                      ModuleDependenciesCache &cache);
+                                      GlobalModuleDependenciesCache &cache);
 
 /// Tries to write the dependency graph to the given path name.
 /// Returns true if there was an error.
 bool writeInterModuleDependenciesCache(DiagnosticEngine &diags,
                                        llvm::StringRef path,
-                                       const ModuleDependenciesCache &cache);
+                                       const GlobalModuleDependenciesCache &cache);
 
 /// Tries to write out the given dependency cache with the given
 /// bitstream writer.
 void writeInterModuleDependenciesCache(llvm::BitstreamWriter &Out,
-                                       const ModuleDependenciesCache &cache);
+                                       const GlobalModuleDependenciesCache &cache);
 
 } // end namespace module_dependency_cache_serialization
 } // end namespace dependencies

include/swift/Frontend/FrontendOptions.h

@@ -309,10 +309,6 @@ class FrontendOptions {
   /// Emit remarks indicating use of the serialized module dependency scanning cache
   bool EmitDependencyScannerCacheRemarks = false;
 
-  /// After performing a dependency scanning action, serialize and deserialize the
-  /// dependency cache before producing the result.
-  bool TestDependencyScannerCacheSerialization = false;
-
   /// When performing an incremental build, ensure that cross-module incremental
   /// build metadata is available in any swift modules emitted by this frontend
   /// job.