Skip to content

Commit

Permalink
[CAS] Don't create the same CAS multiple times from the same Oracle
Browse files Browse the repository at this point in the history
If the oracle has been used to create a CAS for the path, reuse the CAS
if it is asked to create the same CAS again. This not only save the work
of creating the CAS, but also avoid a potential problem that the CAS
closing synchronization doesn't work across threads in the same process
due to its uses of file lock.
  • Loading branch information
cachemeifyoucan committed Dec 19, 2023
1 parent 8e47ffe commit fbc27b6
Show file tree
Hide file tree
Showing 6 changed files with 63 additions and 22 deletions.
2 changes: 1 addition & 1 deletion Sources/SwiftDriver/Driver/Driver.swift
Original file line number Diff line number Diff line change
Expand Up @@ -275,7 +275,7 @@ public struct Driver {
let useClangIncludeTree: Bool

/// CAS instance used for compilation.
var cas: SwiftScanCAS? = nil
@_spi(Testing) public var cas: SwiftScanCAS? = nil

/// Is swift caching enabled.
lazy var isCachingEnabled: Bool = {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -165,11 +165,23 @@ public class InterModuleDependencyOracle {
return diags.isEmpty ? nil : diags
}

public func createCAS(pluginPath: AbsolutePath?, onDiskPath: AbsolutePath?, pluginOptions: [(String, String)]) throws -> SwiftScanCAS {
public func getCAS(pluginPath: AbsolutePath?, onDiskPath: AbsolutePath?, pluginOptions: [(String, String)]) throws -> SwiftScanCAS {
guard let swiftScan = swiftScanLibInstance else {
fatalError("Attempting to reset scanner cache with no scanner instance.")
}
return try swiftScan.createCAS(pluginPath: pluginPath?.pathString, onDiskPath: onDiskPath?.pathString, pluginOptions: pluginOptions)
// Use synchronized queue to avoid creating multiple OnDisk CAS at the same location as that will leave to synchronization issues.
// Assuming plugin CAS will handle synchronization by itself.
return try queue.sync {
if let path = onDiskPath, pluginPath == nil {
if let cas = createdCASMap[path] {
return cas
}
let cas = try swiftScan.createCAS(pluginPath: nil, onDiskPath: path.pathString, pluginOptions: [])
createdCASMap[path] = cas
return cas
}
return try swiftScan.createCAS(pluginPath: pluginPath?.pathString, onDiskPath: onDiskPath?.pathString, pluginOptions: pluginOptions)
}
}

private var hasScannerInstance: Bool { self.swiftScanLibInstance != nil }
Expand All @@ -181,5 +193,8 @@ public class InterModuleDependencyOracle {
private var swiftScanLibInstance: SwiftScan? = nil

internal let scannerRequiresPlaceholderModules: Bool

/// Storing the CAS created via on-disk-path.
internal var createdCASMap: [AbsolutePath: SwiftScanCAS] = [:]
}

Original file line number Diff line number Diff line change
Expand Up @@ -173,9 +173,9 @@ public extension Driver {
}
}
if !fallbackToFrontend && isCachingEnabled {
self.cas = try interModuleDependencyOracle.createCAS(pluginPath: try getCASPluginPath(),
onDiskPath: try getOnDiskCASPath(),
pluginOptions: try getCASPluginOptions())
self.cas = try interModuleDependencyOracle.getCAS(pluginPath: try getCASPluginPath(),
onDiskPath: try getOnDiskCASPath(),
pluginOptions: try getCASPluginOptions())
}
return fallbackToFrontend
}
Expand Down
6 changes: 6 additions & 0 deletions Sources/SwiftDriver/SwiftScan/SwiftScanCAS.swift
Original file line number Diff line number Diff line change
Expand Up @@ -244,6 +244,12 @@ public final class SwiftScanCAS {
}
}

extension SwiftScanCAS: Equatable {
static public func == (lhs: SwiftScanCAS, rhs: SwiftScanCAS) -> Bool {
return lhs.cas == rhs.cas
}
}

