Add comments and direct/indirect naming

Author: David Ungar

Sources/SwiftDriver/IncrementalCompilation/InitialStateComputer.swift

@@ -152,9 +152,11 @@ extension IncrementalCompilationState.InitialStateComputer {
 
     // Any externals not already in graph must be additions which should trigger
     // recompilation. Thus, `ChangedOrAdded`.
-    let nodesInvalidatedByExternals = graph.collectNodesInvalidatedByChangedOrAddedExternals()
-    let inputsInvalidatedByExternals = graph.collectInputsUsingTransitivelyInvalidated(nodes: nodesInvalidatedByExternals)
-    return (graph, inputsInvalidatedByExternals)
+    let nodesDirectlyInvalidatedByExternals = graph.collectNodesDirectlyInvalidatedByChangedOrAddedExternals()
+    // Wait till the last minute to do the transitive closure as an optimization.
+    let inputsTransitivelyInvalidatedByExternals = graph.collectInputsUsingTransitivelyInvalidated(
+      nodes: nodesDirectlyInvalidatedByExternals)
+    return (graph, inputsTransitivelyInvalidatedByExternals)
   }
 
   /// Builds a graph

Sources/SwiftDriver/IncrementalCompilation/ModuleDependencyGraph.swift

@@ -106,10 +106,10 @@ extension ModuleDependencyGraph {
 
 // MARK: - Getting a graph read from priors ready to use
 extension ModuleDependencyGraph {
-  func collectNodesInvalidatedByChangedOrAddedExternals() -> Set<Node> {
+  func collectNodesDirectlyInvalidatedByChangedOrAddedExternals() -> Set<Node> {
     fingerprintedExternalDependencies.reduce(into: Set()) { invalidatedNodes, fed in
       invalidatedNodes.formUnion (
-        self.collectNodesInvalidatedByProcessing(fingerprintedExternalDependency: fed))
+        self.collectNodesDirectlyInvalidatedByProcessing(fingerprintedExternalDependency: fed))
     }
   }
 }
@@ -252,18 +252,18 @@ extension ModuleDependencyGraph {
     /// If reading for the first time, the driver is compiling all outdated source files anyway, so only
     /// nodes invalidated by external dependencies matter.
     /// But when updating, all invalidations matter.
-    let invalidatedNodes = phase.isUpdating
-      ? results.allInvalidatedNodes
-      : results.nodesInvalidatedByUsingSomeExternal
+    let directlyInvalidatedNodes = phase.isUpdating
+      ? results.allDirectlyInvalidatedNodes
+      : results.nodesDirectlyInvalidatedByUsingSomeExternal
 
-    return collectInputsUsingTransitivelyInvalidated(nodes: invalidatedNodes)
+    return collectInputsUsingTransitivelyInvalidated(nodes: directlyInvalidatedNodes)
   }
 
   /// Given nodes that are invalidated, find all the affected inputs that must be recompiled.
   func collectInputsUsingTransitivelyInvalidated(
-    nodes invalidatedNodes: Set<Node>
+    nodes directlyInvalidatedNodes: Set<Node>
   ) -> Set<TypedVirtualPath> {
-    collectSwiftDepsUsingTransitivelyInvalidated(nodes: invalidatedNodes)
+    collectSwiftDepsUsingTransitivelyInvalidated(nodes: directlyInvalidatedNodes)
       .reduce(into: Set()) { invalidatedInputs, invalidatedSwiftDeps in
         invalidatedInputs.insert(getInput(for: invalidatedSwiftDeps))
       }
@@ -276,7 +276,8 @@ extension ModuleDependencyGraph {
   /// Process a possibly-fingerprinted external dependency by reading and integrating, if applicable.
   /// Return the nodes thus invalidated.
   /// But always integrate, in order to detect future changes.
-  func collectNodesInvalidatedByProcessing(
+  /// This function does not to the transitive closure; that is left to the callers
+  func collectNodesDirectlyInvalidatedByProcessing(
     fingerprintedExternalDependency fed: FingerprintedExternalDependency)
   -> Set<Node> {
 
@@ -291,16 +292,28 @@ extension ModuleDependencyGraph {
     let shouldTryToProcess = info.isCrossModuleIncrementalBuildEnabled &&
       (isNewToTheGraph || lazyModTimer.hasExternalFileChanged)
 
-    let invalidatedNodesFromIncrementalExternal = shouldTryToProcess
-      ? collectNodesInvalidatedByAttemptingToProcess(fed, info)
+    // Do this no matter what in order to integrate any incremental external dependencies.
+    let directlyInvalidatedNodesFromIncrementalExternal = shouldTryToProcess
+      ? collectNodesDirectlyInvalidatedByAttemptingToProcess(fed, info)
       : nil
 
-    let callerWantsTheseChanges = (phase.isUpdating && isNewToTheGraph) ||
+    if phase == .buildingAfterEachCompilation {
+      // going to compile every input anyway, less work for callers
+      return Set()
+    }
+
+    /// When building a graph from scratch, an unchanged but new-to-the-graph external dependendcy should be ignored.
+    /// Otherwise, it represents an added Import
+    let callerWantsTheseChanges = (phase != .buildingWithoutAPrior && isNewToTheGraph) ||
       lazyModTimer.hasExternalFileChanged
 
-    return !callerWantsTheseChanges
-      ? Set<Node>()
-      : invalidatedNodesFromIncrementalExternal ?? collectUntracedNodesDirectlyUsing(fed)
+    guard callerWantsTheseChanges else {
+      return Set()
+    }
+
+    // If there was an error integrating the external dependency, or if it was not an incremental one,
+    // return anything that uses that dependency.
+    return directlyInvalidatedNodesFromIncrementalExternal ?? collectUntracedNodesDirectlyUsing(fed)
   }
 
   private struct LazyModTimer {
@@ -311,15 +324,17 @@ extension ModuleDependencyGraph {
       >= info.buildTime
   }
 
-  private func collectNodesInvalidatedByAttemptingToProcess(
+  /// Try to read and integrate an external dependency.
+  /// Return nil if it's not incremental, or if an error occurs.
+  private func collectNodesDirectlyInvalidatedByAttemptingToProcess(
     _ fed: FingerprintedExternalDependency,
     _ info: IncrementalCompilationState.InitialStateComputer) -> Set<Node>? {
     fed.incrementalDependencySource?
       .read(in: info.fileSystem, reporter: info.reporter)
       .map { unserializedDepGraph in
         info.reporter?.report("Integrating changes from: \(fed.externalDependency)")
         return Integrator.integrate(from: unserializedDepGraph, into: self)
-          .allInvalidatedNodes
+          .allDirectlyInvalidatedNodes
       }
   }
 

Sources/SwiftDriver/IncrementalCompilation/ModuleDependencyGraphParts/Integrator.swift

@@ -75,7 +75,8 @@ extension ModuleDependencyGraph.Integrator {
   private mutating func integrate() {
     integrateEachSourceNode()
     handleDisappearedNodes()
-    destination.ensureGraphWillRetrace(results.allInvalidatedNodes)
+    // Ensure transitive closure will get started.
+    destination.ensureGraphWillRetrace(results.allDirectlyInvalidatedNodes)
   }
   private mutating func integrateEachSourceNode() {
     sourceGraph.forEachNode { integrate(oneNode: $0) }
@@ -196,9 +197,9 @@ extension ModuleDependencyGraph.Integrator {
     externalDependency fingerprintedExternalDependency: FingerprintedExternalDependency,
     moduleFileGraphUseNode moduleUseNode: Graph.Node) {
 
-    let invalidated = destination.collectNodesInvalidatedByProcessing(
+    let invalidated = destination.collectNodesDirectlyInvalidatedByProcessing(
       fingerprintedExternalDependency: fingerprintedExternalDependency)
-    results.addNodesInvalidatedByUsingSomeExternal(invalidated)
+    results.addNodesDirectlyInvalidatedByUsingSomeExternal(invalidated)
   }
 }
 
@@ -208,25 +209,25 @@ extension ModuleDependencyGraph.Integrator {
   public struct Results {
     typealias Node = ModuleDependencyGraph.Node
 
-    private(set) var allInvalidatedNodes = Set<Node>()
-    private(set) var nodesInvalidatedByUsingSomeExternal = Set<Node>()
+    private(set) var allDirectlyInvalidatedNodes = Set<Node>()
+    private(set) var nodesDirectlyInvalidatedByUsingSomeExternal = Set<Node>()
 
-    mutating func addNodesInvalidatedByUsingSomeExternal(_ invalidated: Set<Node>)
+    mutating func addNodesDirectlyInvalidatedByUsingSomeExternal(_ invalidated: Set<Node>)
     {
-      allInvalidatedNodes.formUnion(invalidated)
-      nodesInvalidatedByUsingSomeExternal.formUnion(invalidated)
+      allDirectlyInvalidatedNodes.formUnion(invalidated)
+      allDirectlyInvalidatedNodes.formUnion(invalidated)
     }
     mutating func addDisappeared(_ node: Node) {
-      allInvalidatedNodes.insert(node)
+      allDirectlyInvalidatedNodes.insert(node)
     }
     mutating func addChanged(_ node: Node) {
-      allInvalidatedNodes.insert(node)
+      allDirectlyInvalidatedNodes.insert(node)
     }
     mutating func addPatriated(_ node: Node) {
-      allInvalidatedNodes.insert(node)
+      allDirectlyInvalidatedNodes.insert(node)
     }
     mutating func addNew(_ node: Node) {
-      allInvalidatedNodes.insert(node)
+      allDirectlyInvalidatedNodes.insert(node)
     }
   }
 }

Tests/SwiftDriverTests/ModuleDependencyGraphTests.swift

@@ -752,7 +752,7 @@ class ModuleDependencyGraphTests: XCTestCase {
 
   func testLoadPassesWithFingerprint() {
     let graph = ModuleDependencyGraph(mock: de)
-    _ = graph.getInvalidatedNodesForSimulatedLoad(
+    _ = graph.getDirectlyInvalidatedNodesForSimulatedLoad(
       0,
       [MockDependencyKind.nominal: ["A@1"]])
   }
@@ -952,7 +952,7 @@ extension ModuleDependencyGraph {
     includePrivateDeps: Bool = true,
     hadCompilationError: Bool = false)
   {
-    _ = getInvalidatedNodesForSimulatedLoad(
+    _ = getDirectlyInvalidatedNodesForSimulatedLoad(
       swiftDepsIndex, dependencyDescriptions,
       interfaceHash,
       includePrivateDeps: includePrivateDeps,
@@ -966,19 +966,19 @@ extension ModuleDependencyGraph {
                       hadCompilationError: Bool = false)
   -> [Int]
   {
-    let invalidatedNodes = getInvalidatedNodesForSimulatedLoad(
+    let directlyIinvalidatedNodes = getDirectlyInvalidatedNodesForSimulatedLoad(
       swiftDepsIndex,
       dependencyDescriptions,
       interfaceHash,
       includePrivateDeps: includePrivateDeps,
       hadCompilationError: hadCompilationError)
 
-    return collectSwiftDepsUsingTransitivelyInvalidated(nodes: invalidatedNodes)
+    return collectSwiftDepsUsingTransitivelyInvalidated(nodes: directlyIinvalidatedNodes)
       .map { $0.mockID }
   }
 
 
-  func getInvalidatedNodesForSimulatedLoad(
+  func getDirectlyInvalidatedNodesForSimulatedLoad(
     _ swiftDepsIndex: Int,
     _ dependencyDescriptions: [MockDependencyKind: [String]],
     _ interfaceHashIfPresent: String? = nil,
@@ -1001,7 +1001,7 @@ extension ModuleDependencyGraph {
 
     let results = Integrator.integrate(from: sfdg, into: self)
 
-    return results.allInvalidatedNodes
+    return results.allDirectlyInvalidatedNodes
   }
 
   func findUntracedSwiftDepsDependent(onExternal s: String) -> [Int] {