Skip to content

[Incremental Builds] Separately check whether we can skip 'emit-module' on an incremental module-only build #1905

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
46 changes: 39 additions & 7 deletions Sources/SwiftDriver/IncrementalCompilation/FirstWaveComputer.swift
Original file line number Diff line number Diff line change
Expand Up @@ -134,11 +134,23 @@ extension IncrementalCompilationState.FirstWaveComputer {
jobCreatingPch: jobCreatingPch)

// In the case where there are no compilation jobs to run on this build (no source-files were changed),
// we can skip running `beforeCompiles` jobs if we also ensure that none of the `afterCompiles` jobs
// have any dependencies on them.
let skipAllJobs = batchedCompilationJobs.isEmpty ? !nonVerifyAfterCompileJobsDependOnBeforeCompileJobs() : false
let beforeCompileJobs = skipAllJobs ? [] : jobsInPhases.beforeCompiles
var skippedNonCompileJobs = skipAllJobs ? jobsInPhases.beforeCompiles : []
// and the emit-module task does not need to be re-run, we can skip running `beforeCompiles` jobs if we
// also ensure that none of the `afterCompiles` jobs have any dependencies on them.
let skippingAllCompileJobs = batchedCompilationJobs.isEmpty
let skipEmitModuleJobs = try skippingAllCompileJobs && computeCanSkipEmitModuleTasks(buildRecord)
let skipAllJobs = skippingAllCompileJobs && skipEmitModuleJobs && !nonVerifyAfterCompileJobsDependOnBeforeCompileJobs()

let beforeCompileJobs: [Job]
var skippedNonCompileJobs: [Job] = []
if skipAllJobs {
beforeCompileJobs = []
skippedNonCompileJobs = jobsInPhases.beforeCompiles
} else if skipEmitModuleJobs {
beforeCompileJobs = jobsInPhases.beforeCompiles.filter { $0.kind != .emitModule }
skippedNonCompileJobs.append(contentsOf: jobsInPhases.beforeCompiles.filter { $0.kind == .emitModule })
} else {
beforeCompileJobs = jobsInPhases.beforeCompiles
}

// Schedule emitModule job together with verify module interface job.
let afterCompileJobs = jobsInPhases.afterCompiles.compactMap { job -> Job? in
Expand Down Expand Up @@ -170,6 +182,27 @@ extension IncrementalCompilationState.FirstWaveComputer {
}
}

/// Figure out if the emit-module tasks are *not* mandatory. This functionality only runs if there are not actual
/// compilation tasks to be run in this build, for example on an emit-module-only build.
private func computeCanSkipEmitModuleTasks(_ buildRecord: BuildRecord) throws -> Bool {
guard let emitModuleJob = jobsInPhases.beforeCompiles.first(where: { $0.kind == .emitModule }) else {
return false // Nothing to skip, so no special handling is required
}
// If a non-emit-module task exists in 'beforeCompiles', it may be another kind of
// changed dependency so we should re-run the module task as well
guard jobsInPhases.beforeCompiles.allSatisfy({ $0.kind == .emitModule }) else {
return false
}
// If any of the outputs do not exist, they must be re-computed
guard try emitModuleJob.outputs.allSatisfy({ try fileSystem.exists($0.file) }) else {
return false
}

// Ensure that no output is older than any of the inputs
let oldestOutputModTime: TimePoint = try emitModuleJob.outputs.map { try fileSystem.lastModificationTime(for: $0.file) }.min() ?? .distantPast
return try emitModuleJob.inputs.swiftSourceFiles.allSatisfy({ try fileSystem.lastModificationTime(for: $0.typedFile.file) < oldestOutputModTime })
}

