Skip to content

Commit

Permalink
cache binary artifact globally (#7101)
Browse files Browse the repository at this point in the history
motivation: like other dependencies, binary artifacts are good
candidates for user level caching such that they do not need to be
re-downloaded

changes:
* update BinaryArtifactsManager to take cache path and use the cache to
store binary artifacts when downloading them
* update test infra to enable/disable artifacts caching
* update workspaace call sites
* update workspace delegate to indicate when using cached binary
artifact
* update and add tests

rdar://111774147
  • Loading branch information
tomerd authored Nov 27, 2023
1 parent 8e318dc commit e38c81d
Show file tree
Hide file tree
Showing 9 changed files with 374 additions and 56 deletions.
1 change: 1 addition & 0 deletions Sources/Commands/PackageTools/ComputeChecksum.swift
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ struct ComputeChecksum: SwiftCommand {
authorizationProvider: swiftTool.getAuthorizationProvider(),
hostToolchain: swiftTool.getHostToolchain(),
checksumAlgorithm: SHA256(),
cachePath: .none,
customHTTPClient: .none,
customArchiver: .none,
delegate: .none
Expand Down
20 changes: 14 additions & 6 deletions Sources/Commands/ToolWorkspaceDelegate.swift
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ class ToolWorkspaceDelegate: WorkspaceDelegate {
}
}

self.outputHandler("Fetched \(packageLocation ?? package.description) (\(duration.descriptionInSeconds))", false)
self.outputHandler("Fetched \(packageLocation ?? package.description) from cache (\(duration.descriptionInSeconds))", false)
}

