Merge pull request #348 from artemcm/NoRepeatedSwiftModuleRescan

[Explicit Module Builds] Re-use external build artifacts to avoid redundant Swift moduel re-scan

Author: artemcm

Sources/SwiftDriver/CMakeLists.txt

@@ -10,7 +10,7 @@ add_library(SwiftDriver
   "Explicit Module Builds/ClangModuleBuildJobCache.swift"
   "Explicit Module Builds/ClangVersionedDependencyResolution.swift"
   "Explicit Module Builds/CommonDependencyGraphOperations.swift"
-  "Explicit Module Builds/ExplicitModuleBuildHandler.swift"
+  "Explicit Module Builds/ExplicitDependencyBuildPlanner.swift"
   "Explicit Module Builds/InterModuleDependencyGraph.swift"
   "Explicit Module Builds/ModuleDependencyScanning.swift"
   "Explicit Module Builds/PlaceholderDependencyResolution.swift"

Sources/SwiftDriver/Driver/Driver.swift

@@ -252,10 +252,10 @@ public struct Driver {
   /// dependencies in parallel builds.
   var forceEmitModuleBeforeCompile: Bool = false
 
-  /// Handler for constructing module build jobs using Explicit Module Builds.
+  /// Planner for constructing module build jobs using Explicit Module Builds.
   /// Constructed during the planning phase only when all modules will be prebuilt and treated
   /// as explicit by the various compilation jobs.
-  @_spi(Testing) public var explicitModuleBuildHandler: ExplicitModuleBuildHandler? = nil
+  @_spi(Testing) public var explicitDependencyBuildPlanner: ExplicitDependencyBuildPlanner? = nil
 
   /// All external artifacts a build system (e.g. SwiftPM) may pass in as input to the explicit
   /// build of the current module. Consists of a map of externally-built targets, and a map of all previously

Sources/SwiftDriver/Explicit Module Builds/CommonDependencyGraphOperations.swift

@@ -110,8 +110,8 @@ internal extension InterModuleDependencyGraph {
       }
       if let originalModuleIndex = moduleInfo.directDependencies?.firstIndex(of: originalId) {
         moduleInfo.directDependencies![originalModuleIndex] = replacementId;
+        moduleInfoMap[moduleId] = moduleInfo
       }
-      moduleInfoMap[moduleId] = moduleInfo
     }
   }
 }

Sources/SwiftDriver/Explicit Module Builds/ExplicitModuleBuildHandler.swift renamed to Sources/SwiftDriver/Explicit Module Builds/ExplicitDependencyBuildPlanner.swift

@@ -1,4 +1,4 @@
-//===--------------- ExplicitModuleBuildHandler.swift ---------------------===//
+//===--------------- ExplicitDependencyBuildPlanner.swift ---------------------===//
 //
 // This source file is part of the Swift.org open source project
 //
@@ -13,16 +13,16 @@ import TSCBasic
 import TSCUtility
 import Foundation
 
-/// A map from a module identifier to a pair consisting of a path to its .swiftmodule file.
+/// A map from a module identifier to a path to its .swiftmodule file.
 public typealias ExternalTargetModulePathMap = [ModuleDependencyId: AbsolutePath]
 /// A tuple all external artifacts a build system may pass in as input to the explicit build of the current module
 /// Consists of a map of externally-built targets, and a map of all previously discovered/scanned modules.
 public typealias ExternalBuildArtifacts = (ExternalTargetModulePathMap, ModuleInfoMap)
 
-/// In Explicit Module Build mode, this handler is responsible for generating and providing
+/// In Explicit Module Build mode, this planner is responsible for generating and providing
 /// build jobs for all module dependencies and providing compile command options
 /// that specify said explicit module dependencies.