/// Figure out which compilation inputs are *not* mandatory at the start
private func computeInitiallySkippedCompilationInputs(
inputsInvalidatedByExternals: TransitivelyInvalidatedSwiftSourceFileSet,
Expand All @@ -178,7 +211,7 @@ extension IncrementalCompilationState.FirstWaveComputer {
) -> Set<TypedVirtualPath> {
let allCompileJobs = jobsInPhases.compileJobs
// Input == source file
let changedInputs = computeChangedInputs(moduleDependencyGraph, buildRecord)
let changedInputs = computeChangedInputs(buildRecord)

if let reporter = reporter {
for input in inputsInvalidatedByExternals {
Expand Down Expand Up @@ -274,7 +307,6 @@ extension IncrementalCompilationState.FirstWaveComputer {

// Find the inputs that have changed since last compilation, or were marked as needed a build
private func computeChangedInputs(
_ moduleDependencyGraph: ModuleDependencyGraph,
_ outOfDateBuildRecord: BuildRecord
) -> [ChangedInput] {
jobsInPhases.compileJobs.compactMap { job in
Expand Down
2 changes: 0 additions & 2 deletions Tests/SwiftDriverTests/ExplicitModuleBuildTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -913,7 +913,6 @@ final class ExplicitModuleBuildTests: XCTestCase {
try incrementalDriver.run(jobs: incrementalJobs)
XCTAssertFalse(incrementalDriver.diagnosticEngine.hasErrors)
let state = try XCTUnwrap(incrementalDriver.incrementalCompilationState)
XCTAssertTrue(state.mandatoryJobsInOrder.contains { $0.kind == .emitModule })
XCTAssertTrue(state.jobsAfterCompiles.contains { $0.kind == .verifyModuleInterface })

// TODO: emitModule job should run again if interface is deleted.
Expand All @@ -923,7 +922,6 @@ final class ExplicitModuleBuildTests: XCTestCase {
var reDriver = try Driver(args: invocationArguments + ["-color-diagnostics"])
let _ = try reDriver.planBuild()
let reState = try XCTUnwrap(reDriver.incrementalCompilationState)
XCTAssertFalse(reState.mandatoryJobsInOrder.contains { $0.kind == .emitModule })
XCTAssertFalse(reState.jobsAfterCompiles.contains { $0.kind == .verifyModuleInterface })
}
}
Expand Down
67 changes: 67 additions & 0 deletions Tests/SwiftDriverTests/IncrementalCompilationTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -645,6 +645,55 @@ extension IncrementalCompilationTests {
linking
}
}

func testExplicitIncrementalEmitModuleOnly() throws {
guard let sdkArgumentsForTesting = try Driver.sdkArgumentsForTesting()
else {
throw XCTSkip("Cannot perform this test on this host")
}

let args = [
"swiftc",
"-module-name", module,
"-emit-module", "-emit-module-path",
derivedDataPath.appending(component: module + ".swiftmodule").pathString,
"-incremental",
"-driver-show-incremental",
"-driver-show-job-lifecycle",
"-save-temps",
"-output-file-map", OFM.pathString,
"-no-color-diagnostics"
] + inputPathsAndContents.map {$0.0.pathString}.sorted() + explicitBuildArgs + sdkArgumentsForTesting

// Initial build
_ = try doABuildWithoutExpectations(arguments: args)

// Subsequent build, ensure module does not get re-emitted since inputs have not changed
_ = try doABuild(
whenAutolinking: autolinkLifecycleExpectedDiags,
arguments: args
) {
readGraph
explicitIncrementalScanReuseCache(serializedDepScanCachePath.pathString)
explicitIncrementalScanCacheSerialized(serializedDepScanCachePath.pathString)
queuingInitial("main", "other")
}

touch("main")
touch("other")
// Subsequent build, ensure module re-emitted since inputs changed
_ = try doABuild(
whenAutolinking: autolinkLifecycleExpectedDiags,
arguments: args
) {
readGraph
explicitIncrementalScanReuseCache(serializedDepScanCachePath.pathString)
explicitIncrementalScanCacheSerialized(serializedDepScanCachePath.pathString)
queuingInitial("main", "other")
emittingModule(module)
schedulingPostCompileJobs
}
}
}

extension IncrementalCompilationTests {
Expand Down Expand Up @@ -1770,6 +1819,14 @@ extension IncrementalCompilationTests {
}
}

private func doABuild(
whenAutolinking autolinkExpectedDiags: [Diagnostic.Message],
arguments: [String],
@DiagsBuilder expecting expectedDiags: () -> [Diagnostic.Message]
) throws -> Driver {
try doABuild(whenAutolinking: autolinkExpectedDiags, expecting: expectedDiags(), arguments: arguments)
}

private func doABuildWithoutExpectations(arguments: [String]) throws -> Driver {
// If not checking, print out the diagnostics
let diagnosticEngine = DiagnosticsEngine(handlers: [
Expand Down Expand Up @@ -1859,6 +1916,16 @@ extension DiagVerifiable {
@DiagsBuilder func explicitDependencyModuleOlderThanInput(_ dependencyModuleName: String) -> [Diagnostic.Message] {
"Dependency module \(dependencyModuleName) is older than input file"
}
@DiagsBuilder func startEmitModule(_ moduleName: String) -> [Diagnostic.Message] {
"Starting Emitting module for \(moduleName)"
}
@DiagsBuilder func finishEmitModule(_ moduleName: String) -> [Diagnostic.Message] {
"Finished Emitting module for \(moduleName)"
}
@DiagsBuilder func emittingModule(_ moduleName: String) -> [Diagnostic.Message] {
startEmitModule(moduleName)
finishEmitModule(moduleName)
}
@DiagsBuilder func startCompilingExplicitClangDependency(_ dependencyModuleName: String) -> [Diagnostic.Message] {
"Starting Compiling Clang module \(dependencyModuleName)"
}
Expand Down