func fetchingPackage(package: PackageIdentity, packageLocation: String?, progress: Int64, total: Int64?) {
Expand Down Expand Up @@ -135,12 +135,16 @@ class ToolWorkspaceDelegate: WorkspaceDelegate {
self.outputHandler("Computed \(location) at \(version) (\(duration.descriptionInSeconds))", false)
}

func willDownloadBinaryArtifact(from url: String) {
self.outputHandler("Downloading binary artifact \(url)", false)
func willDownloadBinaryArtifact(from url: String, fromCache: Bool) {
if fromCache {
self.outputHandler("Fetching binary artifact \(url) from cache", false)
} else {
self.outputHandler("Downloading binary artifact \(url)", false)
}
}

func didDownloadBinaryArtifact(from url: String, result: Result<AbsolutePath, Error>, duration: DispatchTimeInterval) {
guard case .success = result, !self.observabilityScope.errorsReported else {
func didDownloadBinaryArtifact(from url: String, result: Result<(path: AbsolutePath, fromCache: Bool), Error>, duration: DispatchTimeInterval) {
guard case .success(let fetchDetails) = result, !self.observabilityScope.errorsReported else {
return
}

Expand All @@ -155,7 +159,11 @@ class ToolWorkspaceDelegate: WorkspaceDelegate {
}
}

self.outputHandler("Downloaded \(url) (\(duration.descriptionInSeconds))", false)
if fetchDetails.fromCache {
self.outputHandler("Fetched \(url) from cache (\(duration.descriptionInSeconds))", false)
} else {
self.outputHandler("Downloaded \(url) (\(duration.descriptionInSeconds))", false)
}
}

func downloadingBinaryArtifact(from url: String, bytesDownloaded: Int64, totalBytesToDownload: Int64?) {
Expand Down
4 changes: 2 additions & 2 deletions Sources/SPMTestSupport/MockWorkspace.swift
Original file line number Diff line number Diff line change
Expand Up @@ -893,11 +893,11 @@ public final class MockWorkspaceDelegate: WorkspaceDelegate {
// noop
}

public func willDownloadBinaryArtifact(from url: String) {
public func willDownloadBinaryArtifact(from url: String, fromCache: Bool) {
self.append("downloading binary artifact package: \(url)")
}

public func didDownloadBinaryArtifact(from url: String, result: Result<AbsolutePath, Error>, duration: DispatchTimeInterval) {
public func didDownloadBinaryArtifact(from url: String, result: Result<(path: AbsolutePath, fromCache: Bool), Error>, duration: DispatchTimeInterval) {
self.append("finished downloading binary artifact package: \(url)")
}

Expand Down
2 changes: 1 addition & 1 deletion Sources/SourceControl/RepositoryManager.swift
Original file line number Diff line number Diff line change
Expand Up @@ -441,7 +441,7 @@ public class RepositoryManager: Cancellable {
}

/// Sets up the cache directories if they don't already exist.
public func initializeCacheIfNeeded(cachePath: AbsolutePath) throws {
private func initializeCacheIfNeeded(cachePath: AbsolutePath) throws {
// Create the supplied cache directory.
if !self.fileSystem.exists(cachePath) {
try self.fileSystem.createDirectory(cachePath, recursive: true)
Expand Down
159 changes: 127 additions & 32 deletions Sources/Workspace/Workspace+BinaryArtifacts.swift
Original file line number Diff line number Diff line change
Expand Up @@ -26,10 +26,16 @@ extension Workspace {
public struct CustomBinaryArtifactsManager {
let httpClient: LegacyHTTPClient?
let archiver: Archiver?
let useCache: Bool?

public init(httpClient: LegacyHTTPClient? = .none, archiver: Archiver? = .none) {
public init(
httpClient: LegacyHTTPClient? = .none,
archiver: Archiver? = .none,
useCache: Bool? = .none
) {
self.httpClient = httpClient
self.archiver = archiver
self.useCache = useCache
}
}

Expand All @@ -43,13 +49,15 @@ extension Workspace {
private let httpClient: LegacyHTTPClient
private let archiver: Archiver
private let checksumAlgorithm: HashAlgorithm
private let cachePath: AbsolutePath?
private let delegate: Delegate?

public init(
fileSystem: FileSystem,
authorizationProvider: AuthorizationProvider?,
hostToolchain: UserToolchain,
checksumAlgorithm: HashAlgorithm,
cachePath: AbsolutePath?,
customHTTPClient: LegacyHTTPClient?,
customArchiver: Archiver?,
delegate: Delegate?
Expand All @@ -60,6 +68,7 @@ extension Workspace {
self.checksumAlgorithm = checksumAlgorithm
self.httpClient = customHTTPClient ?? LegacyHTTPClient()
self.archiver = customArchiver ?? ZipArchiver(fileSystem: fileSystem)
self.cachePath = cachePath
self.delegate = delegate
}

Expand Down Expand Up @@ -126,7 +135,7 @@ extension Workspace {
return (local: localArtifacts, remote: remoteArtifacts)
}

func download(
func fetch(
_ artifacts: [RemoteArtifact],
artifactsDirectory: AbsolutePath,
observabilityScope: ObservabilityScope
Expand Down Expand Up @@ -229,37 +238,24 @@ extension Workspace {
}

group.enter()
var headers = HTTPClientHeaders()
headers.add(name: "Accept", value: "application/octet-stream")
var request = LegacyHTTPClient.Request.download(
url: artifact.url,
headers: headers,
fileSystem: self.fileSystem,
destination: archivePath
)
request.options.authorizationProvider = self.authorizationProvider?.httpAuthorizationHeader(for:)
request.options.retryStrategy = .exponentialBackoff(maxAttempts: 3, baseDelay: .milliseconds(50))
request.options.validResponseCodes = [200]

let downloadStart: DispatchTime = .now()
self.delegate?.willDownloadBinaryArtifact(from: artifact.url.absoluteString)
observabilityScope.emit(debug: "downloading \(artifact.url) to \(archivePath)")
self.httpClient.execute(
request,
let fetchStart: DispatchTime = .now()
self.fetch(
artifact: artifact,
destination: archivePath,
observabilityScope: observabilityScope,
progress: { bytesDownloaded, totalBytesToDownload in
self.delegate?.downloadingBinaryArtifact(
from: artifact.url.absoluteString,
bytesDownloaded: bytesDownloaded,
totalBytesToDownload: totalBytesToDownload
)
},
completion: { downloadResult in
completion: { fetchResult in
defer { group.leave() }

// TODO: Use the same extraction logic for both remote and local archived artifacts.
switch downloadResult {
case .success:

switch fetchResult {
case .success(let cached):
// TODO: Use the same extraction logic for both remote and local archived artifacts.
group.enter()
observabilityScope.emit(debug: "validating \(archivePath)")
self.archiver.validate(path: archivePath, completion: { validationResult in
Expand Down Expand Up @@ -381,8 +377,8 @@ extension Workspace {
)
self.delegate?.didDownloadBinaryArtifact(
from: artifact.url.absoluteString,
result: .success(artifactPath),
duration: downloadStart.distance(to: .now())
result: .success((path: artifactPath, fromCache: cached)),
duration: fetchStart.distance(to: .now())
)
case .failure(let error):
observabilityScope.emit(.remoteArtifactFailedExtraction(
Expand All @@ -393,7 +389,7 @@ extension Workspace {
self.delegate?.didDownloadBinaryArtifact(
from: artifact.url.absoluteString,
result: .failure(error),
duration: downloadStart.distance(to: .now())
duration: fetchStart.distance(to: .now())
)
}

Expand All @@ -409,7 +405,7 @@ extension Workspace {
self.delegate?.didDownloadBinaryArtifact(
from: artifact.url.absoluteString,
result: .failure(error),
duration: downloadStart.distance(to: .now())
duration: fetchStart.distance(to: .now())
)
}
})
Expand All @@ -423,7 +419,7 @@ extension Workspace {
self.delegate?.didDownloadBinaryArtifact(
from: artifact.url.absoluteString,
result: .failure(error),
duration: downloadStart.distance(to: .now())
duration: fetchStart.distance(to: .now())
)
}
}
Expand Down Expand Up @@ -563,17 +559,116 @@ extension Workspace {
try cancellableArchiver.cancel(deadline: deadline)
}
}

private func fetch(
artifact: RemoteArtifact,
destination: AbsolutePath,
observabilityScope: ObservabilityScope,
progress: @escaping (Int64, Optional<Int64>) -> Void,
completion: @escaping (Result<Bool, Error>) -> Void
) {
// not using cache, download directly
guard let cachePath = self.cachePath else {
self.delegate?.willDownloadBinaryArtifact(from: artifact.url.absoluteString, fromCache: false)
return self.download(
artifact: artifact,
destination: destination,
observabilityScope: observabilityScope,
progress: progress,
completion: { result in
// not fetched from cache
completion(result.map{ _ in false })
}
)
}

// initialize cache if necessary
do {
if !self.fileSystem.exists(cachePath) {
try self.fileSystem.createDirectory(cachePath, recursive: true)
}
} catch {
return completion(.failure(error))
}


// try to fetch from cache, or download and cache
// / FIXME: use better escaping of URL
let cacheKey = artifact.url.absoluteString.spm_mangledToC99ExtendedIdentifier()
let cachedArtifactPath = cachePath.appending(cacheKey)

if self.fileSystem.exists(cachedArtifactPath) {
observabilityScope.emit(debug: "copying cached binary artifact for \(artifact.url) from \(cachedArtifactPath)")
self.delegate?.willDownloadBinaryArtifact(from: artifact.url.absoluteString, fromCache: true)
return completion(
Result.init(catching: {
// copy from cache to destination
try self.fileSystem.copy(from: cachedArtifactPath, to: destination)
return true // fetched from cache
})
)
}

// download to the cache
observabilityScope.emit(debug: "downloading binary artifact for \(artifact.url) to cached at \(cachedArtifactPath)")
self.download(
artifact: artifact,
destination: cachedArtifactPath,
observabilityScope: observabilityScope,
progress: progress,
completion: { result in
self.delegate?.willDownloadBinaryArtifact(from: artifact.url.absoluteString, fromCache: false)
completion(result.flatMap {
Result.init(catching: {
// copy from cache to destination
try self.fileSystem.copy(from: cachedArtifactPath, to: destination)
return false // not fetched from cache
})
})
}
)
}

private func download(
artifact: RemoteArtifact,
destination: AbsolutePath,
observabilityScope: ObservabilityScope,
progress: @escaping (Int64, Optional<Int64>) -> Void,
completion: @escaping (Result<Void, Error>) -> Void
) {
observabilityScope.emit(debug: "downloading \(artifact.url) to \(destination)")

var headers = HTTPClientHeaders()
headers.add(name: "Accept", value: "application/octet-stream")
var request = LegacyHTTPClient.Request.download(
url: artifact.url,
headers: headers,
fileSystem: self.fileSystem,
destination: destination
)
request.options.authorizationProvider = self.authorizationProvider?.httpAuthorizationHeader(for:)
request.options.retryStrategy = .exponentialBackoff(maxAttempts: 3, baseDelay: .milliseconds(50))
request.options.validResponseCodes = [200]

self.httpClient.execute(
request,
progress: progress,
completion: { result in
completion(result.map{ _ in Void() })
}
)
}
}
}

/// Delegate to notify clients about actions being performed by BinaryArtifactsDownloadsManage.
public protocol BinaryArtifactsManagerDelegate {
/// The workspace has started downloading a binary artifact.
func willDownloadBinaryArtifact(from url: String)
func willDownloadBinaryArtifact(from url: String, fromCache: Bool)
/// The workspace has finished downloading a binary artifact.
func didDownloadBinaryArtifact(
from url: String,
result: Result<AbsolutePath, Error>,
result: Result<(path: AbsolutePath, fromCache: Bool), Error>,
duration: DispatchTimeInterval
)
/// The workspace is downloading a binary artifact.
Expand Down Expand Up @@ -833,7 +928,7 @@ extension Workspace {
}

// Download the artifacts
let downloadedArtifacts = try self.binaryArtifactsManager.download(
let downloadedArtifacts = try self.binaryArtifactsManager.fetch(
artifactsToDownload,
artifactsDirectory: self.location.artifactsDirectory,
observabilityScope: observabilityScope
Expand Down
5 changes: 5 additions & 0 deletions Sources/Workspace/Workspace+Configuration.swift
Original file line number Diff line number Diff line change
Expand Up @@ -145,6 +145,11 @@ extension Workspace {
self.sharedCacheDirectory.map { $0.appending(components: "registry", "downloads") }
}

/// Path to the shared repositories cache.
public var sharedBinaryArtifactsCacheDirectory: AbsolutePath? {
self.sharedCacheDirectory.map { $0.appending("artifacts") }
}

/// Create a new workspace location.
///
/// - Parameters:
Expand Down
10 changes: 5 additions & 5 deletions Sources/Workspace/Workspace+Delegation.swift
Original file line number Diff line number Diff line change
Expand Up @@ -126,11 +126,11 @@ public protocol WorkspaceDelegate: AnyObject {
func resolvedFileChanged()

/// The workspace has started downloading a binary artifact.
func willDownloadBinaryArtifact(from url: String)
func willDownloadBinaryArtifact(from url: String, fromCache: Bool)
/// The workspace has finished downloading a binary artifact.
func didDownloadBinaryArtifact(
from url: String,
result: Result<AbsolutePath, Error>,
result: Result<(path: AbsolutePath, fromCache: Bool), Error>,
duration: DispatchTimeInterval
)
/// The workspace is downloading a binary artifact.
Expand Down Expand Up @@ -399,13 +399,13 @@ struct WorkspaceBinaryArtifactsManagerDelegate: Workspace.BinaryArtifactsManager
self.workspaceDelegate = workspaceDelegate
}

func willDownloadBinaryArtifact(from url: String) {
self.workspaceDelegate?.willDownloadBinaryArtifact(from: url)
func willDownloadBinaryArtifact(from url: String, fromCache: Bool) {
self.workspaceDelegate?.willDownloadBinaryArtifact(from: url, fromCache: fromCache)
}

func didDownloadBinaryArtifact(
from url: String,
result: Result<AbsolutePath, Error>,
result: Result<(path: AbsolutePath, fromCache: Bool), Error>,
duration: DispatchTimeInterval
) {
self.workspaceDelegate?.didDownloadBinaryArtifact(from: url, result: result, duration: duration)
Expand Down
1 change: 1 addition & 0 deletions Sources/Workspace/Workspace.swift
Original file line number Diff line number Diff line change
Expand Up @@ -528,6 +528,7 @@ public class Workspace {
authorizationProvider: authorizationProvider,
hostToolchain: hostToolchain,
checksumAlgorithm: checksumAlgorithm,
cachePath: customBinaryArtifactsManager?.useCache == false || !configuration.sharedDependenciesCacheEnabled ? .none : location.sharedBinaryArtifactsCacheDirectory,
customHTTPClient: customBinaryArtifactsManager?.httpClient,
customArchiver: customBinaryArtifactsManager?.archiver,
delegate: delegate.map(WorkspaceBinaryArtifactsManagerDelegate.init(workspaceDelegate:))
Expand Down
Loading

0 comments on commit e38c81d

Please sign in to comment.