extension swiftscan_cached_compilation_t {
func convert(_ lib: SwiftScan) -> CachedCompilation {
return CachedCompilation(self, lib: lib)
Expand Down
46 changes: 32 additions & 14 deletions Tests/SwiftDriverTests/CachingBuildTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -205,6 +205,8 @@ private func checkCachingBuildJobDependencies(job: Job,


final class CachingBuildTests: XCTestCase {
let dependencyOracle = InterModuleDependencyOracle()

override func setUpWithError() throws {
try super.setUpWithError()

Expand Down Expand Up @@ -499,12 +501,12 @@ final class CachingBuildTests: XCTestCase {
"-cache-compile-job", "-cas-path", casPath.nativePathString(escaped: true),
"-working-directory", path.nativePathString(escaped: true),
main.nativePathString(escaped: true)] + sdkArgumentsForTesting,
env: ProcessEnv.vars)
env: ProcessEnv.vars,
interModuleDependencyOracle: dependencyOracle)
let jobs = try driver.planBuild()
try driver.run(jobs: jobs)
XCTAssertFalse(driver.diagnosticEngine.hasErrors)

let dependencyOracle = InterModuleDependencyOracle()
let scanLibPath = try XCTUnwrap(driver.toolchain.lookupSwiftScanLib())
guard try dependencyOracle
.verifyOrCreateScannerInstance(fileSystem: localFileSystem,
Expand All @@ -513,7 +515,12 @@ final class CachingBuildTests: XCTestCase {
return
}

let cas = try dependencyOracle.createCAS(pluginPath: nil, onDiskPath: casPath, pluginOptions: [])
let cas = try dependencyOracle.getCAS(pluginPath: nil, onDiskPath: casPath, pluginOptions: [])
if let driverCAS = driver.cas {
XCTAssertEqual(cas, driverCAS, "CAS should only be created once")
} else {
XCTFail("Cached compilation doesn't have a CAS")
}
try checkCASForResults(jobs: jobs, cas: cas, fs: driver.fileSystem)
}
}
Expand Down Expand Up @@ -556,9 +563,9 @@ final class CachingBuildTests: XCTestCase {
"-pch-output-dir", PCHPath.nativePathString(escaped: true),
FooInstallPath.appending(component: "Foo.swiftmodule").nativePathString(escaped: true)]
+ sdkArgumentsForTesting,
env: ProcessEnv.vars)
env: ProcessEnv.vars,
interModuleDependencyOracle: dependencyOracle)

