Merge pull request #502 from davidungar/isUntraced

Optimize performance by replacing a set of nodes with a bit in each node.

Author: David Ungar

Sources/SwiftDriver/IncrementalCompilation/ModuleDependencyGraph.swift

@@ -23,10 +23,6 @@ import SwiftOptions
 
   @_spi(Testing) public var nodeFinder = NodeFinder()
 
-  /// When integrating a change, want to find untraced nodes so we can kick off jobs that have not been
-  /// kicked off yet
-  private var tracedNodes = Set<Node>()
-
   /// Maps input files (e.g. .swift) to and from the DependencySource object
   @_spi(Testing) public private(set) var inputDependencySourceMap = BidirectionalMap<TypedVirtualPath, DependencySource>()
 
@@ -207,7 +203,7 @@ extension ModuleDependencyGraph {
                     dependencySource: nil)
     return nodeFinder
       .uses(of: node)
-      .filter({ use in isUntraced(use) })
+      .filter({ use in use.isUntraced })
   }
 
   /// Find all the inputs known to need recompilation as a consequence of reading a swiftdeps or swiftmodule
@@ -345,19 +341,12 @@ extension OutputFileMap {
 // MARK: - tracking traced nodes
 extension ModuleDependencyGraph {
 
-  func isUntraced(_ n: Node) -> Bool {
-    !isTraced(n)
-  }
-  @_spi(Testing) public func isTraced(_ n: Node) -> Bool {
-    tracedNodes.contains(n)
-  }
-  func amTracing(_ n: Node) {
-    tracedNodes.insert(n)
-  }
-  func ensureGraphWillRetraceDependents<Nodes: Sequence>(of nodes: Nodes)
+ func ensureGraphWillRetrace<Nodes: Sequence>(_ nodes: Nodes)
   where Nodes.Element == Node
   {
-    nodes.forEach { tracedNodes.remove($0) }
+    for node in nodes {
+      node.setUntraced()
+    }
   }
 }
 
@@ -1074,7 +1063,6 @@ extension Diagnostic.Message {
 extension ModuleDependencyGraph {
   func matches(_ other: ModuleDependencyGraph) -> Bool {
     guard nodeFinder.matches(other.nodeFinder),
-          tracedNodes.matches(other.tracedNodes),
           inputDependencySourceMap.matches(other.inputDependencySourceMap),
           fingerprintedExternalDependencies.matches(other.fingerprintedExternalDependencies)
     else {

Sources/SwiftDriver/IncrementalCompilation/ModuleDependencyGraphParts/Integrator.swift

@@ -91,7 +91,7 @@ extension ModuleDependencyGraph.Integrator {
   private mutating func integrate() {
     integrateEachSourceNode()
     handleDisappearedNodes()
-    destination.ensureGraphWillRetraceDependents(of: results.allInvalidatedNodes)
+    destination.ensureGraphWillRetrace(results.allInvalidatedNodes)
   }
   private mutating func integrateEachSourceNode() {
     sourceGraph.forEachNode { integrate(oneNode: $0) }

Sources/SwiftDriver/IncrementalCompilation/ModuleDependencyGraphParts/Node.swift

@@ -24,7 +24,6 @@ extension ModuleDependencyGraph {
   ///
   /// Use a class, not a struct because otherwise it would be duplicated for each thing it uses
 
-  /*@_spi(Testing)*/
   public final class Node {
 
     /*@_spi(Testing)*/ public typealias Graph = ModuleDependencyGraph
@@ -44,6 +43,12 @@ extension ModuleDependencyGraph {
     let dependencySource: DependencySource?
     var isExpat: Bool { dependencySource == nil }
 
+    /// When integrating a change, the driver finds untraced nodes so it can kick off jobs that have not been
+    /// kicked off yet. (Within any one driver invocation, compiling a source file is idempotent.)
+    /// When reading a serialized, prior graph, *don't* recover this state, since it will be a new driver
+    /// invocation that has not kicked off any compiles yet.
+    @_spi(Testing) public private(set) var isTraced: Bool = false
+
     /// This dependencySource is the file where the swiftDeps, etc. was read, not necessarily anything in the
     /// SourceFileDependencyGraph or the DependencyKeys
     init(key: DependencyKey, fingerprint: String?,
@@ -53,6 +58,14 @@ extension ModuleDependencyGraph {
     }
   }
 }
+
+// MARK: - trace status
+extension ModuleDependencyGraph.Node {
+  var isUntraced: Bool { !isTraced }
+  func setTraced() { isTraced = true }
+  func setUntraced() { isTraced = false }
+}
+
 // MARK: - comparing, hashing
 extension ModuleDependencyGraph.Node: Equatable, Hashable {
   public static func == (lhs: Graph.Node, rhs: Graph.Node) -> Bool {

Sources/SwiftDriver/IncrementalCompilation/ModuleDependencyGraphParts/Tracer.swift

@@ -72,8 +72,8 @@ extension ModuleDependencyGraph.Tracer {
   private mutating func collectNextPreviouslyUntracedDependent(
     of definition: ModuleDependencyGraph.Node
   ) {
-    guard graph.isUntraced(definition) else { return }
-    graph.amTracing(definition)
+    guard definition.isUntraced else { return }
+    definition.setTraced()
 
     tracedUses.append(definition)
 

Tests/SwiftDriverTests/Helpers/MockingIncrementalCompilation.swift

@@ -20,11 +20,11 @@ extension ModuleDependencyGraph {
     let dependencySource = DependencySource(mock: i)
     // optimization
     if let fileNode = nodeFinder.findFileInterfaceNode(forMock: dependencySource),
-       isTraced(fileNode) {
+       fileNode.isTraced {
       return true
     }
     if let nodes = nodeFinder.findNodes(for: dependencySource)?.values,
-       nodes.contains(where: isTraced) {
+       nodes.contains(where: {$0.isTraced}) {
       return true
     }
     return false