Refactor BuildRecordInfo

Steal a number of its file reading, writing, and validation routines away from it. These are better managed by the incremental build state.

Author: CodaFi

Sources/SwiftDriver/IncrementalCompilation/BuildRecordInfo.swift

@@ -156,15 +156,8 @@ import class Dispatch.DispatchQueue
   ///   - skippedInputs: All primary inputs that were not compiled because the
   ///                    incremental build plan determined they could be
   ///                    skipped.
-  func writeBuildRecord(_ jobs: [Job], _ skippedInputs: Set<TypedVirtualPath>?) {
-    guard let absPath = buildRecordPath.absolutePath else {
-      diagnosticEngine.emit(
-        .warning_could_not_write_build_record_not_absolutePath(buildRecordPath))
-      return
-    }
-    preservePreviousBuildRecord(absPath)
-
-    let buildRecord = self.confinementQueue.sync {
+  @_spi(Testing) public func buildRecord(_ jobs: [Job], _ skippedInputs: Set<TypedVirtualPath>?) -> BuildRecord {
+    return self.confinementQueue.sync {
       BuildRecord(
         jobs: jobs,
         finishedJobResults: finishedJobResults,
@@ -175,18 +168,6 @@ import class Dispatch.DispatchQueue
         timeBeforeFirstJob: timeBeforeFirstJob,
         timeAfterLastJob: .now())
     }
-
-    guard let contents = buildRecord.encode(currentArgsHash: currentArgsHash,
-                                            diagnosticEngine: diagnosticEngine)
-    else {
-      return
-    }
-    do {
-      try fileSystem.writeFileContents(absPath,
-                                       bytes: ByteString(encodingAsUTF8: contents))
-    } catch {
-      diagnosticEngine.emit(.warning_could_not_write_build_record(absPath))
-    }
   }
 
   func removeBuildRecord() {
@@ -203,67 +184,9 @@ import class Dispatch.DispatchQueue
     try? fileSystem.removeFileTree(absPath)
   }
 
-  /// Before writing to the dependencies file path, preserve any previous file
-  /// that may have been there. No error handling -- this is just a nicety, it
-  /// doesn't matter if it fails.
-  /// Added for the sake of compatibility with the legacy driver.
-  private func preservePreviousBuildRecord(_ oldPath: AbsolutePath) {
-    let newPath = oldPath.withTilde()
-    try? fileSystem.move(from: oldPath, to: newPath)
-  }
-
-
-  // TODO: Incremental too many names, buildRecord BuildRecord outofdatemap
-  func populateOutOfDateBuildRecord(
-    reporter: IncrementalCompilationState.Reporter?
-  ) -> BuildRecord? {
-    let contents: String
-    do {
-      contents = try fileSystem.readFileContents(buildRecordPath).cString
-     } catch {
-      reporter?.report("Incremental compilation could not read build record at ", buildRecordPath)
-      reporter?.reportDisablingIncrementalBuild("could not read build record")
-      return nil
-    }
-    func failedToReadOutOfDateMap(_ reason: String? = nil) {
-      let why = "malformed build record file\(reason.map {" " + $0} ?? "")"
-      reporter?.report(
-        "Incremental compilation has been disabled due to \(why)", buildRecordPath)
-      reporter?.reportDisablingIncrementalBuild(why)
-    }
-    guard let outOfDateBuildRecord = BuildRecord(contents: contents,
-                                                 failedToReadOutOfDateMap: failedToReadOutOfDateMap)
-    else {
-      return nil
-    }
-    guard actualSwiftVersion == outOfDateBuildRecord.swiftVersion
-    else {
-      let why = "compiler version mismatch. Compiling with: \(actualSwiftVersion). Previously compiled with: \(outOfDateBuildRecord.swiftVersion)"
-      // mimic legacy
-      reporter?.reportIncrementalCompilationHasBeenDisabled("due to a " + why)
-      reporter?.reportDisablingIncrementalBuild(why)
-      return nil
-    }
-    guard outOfDateBuildRecord.argsHash.map({ $0 == currentArgsHash }) ?? true else {
-      let why = "different arguments were passed to the compiler"
-      // mimic legacy
-      reporter?.reportIncrementalCompilationHasBeenDisabled("because " + why)
-      reporter?.reportDisablingIncrementalBuild(why)
-      return nil
-    }
-    return outOfDateBuildRecord
-  }
-
   func readOutOfDateInterModuleDependencyGraph(
-    buildRecord: BuildRecord?,
     reporter: IncrementalCompilationState.Reporter?
   ) -> InterModuleDependencyGraph? {
-    // If a valid build record could not be produced, do not bother here
-    guard buildRecord != nil else {
-      reporter?.report("Incremental compilation did not attempt to read inter-module dependency graph.")
-      return nil
-    }
-
     let decodedGraph: InterModuleDependencyGraph
     do {
       let contents = try fileSystem.readFileContents(interModuleDependencyGraphPath).cString
@@ -309,9 +232,3 @@ import class Dispatch.DispatchQueue
     buildRecordPath.parentDirectory
   }
 }
-
-fileprivate extension AbsolutePath {
-  func withTilde() -> Self {
-    parentDirectory.appending(component: basename + "~")
-  }
-}

Sources/SwiftDriver/IncrementalCompilation/IncrementalCompilationProtectedState.swift

@@ -150,7 +150,7 @@ extension IncrementalCompilationState.ProtectedState {
 
   func writeGraph(to path: VirtualPath,
                   on fs: FileSystem,
-                  buildRecord: BuildRecordInfo,
+                  buildRecord: BuildRecord,
                   mockSerializedGraphVersion: Version? = nil
   ) throws {
     accessSafetyPrecondition()

Sources/SwiftDriver/IncrementalCompilation/IncrementalCompilationState+Extensions.swift

@@ -393,22 +393,16 @@ extension IncrementalCompilationState {
     }
   }
 
-  func writeDependencyGraph(_ buildRecordInfo: BuildRecordInfo?) throws {
-    // If the cross-module build is not enabled, the status quo dictates we
-    // not emit this file.
-    guard info.isCrossModuleIncrementalBuildEnabled else {
-      return
-    }
-    guard
-      let recordInfo = buildRecordInfo
-    else {
-      throw WriteDependencyGraphError.noBuildRecordInfo
-    }
-    try blockingConcurrentMutationToProtectedState {
+  func writeDependencyGraph(
+    to path: VirtualPath,
+    _ buildRecord: BuildRecord
+  ) throws {
+    precondition(info.isCrossModuleIncrementalBuildEnabled)
+    try blockingConcurrentAccessOrMutationToProtectedState {
       try $0.writeGraph(
-        to: recordInfo.dependencyGraphPath,
+        to: path,
         on: info.fileSystem,
-        buildRecord: recordInfo)
+        buildRecord: buildRecord)
     }
   }
 
@@ -488,7 +482,12 @@ extension OutputFileMap {
 // MARK: SourceFiles
 
 /// Handy information about the source files in the current invocation
-@_spi(Testing) public struct SourceFiles {
+///
+/// Usages of this structure are deprecated and should be removed on sight. For
+/// large driver jobs, it is extremely expensive both in terms of memory and
+/// compilation latency to instantiate as it rematerializes the entire input set
+/// multiple times.
+struct SourceFiles {
   /// The current (.swift) files in same order as the invocation
   let currentInOrder: [SwiftSourceFile]
 
@@ -501,10 +500,10 @@ extension OutputFileMap {
   /// The files that were in the previous but not in the current invocation
   let disappeared: [SwiftSourceFile]
 
-  init(inputFiles: [TypedVirtualPath], buildRecord: BuildRecord?) {
+  init(inputFiles: [TypedVirtualPath], buildRecord: BuildRecord) {
     self.currentInOrder = inputFiles.swiftSourceFiles
     self.currentSet = Set(currentInOrder)
-    guard let buildRecord = buildRecord else {
+    guard !buildRecord.inputInfos.isEmpty else {
       self.previousSet = Set()
       self.disappeared = []
       return

Sources/SwiftDriver/IncrementalCompilation/IncrementalCompilationState.swift

@@ -64,9 +64,12 @@ public final class IncrementalCompilationState {
     reporter?.reportOnIncrementalImports(
       initialState.incrementalOptions.contains(.enableCrossModuleIncrementalBuild))
 
-    let firstWave =
-      try FirstWaveComputer(initialState: initialState, jobsInPhases: jobsInPhases,
-                            driver: driver, reporter: reporter).compute(batchJobFormer: &driver)
+    let firstWave = try FirstWaveComputer(
+      initialState: initialState,
+      jobsInPhases: jobsInPhases,
+      driver: driver,
+      reporter: reporter)
+      .compute(batchJobFormer: &driver)
 
     self.info = initialState.graph.info
     self.upToDateInterModuleDependencyGraph = interModuleDependencyGraph

Tests/SwiftDriverTests/Helpers/MockingIncrementalCompilation.swift

@@ -142,7 +142,7 @@ extension IncrementalCompilationState.IncrementalDependencyAndInputSetup {
       compilerVersion: "for-testing")
     return Self(options, outputFileMap,
                 buildRecord,
-                nil, nil, nil, inputFiles, fileSystem,
+                nil, inputFiles, fileSystem,
                 diagnosticEngine)
   }
 }
@@ -176,19 +176,18 @@ struct MockModuleDependencyGraphCreator {
   }
 
   func mockUpAGraph() -> ModuleDependencyGraph {
-    .createForBuildingFromSwiftDeps(info)
+    .createForBuildingFromSwiftDeps(info.buildRecordInfo.buildRecord([], []), info)
   }
 }
 
 
 extension OutputFileMap {
   static func mock(maxIndex: Int) -> Self {
-    OutputFileMap( entries: (0...maxIndex) .reduce(into: [:]) {
+    OutputFileMap(entries: (0...maxIndex) .reduce(into: [:]) {
       entries, index in
       let inputHandle = SwiftSourceFile(mock: index).fileHandle
       let swiftDepsHandle = SwiftSourceFile(mock: index).fileHandle
       entries[inputHandle] = [.swiftDeps: swiftDepsHandle]
-    }
-    )
+    })
   }
 }

Tests/SwiftDriverTests/IncrementalBuildPerformanceTests.swift

@@ -38,7 +38,7 @@ class IncrementalBuildPerformanceTests: XCTestCase {
       throw XCTSkip()
 #else
 
-    let packageRootPath = AbsolutePath(#file)
+    let packageRootPath = try AbsolutePath(validating: #file)
       .parentDirectory
       .parentDirectory
       .parentDirectory
@@ -69,7 +69,7 @@ class IncrementalBuildPerformanceTests: XCTestCase {
     let info = IncrementalCompilationState.IncrementalDependencyAndInputSetup
       .mock(options: [], outputFileMap: outputFileMap)
 
-    let g = ModuleDependencyGraph.createForSimulatingCleanBuild(info)
+    let g = ModuleDependencyGraph.createForSimulatingCleanBuild(info.buildRecordInfo.buildRecord([], []), info)
     g.blockingConcurrentAccessOrMutation {
       switch whatToMeasure {
       case .readingSwiftDeps:
@@ -79,14 +79,14 @@ class IncrementalBuildPerformanceTests: XCTestCase {
         measure {
           _ = ModuleDependencyGraph.Serializer.serialize(
             g,
-            info.buildRecordInfo,
+            g.buildRecord,
             ModuleDependencyGraph.serializedGraphVersion)
         }
       case .readingPriors:
         readSwiftDeps(for: inputs, into: g)
         let data = ModuleDependencyGraph.Serializer.serialize(
           g,
-          info.buildRecordInfo,
+          g.buildRecord,
           ModuleDependencyGraph.serializedGraphVersion)
         measure {
           try? XCTAssertNoThrow(ModuleDependencyGraph.deserialize(data, info: info))