let dependencyOracle = InterModuleDependencyOracle()
let scanLibPath = try XCTUnwrap(fooBuildDriver.toolchain.lookupSwiftScanLib())
guard try dependencyOracle
.verifyOrCreateScannerInstance(fileSystem: localFileSystem,
Expand All @@ -574,7 +581,12 @@ final class CachingBuildTests: XCTestCase {
try fooBuildDriver.run(jobs: fooJobs)
XCTAssertFalse(fooBuildDriver.diagnosticEngine.hasErrors)

let cas = try dependencyOracle.createCAS(pluginPath: nil, onDiskPath: casPath, pluginOptions: [])
let cas = try dependencyOracle.getCAS(pluginPath: nil, onDiskPath: casPath, pluginOptions: [])
if let driverCAS = fooBuildDriver.cas {
XCTAssertEqual(cas, driverCAS, "CAS should only be created once")
} else {
XCTFail("Cached compilation doesn't have a CAS")
}
try checkCASForResults(jobs: fooJobs, cas: cas, fs: fooBuildDriver.fileSystem)

var driver = try Driver(args: ["swiftc",
Expand All @@ -585,7 +597,8 @@ final class CachingBuildTests: XCTestCase {
"-cache-compile-job", "-cas-path", casPath.nativePathString(escaped: true),
"-working-directory", path.nativePathString(escaped: true),
main.nativePathString(escaped: true)] + sdkArgumentsForTesting,
env: ProcessEnv.vars)
env: ProcessEnv.vars,
interModuleDependencyOracle: dependencyOracle)
// This is currently not supported.
XCTAssertThrowsError(try driver.planBuild()) {
XCTAssertEqual($0 as? Driver.Error, .unsupportedConfigurationForCaching("module Foo has prebuilt header dependency"))
Expand Down Expand Up @@ -623,8 +636,8 @@ final class CachingBuildTests: XCTestCase {
"-Xcc", "-ivfsoverlay", "-Xcc", vfsoverlay.nativePathString(escaped: true),
"-disable-clang-target",
main.nativePathString(escaped: true)] + sdkArgumentsForTesting,
env: ProcessEnv.vars)
let dependencyOracle = InterModuleDependencyOracle()
env: ProcessEnv.vars,
interModuleDependencyOracle: dependencyOracle)
let scanLibPath = try XCTUnwrap(driver.toolchain.lookupSwiftScanLib())
guard try dependencyOracle
.verifyOrCreateScannerInstance(fileSystem: localFileSystem,
Expand Down Expand Up @@ -760,11 +773,11 @@ final class CachingBuildTests: XCTestCase {
"-scanner-prefix-map", testInputsPath.description + "=/^src",
"-scanner-prefix-map", path.description + "=/^tmp",
main.nativePathString(escaped: true)] + sdkArgumentsForTesting,
env: ProcessEnv.vars)
env: ProcessEnv.vars,
interModuleDependencyOracle: dependencyOracle)
guard driver.isFrontendArgSupported(.scannerPrefixMap) else {
throw XCTSkip("frontend doesn't support prefix map")
}
let dependencyOracle = InterModuleDependencyOracle()
let scanLibPath = try XCTUnwrap(driver.toolchain.lookupSwiftScanLib())
guard try dependencyOracle
.verifyOrCreateScannerInstance(fileSystem: localFileSystem,
Expand Down Expand Up @@ -830,12 +843,12 @@ final class CachingBuildTests: XCTestCase {
"-output-file-map", ofm.nativePathString(escaped: true),
"-working-directory", path.nativePathString(escaped: true),
main.nativePathString(escaped: true)] + sdkArgumentsForTesting,
env: ProcessEnv.vars)
env: ProcessEnv.vars,
interModuleDependencyOracle: dependencyOracle)
let jobs = try driver.planBuild()
try driver.run(jobs: jobs)
XCTAssertFalse(driver.diagnosticEngine.hasErrors)

let dependencyOracle = InterModuleDependencyOracle()
let scanLibPath = try XCTUnwrap(driver.toolchain.lookupSwiftScanLib())
guard try dependencyOracle
.verifyOrCreateScannerInstance(fileSystem: localFileSystem,
Expand All @@ -844,7 +857,12 @@ final class CachingBuildTests: XCTestCase {
return
}

let cas = try dependencyOracle.createCAS(pluginPath: nil, onDiskPath: casPath, pluginOptions: [])
let cas = try dependencyOracle.getCAS(pluginPath: nil, onDiskPath: casPath, pluginOptions: [])
if let driverCAS = driver.cas {
XCTAssertEqual(cas, driverCAS, "CAS should only be created once")
} else {
XCTFail("Cached compilation doesn't have a CAS")
}
try checkCASForResults(jobs: jobs, cas: cas, fs: driver.fileSystem)

// try replan the job and make sure some key command-line options are generated.
Expand Down
6 changes: 4 additions & 2 deletions Tests/TestUtilities/DriverExtensions.swift
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,8 @@ extension Driver {
env: [String: String] = ProcessEnv.vars,
diagnosticsEngine: DiagnosticsEngine = DiagnosticsEngine(handlers: [Driver.stderrDiagnosticsHandler]),
fileSystem: FileSystem = localFileSystem,
integratedDriver: Bool = true
integratedDriver: Bool = true,
interModuleDependencyOracle: InterModuleDependencyOracle? = nil
) throws {
let executor = try SwiftDriverExecutor(diagnosticsEngine: diagnosticsEngine,
processSet: ProcessSet(),
Expand All @@ -34,7 +35,8 @@ extension Driver {
diagnosticsOutput: .engine(diagnosticsEngine),
fileSystem: fileSystem,
executor: executor,
integratedDriver: integratedDriver)
integratedDriver: integratedDriver,
interModuleDependencyOracle: interModuleDependencyOracle)
}

/// For tests that need to set the sdk path.
Expand Down

0 comments on commit fbc27b6

Please sign in to comment.