-@_spi(Testing) public struct ExplicitModuleBuildHandler {
+@_spi(Testing) public struct ExplicitDependencyBuildPlanner {
   /// The module dependency graph.
   public var dependencyGraph: InterModuleDependencyGraph
 
@@ -201,7 +201,7 @@ public typealias ExternalBuildArtifacts = (ExternalTargetModulePathMap, ModuleIn
 
     // Encode the target triple pcm args into the output `.pcm` filename
     let targetEncodedModulePath =
-      try ExplicitModuleBuildHandler.targetEncodedClangModuleFilePath(for: moduleInfo,
+      try ExplicitDependencyBuildPlanner.targetEncodedClangModuleFilePath(for: moduleInfo,
                                                                       pcmArgs: pcmArgs)
     outputs.append(TypedVirtualPath(file: targetEncodedModulePath, type: .pcm))
     commandLine.appendFlags("-emit-pcm", "-module-name", moduleId.moduleName,
@@ -384,7 +384,7 @@ public typealias ExternalBuildArtifacts = (ExternalTargetModulePathMap, ModuleIn
     let dependencyInfo = try dependencyGraph.moduleInfo(of: dependencyId)
     let dependencyClangModuleDetails = try dependencyGraph.clangModuleDetails(of: dependencyId)
     let clangModulePath =
-      try ExplicitModuleBuildHandler.targetEncodedClangModuleFilePath(for: dependencyInfo,
+      try ExplicitDependencyBuildPlanner.targetEncodedClangModuleFilePath(for: dependencyInfo,
                                                                       pcmArgs: pcmArgs)
 
     // Collect the requried information about this module
@@ -430,7 +430,7 @@ public typealias ExternalBuildArtifacts = (ExternalTargetModulePathMap, ModuleIn
 }
 
 /// Utility methods for encoding PCM's target triple into its name.
-extension ExplicitModuleBuildHandler {
+extension ExplicitDependencyBuildPlanner {
   /// Compute a full path to the resulting .pcm file for a given Clang module, with the
   /// target triple encoded in the name.
   public static func targetEncodedClangModuleFilePath(for moduleInfo: ModuleInfo,
@@ -455,7 +455,7 @@ extension ExplicitModuleBuildHandler {
   }
 }
 
-/// Encapsulates some of the common queries of the ExplicitModuleBuildeHandler with error-checking
+/// Encapsulates some of the common queries of the ExplicitDependencyBuildPlanner with error-checking
 /// on the dependency graph's structure.
 internal extension InterModuleDependencyGraph {
   func moduleInfo(of moduleId: ModuleDependencyId) throws -> ModuleInfo {
@@ -496,7 +496,7 @@ internal extension InterModuleDependencyGraph {
 }
 
 // InterModuleDependencyGraph printing, useful for debugging
-private extension InterModuleDependencyGraph {
+internal extension InterModuleDependencyGraph {
   func prettyPrintString() throws -> String {
     let encoder = JSONEncoder()
     encoder.outputFormatting = [.prettyPrinted]
@@ -505,10 +505,10 @@ private extension InterModuleDependencyGraph {
   }
 }
 
-// To keep the ExplicitModuleBuildHandler an implementation detail, provide an API
+// To keep the ExplicitDependencyBuildPlanner an implementation detail, provide an API
 // to access the dependency graph
 extension Driver {
   public var interModuleDependencyGraph: InterModuleDependencyGraph? {
-    return explicitModuleBuildHandler?.dependencyGraph
+    return explicitDependencyBuildPlanner?.dependencyGraph
   }
 }

Sources/SwiftDriver/Explicit Module Builds/ModuleDependencyScanning.swift

@@ -32,8 +32,7 @@ extension Driver {
     // Pass in external target dependencies to be treated as placeholder dependencies by the scanner
     if let externalBuildArtifacts = externalBuildArtifacts {
       let dependencyPlaceholderMapFile =
-        try serializeExternalDependencyArtifacts(externalTargetModulePathMap:
-                                                  externalBuildArtifacts.0)
+        try serializeExternalDependencyArtifacts(externalBuildArtifacts: externalBuildArtifacts)
       commandLine.appendFlag("-placeholder-dependency-module-map-file")
       commandLine.appendPath(dependencyPlaceholderMapFile)
     }
@@ -54,18 +53,31 @@ extension Driver {
   }
 
   /// Serialize a map of placeholder (external) dependencies for the dependency scanner.
-  func serializeExternalDependencyArtifacts(externalTargetModulePathMap: ExternalTargetModulePathMap)
+  func serializeExternalDependencyArtifacts(externalBuildArtifacts: ExternalBuildArtifacts)
   throws -> VirtualPath {
+    let (externalTargetModulePathMap, externalModuleInfoMap)  = externalBuildArtifacts
     var placeholderArtifacts: [SwiftModuleArtifactInfo] = []
+
+    // Explicit external targets
     for (moduleId, binaryModulePath) in externalTargetModulePathMap {
       placeholderArtifacts.append(
           SwiftModuleArtifactInfo(name: moduleId.moduleName,
                                   modulePath: binaryModulePath.description))
     }
+
+    // All other already-scanned Swift modules
+    for (moduleId, moduleInfo) in externalModuleInfoMap
+    where !externalTargetModulePathMap.keys.contains(moduleId) {
+      guard case .swift(_) = moduleId else { continue }
+      placeholderArtifacts.append(
+          SwiftModuleArtifactInfo(name: moduleId.moduleName,
+                                  modulePath: moduleInfo.modulePath))
+    }
     let encoder = JSONEncoder()
     encoder.outputFormatting = [.prettyPrinted]
     let contents = try encoder.encode(placeholderArtifacts)
-    return .temporaryWithKnownContents(.init("\(moduleOutputInfo.name)-placeholder-modules.json"), contents)
+    return .temporaryWithKnownContents(.init("\(moduleOutputInfo.name)-placeholder-modules.json"),
+                                       contents)
   }
 
   mutating func performBatchDependencyScan(moduleInfos: [BatchScanModuleInfo])
@@ -163,6 +175,7 @@ extension Driver {
     let encoder = JSONEncoder()
     encoder.outputFormatting = [.prettyPrinted]
     let contents = try encoder.encode(moduleInfos)
-    return .temporaryWithKnownContents(.init("\(moduleOutputInfo.name)-batch-module-scan.json"), contents)
+    return .temporaryWithKnownContents(.init("\(moduleOutputInfo.name)-batch-module-scan.json"),
+                                       contents)
   }
 }

Sources/SwiftDriver/Explicit Module Builds/PlaceholderDependencyResolution.swift

@@ -45,24 +45,46 @@ import Foundation
   throws {
     let externalTargetModulePathMap = externalBuildArtifacts.0
     let externalModuleInfoMap = externalBuildArtifacts.1
-    let placeholderModules = modules.keys.filter {
+    let placeholderFilter : (ModuleDependencyId) -> Bool = {
       if case .swiftPlaceholder(_) = $0 {
         return true
       }
       return false
     }
+    var placeholderModules = modules.keys.filter(placeholderFilter)
 
     // Resolve all target placeholder modules
     let placeholderTargetModules = placeholderModules.filter { externalTargetModulePathMap[$0] != nil }
     for moduleId in placeholderTargetModules {
       guard let placeholderModulePath = externalTargetModulePathMap[moduleId] else {
         throw Driver.Error.missingExternalDependency(moduleId.moduleName)
       }
-
       try resolveTargetPlaceholder(placeholderId: moduleId,
                                    placeholderPath: placeholderModulePath,
                                    externalModuleInfoMap: externalModuleInfoMap)
     }
+
+    // Process remaining placeholders until there are none left
+    placeholderModules = modules.keys.filter(placeholderFilter)
+    while !placeholderModules.isEmpty {
+      let moduleId = placeholderModules.first!
+      let swiftModuleId = ModuleDependencyId.swift(moduleId.moduleName)
+
+      guard externalModuleInfoMap[swiftModuleId] != nil else {
+        throw Driver.Error.missingExternalDependency(moduleId.moduleName)
+      }
+      let moduleInfo = externalModuleInfoMap[swiftModuleId]!
+
+      // Insert the resolved module, replacing the placeholder.
+      try Self.mergeModule(swiftModuleId, moduleInfo, into: &modules)
+
+      // Traverse and add all of this external module's dependencies to the current graph.
+      try resolvePlaceholderModuleDependencies(moduleId: swiftModuleId,
+                                               externalModuleInfoMap: externalModuleInfoMap)
+
+      // Update the set of remaining placeholders to resolve
+      placeholderModules = modules.keys.filter(placeholderFilter)
+    }
   }
 }
 

Sources/SwiftDriver/Jobs/FrontendJobHelpers.swift

@@ -424,19 +424,19 @@ extension Driver {
   /// to inputs and command line arguments of a compile job.
   func addExplicitModuleBuildArguments(inputs: inout [TypedVirtualPath],
                                        commandLine: inout [Job.ArgTemplate]) throws {
-    guard var handler = explicitModuleBuildHandler else {
-      fatalError("No handler in Explicit Module Build mode.")
+    guard var dependencyPlanner = explicitDependencyBuildPlanner else {
+      fatalError("No dependency planner in Explicit Module Build mode.")
     }
-    try handler.resolveMainModuleDependencies(inputs: &inputs, commandLine: &commandLine)
+    try dependencyPlanner.resolveMainModuleDependencies(inputs: &inputs, commandLine: &commandLine)
   }
 
   /// In Explicit Module Build mode, distinguish between main module jobs and intermediate dependency build jobs,
   /// such as Swift modules built from .swiftmodule files and Clang PCMs.
   public func isExplicitMainModuleJob(job: Job) -> Bool {
-    guard let handler = explicitModuleBuildHandler else {
-      fatalError("No handler in Explicit Module Build mode.")
+    guard let dependencyPlanner = explicitDependencyBuildPlanner else {
+      fatalError("No dependency planner in Explicit Module Build mode.")
     }
-    return job.moduleName == handler.dependencyGraph.mainModuleName
+    return job.moduleName == dependencyPlanner.dependencyGraph.mainModuleName
   }
 }
 

Sources/SwiftDriver/Jobs/Planning.swift

@@ -372,10 +372,10 @@ extension Driver {
   /// by re-scanning all Clang modules against all possible targets they will be built against.
   public mutating func generateExplicitModuleDependenciesJobs() throws -> [Job] {
     let dependencyGraph = try generateInterModuleDependencyGraph()
-    explicitModuleBuildHandler =
-        try ExplicitModuleBuildHandler(dependencyGraph: dependencyGraph,
+    explicitDependencyBuildPlanner =
+        try ExplicitDependencyBuildPlanner(dependencyGraph: dependencyGraph,
                                        toolchain: toolchain)
-    return try explicitModuleBuildHandler!.generateExplicitModuleDependenciesBuildJobs()
+    return try explicitDependencyBuildPlanner!.generateExplicitModuleDependenciesBuildJobs()
   }
 
   private mutating func generateInterModuleDependencyGraph() throws -> InterModuleDependencyGraph {
@@ -389,7 +389,7 @@ extension Driver {
                                 recordedInputModificationDates: recordedInputModificationDates)
 
     // Resolve placeholder dependencies in the dependency graph, if any.
-    if externalBuildArtifacts != nil, !externalBuildArtifacts!.0.isEmpty {
+    if externalBuildArtifacts != nil {
       try dependencyGraph.resolvePlaceholderDependencies(using: externalBuildArtifacts!)
     }
 
@@ -490,7 +490,6 @@ extension Driver {
 
   /// Plan a build by producing a set of jobs to complete the build.
   public mutating func planBuild() throws -> [Job] {
-
     if let job = try immediateForwardingJob() {
       return [job]
     }

Tests/SwiftDriverTests/ExplicitModuleBuildTests.swift

@@ -116,7 +116,7 @@ private func checkExplicitModuleBuildJobDependencies(job: Job,
         XCTAssertEqual(dependencyArtifacts!.modulePath, prebuiltModuleDetails.compiledModulePath)
       case .clang(let clangDependencyDetails):
         let clangDependencyModulePathString =
-          try ExplicitModuleBuildHandler.targetEncodedClangModuleFilePath(
+          try ExplicitDependencyBuildPlanner.targetEncodedClangModuleFilePath(
           for: dependencyInfo, pcmArgs: pcmArgs)
         let clangDependencyModulePath =
           TypedVirtualPath(file: clangDependencyModulePathString, type: .pcm)
@@ -147,7 +147,7 @@ private func checkExplicitModuleBuildJobDependencies(job: Job,
 private func pcmArgsEncodedRelativeModulePath(for moduleName: String, with pcmArgs: [String]
 ) throws -> RelativePath {
   return RelativePath(
-    try ExplicitModuleBuildHandler.targetEncodedClangModuleName(for: moduleName,
+    try ExplicitDependencyBuildPlanner.targetEncodedClangModuleName(for: moduleName,
                                                                 pcmArgs: pcmArgs) + ".pcm")
 }
 
@@ -163,10 +163,10 @@ final class ExplicitModuleBuildTests: XCTestCase {
             try JSONDecoder().decode(
               InterModuleDependencyGraph.self,
               from: ModuleDependenciesInputs.fastDependencyScannerOutput.data(using: .utf8)!)
-      driver.explicitModuleBuildHandler = try ExplicitModuleBuildHandler(dependencyGraph: moduleDependencyGraph,
+      driver.explicitDependencyBuildPlanner = try ExplicitDependencyBuildPlanner(dependencyGraph: moduleDependencyGraph,
                                                                          toolchain: driver.toolchain)
       let modulePrebuildJobs =
-        try driver.explicitModuleBuildHandler!.generateExplicitModuleDependenciesBuildJobs()
+        try driver.explicitDependencyBuildPlanner!.generateExplicitModuleDependenciesBuildJobs()
       XCTAssertEqual(modulePrebuildJobs.count, 4)
       for job in modulePrebuildJobs {
         XCTAssertEqual(job.outputs.count, 1)
@@ -208,7 +208,8 @@ final class ExplicitModuleBuildTests: XCTestCase {
         [ModuleDependencyId.swiftPlaceholder("B"):AbsolutePath("/Somewhere/B.swiftmodule")]
       let externalModuleInfoMap: ModuleInfoMap = inputDependencyGraph.modules
 
-      // Construct a module dependency graph that will contain .swiftPlaceholder("B")
+      // Construct a module dependency graph that will contain .swiftPlaceholder("B"),
+      // .swiftPlaceholder("Swift"), .swiftPlaceholder("SwiftOnoneSupport")
       var moduleDependencyGraph =
             try JSONDecoder().decode(
               InterModuleDependencyGraph.self,
@@ -228,10 +229,20 @@ final class ExplicitModuleBuildTests: XCTestCase {
       // Plan explicit dependency jobs, after resolving placeholders to actual dependencies.
       try moduleDependencyGraph.resolvePlaceholderDependencies(
         using: (targetModulePathMap, externalModuleInfoMap))
-      driver.explicitModuleBuildHandler = try ExplicitModuleBuildHandler(dependencyGraph: moduleDependencyGraph,
-                                                                         toolchain: driver.toolchain)
+
+      // Ensure the graph no longer contains any placeholders
+      XCTAssertFalse(moduleDependencyGraph.modules.keys.contains {
+        if case .swiftPlaceholder(_) = $0 {
+          return true
+        }
+        return false
+      })
+
+      driver.explicitDependencyBuildPlanner =
+        try ExplicitDependencyBuildPlanner(dependencyGraph: moduleDependencyGraph,
+                                           toolchain: driver.toolchain)
       let modulePrebuildJobs =
-        try driver.explicitModuleBuildHandler!.generateExplicitModuleDependenciesBuildJobs()
+        try driver.explicitDependencyBuildPlanner!.generateExplicitModuleDependenciesBuildJobs()
 
       // Verify that the dependency graph contains no placeholders.
       let placeholdersInTheGraph = driver.interModuleDependencyGraph!.modules.keys
@@ -291,7 +302,7 @@ final class ExplicitModuleBuildTests: XCTestCase {
 
       let jobs = try driver.planBuild()
       // Figure out which Triples to use.
-      let dependencyGraph = driver.explicitModuleBuildHandler!.dependencyGraph
+      let dependencyGraph = driver.explicitDependencyBuildPlanner!.dependencyGraph
       guard case .swift(let mainModuleSwiftDetails) = dependencyGraph.mainModule.details else {
         XCTFail("Main module does not have Swift details field")
         return