Merge pull request #681 from CodaFi/depshire

Revert #664

Author: varungandhi-apple

Sources/SwiftDriver/IncrementalCompilation/IncrementalDependencyAndInputSetup.swift

@@ -226,7 +226,7 @@ extension IncrementalCompilationState.IncrementalDependencyAndInputSetup {
     else {
       return buildInitialGraphFromSwiftDepsAndCollectInputsInvalidatedByChangedExternals()
     }
-    guard graph.populateInputDependencySourceMap(for: .inputsAddedSincePriors) else {
+    guard graph.populateInputDependencySourceMap() else {
       return nil
     }
     graph.dotFileWriter?.write(graph)
@@ -256,8 +256,7 @@ extension IncrementalCompilationState.IncrementalDependencyAndInputSetup {
   {
     let graph = ModuleDependencyGraph(self, .buildingWithoutAPrior)
     assert(outputFileMap.onlySourceFilesHaveSwiftDeps())
-    
-    guard graph.populateInputDependencySourceMap(for: .buildingFromSwiftDeps) else {
+    guard graph.populateInputDependencySourceMap() else {
       return nil
     }
 

Sources/SwiftDriver/IncrementalCompilation/ModuleDependencyGraph.swift

@@ -24,7 +24,19 @@ import SwiftOptions
   @_spi(Testing) public var nodeFinder = NodeFinder()
 
   /// Maps input files (e.g. .swift) to and from the DependencySource object.
-  @_spi(Testing) public private(set) var inputDependencySourceMap = InputDependencySourceMap()
+  ///
+  // FIXME: The map between swiftdeps and swift files is absolutely *not*
+  // a bijection. In particular, more than one swiftdeps file can be encountered
+  // in the course of deserializing priors *and* reading the output file map
+  // *and* re-reading swiftdeps files after frontends complete
+  // that correspond to the same swift file. These cause two problems:
+  // - overwrites in this data structure that lose data and
+  // - cache misses in `getInput(for:)` that cause the incremental build to
+  // turn over when e.g. entries in the output file map change. This should be
+  // replaced by a multi-map from swift files to dependency sources,
+  // and a regular map from dependency sources to swift files -
+  // since that direction really is one-to-one.
+  @_spi(Testing) public private(set) var inputDependencySourceMap = BidirectionalMap<TypedVirtualPath, DependencySource>()
 
   // The set of paths to external dependencies known to be in the graph
   public internal(set) var fingerprintedExternalDependencies = Set<FingerprintedExternalDependency>()
@@ -54,20 +66,22 @@ import SwiftOptions
     self.creationPhase = phase
   }
 
-  @_spi(Testing) public func source(requiredFor input: TypedVirtualPath,
-                                    function: String = #function,
-                                    file: String = #file,
-                                    line: Int = #line) -> DependencySource {
-    guard let source = inputDependencySourceMap.sourceIfKnown(for: input)
-    else {
+  private func addMapEntry(_ input: TypedVirtualPath, _ dependencySource: DependencySource) {
+    assert(input.type == .swift && dependencySource.typedFile.type == .swiftDeps)
+    inputDependencySourceMap[input] = dependencySource
+  }
+
+  @_spi(Testing) public func getSource(for input: TypedVirtualPath,
+                                       function: String = #function,
+                                       file: String = #file,
+                                       line: Int = #line) -> DependencySource {
+    guard let source = inputDependencySourceMap[input] else {
       fatalError("\(input.file.basename) not found in inputDependencySourceMap, \(file):\(line) in \(function)")
     }
     return source
   }
-
-  @_spi(Testing) public func input(neededFor source: DependencySource) -> TypedVirtualPath? {
-    guard let input = inputDependencySourceMap.input(ifKnownFor: source)
-    else {
+  @_spi(Testing) public func getInput(for source: DependencySource) -> TypedVirtualPath? {
+    guard let input = inputDependencySourceMap[source] else {
       info.diagnosticEngine.emit(warning: "Failed to find source file for '\(source.file.basename)', recovering with a full rebuild. Next build will be incremental.")
       return nil
     }
@@ -141,7 +155,7 @@ extension ModuleDependencyGraph {
       return TransitivelyInvalidatedInputSet()
     }
     return collectInputsRequiringCompilationAfterProcessing(
-      dependencySource: source(requiredFor: input))
+      dependencySource: getSource(for: input))
   }
 }
 
@@ -161,17 +175,17 @@ extension ModuleDependencyGraph {
   /// speculatively scheduled in the first wave.
   func collectInputsInvalidatedBy(input: TypedVirtualPath
   ) -> TransitivelyInvalidatedInputArray {
-    let changedSource = source(requiredFor: input)
+    let changedSource = getSource(for: input)
     let allDependencySourcesToRecompile =
       collectSwiftDepsUsing(dependencySource: changedSource)
 
     return allDependencySourcesToRecompile.compactMap {
       depedencySource in
       guard depedencySource != changedSource else {return nil}
-      let dependentInput = inputDependencySourceMap.input(ifKnownFor: depedencySource)
+      let dependentSource = inputDependencySourceMap[depedencySource]
       info.reporter?.report(
-        "Found dependent of \(input.file.basename):", dependentInput)
-      return dependentInput
+        "Found dependent of \(input.file.basename):", dependentSource)
+      return dependentSource
     }
   }
 
@@ -188,7 +202,7 @@ extension ModuleDependencyGraph {
   /// Does the graph contain any dependency nodes for a given source-code file?
   func containsNodes(forSourceFile file: TypedVirtualPath) -> Bool {
     precondition(file.type == .swift)
-    guard let source = inputDependencySourceMap.sourceIfKnown(for: file) else {
+    guard let source = inputDependencySourceMap[file] else {
       return false
     }
     return containsNodes(forDependencySource: source)
@@ -198,46 +212,17 @@ extension ModuleDependencyGraph {
     return nodeFinder.findNodes(for: source).map {!$0.isEmpty}
       ?? false
   }
-  
-  /// Returns: false on error
-  func populateInputDependencySourceMap(
-    for purpose: InputDependencySourceMap.AdditionPurpose
-  ) -> Bool {
+
+  /// Return true on success
+  func populateInputDependencySourceMap() -> Bool {
     let ofm = info.outputFileMap
-    let diags = info.diagnosticEngine
-    var allFound = true
-    for input in info.inputFiles {
-      if let source = ofm.dependencySource(for: input, diagnosticEngine: diags) {
-        inputDependencySourceMap.addEntry(input, source, for: purpose)
-      } else {
-        // Don't break in order to report all failures.
-        allFound = false
-      }
-    }
-    return allFound
-  }
-}
-extension OutputFileMap {
-  fileprivate func dependencySource(
-    for sourceFile: TypedVirtualPath,
-    diagnosticEngine: DiagnosticsEngine
-  ) -> DependencySource? {
-    assert(sourceFile.type == FileType.swift)
-    guard let swiftDepsPath = existingOutput(inputFile: sourceFile.fileHandle,
-                                             outputType: .swiftDeps)
-    else {
-      // The legacy driver fails silently here.
-      diagnosticEngine.emit(
-        .remarkDisabled("\(sourceFile.file.basename) has no swiftDeps file")
-      )
-      return nil
+    let de = info.diagnosticEngine
+    return info.inputFiles.reduce(true) { okSoFar, input in
+      ofm.getDependencySource(for: input, diagnosticEngine: de)
+        .map {source in addMapEntry(input, source); return okSoFar } ?? false
     }
-    assert(VirtualPath.lookup(swiftDepsPath).extension == FileType.swiftDeps.rawValue)
-    let typedSwiftDepsFile = TypedVirtualPath(file: swiftDepsPath, type: .swiftDeps)
-    return DependencySource(typedSwiftDepsFile)
   }
 }
-
 // MARK: - Scheduling the 2nd wave
 extension ModuleDependencyGraph {
   /// After `source` has been compiled, figure out what other source files need compiling.
@@ -247,7 +232,7 @@ extension ModuleDependencyGraph {
   func collectInputsRequiringCompilation(byCompiling input: TypedVirtualPath
   ) -> TransitivelyInvalidatedInputSet? {
     precondition(input.type == .swift)
-    let dependencySource = source(requiredFor: input)
+    let dependencySource = getSource(for: input)
     return collectInputsRequiringCompilationAfterProcessing(
       dependencySource: dependencySource)
   }
@@ -342,7 +327,7 @@ extension ModuleDependencyGraph {
   ) -> TransitivelyInvalidatedInputSet? {
     var invalidatedInputs = TransitivelyInvalidatedInputSet()
     for invalidatedSwiftDeps in collectSwiftDepsUsingInvalidated(nodes: directlyInvalidatedNodes) {
-      guard let invalidatedInput = input(neededFor: invalidatedSwiftDeps) else {
+      guard let invalidatedInput = getInput(for: invalidatedSwiftDeps) else {
         return nil
       }
       invalidatedInputs.insert(invalidatedInput)
@@ -466,6 +451,27 @@ extension ModuleDependencyGraph {
   }
 }
 
+extension OutputFileMap {
+  fileprivate func getDependencySource(
+    for sourceFile: TypedVirtualPath,
+    diagnosticEngine: DiagnosticsEngine
+  ) -> DependencySource? {
+    assert(sourceFile.type == FileType.swift)
+    guard let swiftDepsPath = existingOutput(inputFile: sourceFile.fileHandle,
+                                             outputType: .swiftDeps)
+    else {
+      // The legacy driver fails silently here.
+      diagnosticEngine.emit(
+        .remarkDisabled("\(sourceFile.file.basename) has no swiftDeps file")
+      )
+      return nil
+    }
+    assert(VirtualPath.lookup(swiftDepsPath).extension == FileType.swiftDeps.rawValue)
+    let typedSwiftDepsFile = TypedVirtualPath(file: swiftDepsPath, type: .swiftDeps)
+    return DependencySource(typedSwiftDepsFile)
+  }
+}
+
 // MARK: - tracking traced nodes
 extension ModuleDependencyGraph {
 
@@ -590,11 +596,10 @@ extension ModuleDependencyGraph {
             .record(def: dependencyKey, use: self.allNodes[useID])
           assert(isNewUse, "Duplicate use def-use arc in graph?")
         }
-        for (input, dependencySource) in inputDependencySourceMap {
-          graph.inputDependencySourceMap.addEntry(input,
-                                                  dependencySource,
-                                                  for: .readingPriors)
+        for (input, source) in inputDependencySourceMap {
+          graph.addMapEntry(input, source)
         }
+
         return self.graph
       }
 
@@ -890,7 +895,7 @@ extension ModuleDependencyGraph {
         }
       }
 
-      graph.inputDependencySourceMap.enumerateToSerializePriors { input, dependencySource in
+      for (input, dependencySource) in graph.inputDependencySourceMap {
         self.addIdentifier(input.file.name)
         self.addIdentifier(dependencySource.file.name)
       }
@@ -1035,8 +1040,7 @@ extension ModuleDependencyGraph {
             }
           }
         }
-        graph.inputDependencySourceMap.enumerateToSerializePriors {
-          input, dependencySource in
+        for (input, dependencySource) in graph.inputDependencySourceMap {
           serializer.stream.writeRecord(serializer.abbreviations[.mapNode]!) {
             $0.append(RecordID.mapNode)
             $0.append(serializer.lookupIdentifierCode(for: input.file.name))
@@ -1154,7 +1158,7 @@ extension Set where Element == ModuleDependencyGraph.Node {
   }
 }
 
-extension InputDependencySourceMap {
+extension BidirectionalMap where T1 == TypedVirtualPath, T2 == DependencySource {
   fileprivate func matches(_ other: Self) -> Bool {
     self == other
   }
@@ -1172,6 +1176,6 @@ extension ModuleDependencyGraph {
     _ mockInput: TypedVirtualPath,
     _ mockDependencySource: DependencySource
   ) {
-    inputDependencySourceMap.addEntry(mockInput, mockDependencySource, for: .mocking)
+    addMapEntry(mockInput, mockDependencySource)
   }
 }

Sources/SwiftDriver/IncrementalCompilation/ModuleDependencyGraphParts/InputDependencySourceMap.swift

@@ -121,8 +121,7 @@ extension ModuleDependencyGraph.Tracer {
         path.compactMap { node in
           node.dependencySource.map {
             source in
-            graph.inputDependencySourceMap.input(ifKnownFor: source).map {
-              input in
+            graph.inputDependencySourceMap[source].map { input in
               "\(node.key) in \(input.file.basename)"
             }
             ?? "\(node.key)"