Skip to content

Cache DownloadableArtifacts with GeneratorEngine #34

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

Merged
merged 20 commits into from
Nov 3, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
4db7eaf
Add `SystemSQLite` target for future use in caching
MaxDesiatov Oct 25, 2023
3ea95f4
Add `GeneratorEngine` module with tests and macros
MaxDesiatov Oct 25, 2023
73c1f4e
Fix EngineTests after API updates
MaxDesiatov Oct 25, 2023
a5e5254
Merge branch 'main' into maxd/engine
MaxDesiatov Oct 26, 2023
c525ec9
Fix duplicate `SystemSQLite` target
MaxDesiatov Oct 26, 2023
09201ea
Add `libsqlite-dev` dependency to `Dockerfile`
MaxDesiatov Oct 26, 2023
80a4173
Bump default version to 5.9 in `Docker/Dockerfile`
MaxDesiatov Oct 26, 2023
b5304e8
Use `libsqlite3-dev` instead of `libsqlite-dev` in `Dockerfile`
MaxDesiatov Oct 26, 2023
c96bda6
Fix incompatibility with swift-corelibs-foundation
MaxDesiatov Oct 26, 2023
2d677b5
Address PR feedback
MaxDesiatov Oct 30, 2023
5ccecd4
Avoid passing keys as strings in most places
MaxDesiatov Oct 30, 2023
aef9fcb
Merge branch 'main' into maxd/engine
MaxDesiatov Oct 30, 2023
996fd27
Merge branch 'main' into maxd/engine
MaxDesiatov Oct 31, 2023
e477ac5
Cache `DownloadableArtifacts` with `GenratorEngine`
MaxDesiatov Oct 30, 2023
6ca73f1
Remove redundant checkum validation
MaxDesiatov Oct 30, 2023
a9d1054
Merge branch 'main' into maxd/engine-downloads
MaxDesiatov Nov 3, 2023
6385163
Fix `ArchitectureMappingTests` failure
MaxDesiatov Nov 3, 2023
105e809
Merge branch 'main' into maxd/engine-downloads
MaxDesiatov Nov 3, 2023
b4de18f
Merge branch 'main' into maxd/engine-downloads
MaxDesiatov Nov 3, 2023
49255cb
Bump swift-syntax to 509.0.2 to fix macros crash
MaxDesiatov Nov 3, 2023
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
4 changes: 2 additions & 2 deletions Package.resolved
Original file line number Diff line number Diff line change
Expand Up @@ -113,8 +113,8 @@
"kind" : "remoteSourceControl",
"location" : "https://github.com/apple/swift-syntax.git",
"state" : {
"revision" : "ffa3cd6fc2aa62adbedd31d3efaf7c0d86a9f029",
"version" : "509.0.1"
"revision" : "6ad4ea24b01559dde0773e3d091f1b9e36175036",
"version" : "509.0.2"
}
},
{
Expand Down
2 changes: 1 addition & 1 deletion Package.swift
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ let package = Package(
.package(url: "https://github.com/apple/swift-nio.git", from: "2.58.0"),
.package(url: "https://github.com/apple/swift-nio-extras.git", from: "1.19.0"),
.package(url: "https://github.com/apple/swift-log.git", from: "1.5.3"),
.package(url: "https://github.com/apple/swift-syntax.git", from: "509.0.1"),
.package(url: "https://github.com/apple/swift-syntax.git", from: "509.0.2"),
],
targets: [
// Targets are the basic building blocks of a package. A target can define a module or a test suite.
Expand Down
10 changes: 8 additions & 2 deletions Sources/GeneratorCLI/GeneratorCLI.swift
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ struct GeneratorCLI: AsyncParsableCommand {
let linuxDistribution = try LinuxDistribution(name: linuxDistributionName, version: linuxDistributionVersion)

let elapsed = try await ContinuousClock().measure {
try await SwiftSDKGenerator(
let generator = try await SwiftSDKGenerator(
hostCPUArchitecture: self.hostArch,
targetCPUArchitecture: self.targetArch,
swiftVersion: self.swiftVersion,
Expand All @@ -98,7 +98,13 @@ struct GeneratorCLI: AsyncParsableCommand {
shouldUseDocker: self.withDocker,
isVerbose: self.verbose
)
.generateBundle(shouldGenerateFromScratch: !self.incremental)
do {
try await generator.generateBundle(shouldGenerateFromScratch: !self.incremental)
try await generator.shutDown()
} catch {
try await generator.shutDown()
throw error
}
Comment on lines +101 to +107
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be nice add a withGenerator wrapper which could take care of shutting down and rethrowing if necessary.

}

print("\nTime taken for this generator run: \(elapsed.intervalString).")
Expand Down
8 changes: 4 additions & 4 deletions Sources/GeneratorEngine/Cache/FileCacheRecord.swift
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@

import struct SystemPackage.FilePath

struct FileCacheRecord {
public struct FileCacheRecord: Sendable {
let path: FilePath
let hash: String
}
Expand All @@ -23,14 +23,14 @@ extension FileCacheRecord: Codable {
case hash
}

// FIXME: `Codable` on `FilePath` is broken
init(from decoder: any Decoder) throws {
// FIXME: `Codable` on `FilePath` is broken, thus all `Codable` types with `FilePath` properties need a custom impl.
public init(from decoder: any Decoder) throws {
let container = try decoder.container(keyedBy: CodingKeys.self)
self.path = try FilePath(container.decode(String.self, forKey: .path))
self.hash = try container.decode(String.self, forKey: .hash)
}

func encode(to encoder: any Encoder) throws {
public func encode(to encoder: any Encoder) throws {
var container = encoder.container(keyedBy: CodingKeys.self)
try container.encode(self.path.string, forKey: .path)
try container.encode(self.hash, forKey: .hash)
Expand Down
10 changes: 5 additions & 5 deletions Sources/GeneratorEngine/Engine.swift
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@ public actor Engine {
/// `defer { engine.shutDown }` on the line that follows this initializer call.
/// - Parameter fileSystem: Implementation of a file system this engine should use.
/// - Parameter cacheLocation: Location of cache storage used by the engine.
/// - Parameter httpClient: HTTP client to use in queries that need it.
/// - Parameter logger: Logger to use during queries execution.
public init(
_ fileSystem: any FileSystem,
Expand Down Expand Up @@ -63,7 +62,7 @@ public actor Engine {
/// Executes a given query if no cached result of it is available. Otherwise fetches the result from engine's cache.
/// - Parameter query: A query value to execute.
/// - Returns: A file path to query's result recorded in a file.
public subscript(_ query: some QueryProtocol) -> FilePath {
public subscript(_ query: some QueryProtocol) -> FileCacheRecord {
get async throws {
var hashFunction = SHA512()
query.hash(with: &hashFunction)
Expand All @@ -78,7 +77,7 @@ public actor Engine {

if fileHash == fileRecord.hash {
self.cacheHits += 1
return fileRecord.path
return fileRecord
}
}

Expand All @@ -90,11 +89,12 @@ public actor Engine {
try await $0.hash(with: &hashFunction)
}
let resultHash = hashFunction.finalize()
let result = FileCacheRecord(path: resultPath, hash: resultHash.description)

// FIXME: update `SQLiteBackedCache` to store `resultHash` directly instead of relying on string conversions
try self.resultsCache.set(key, to: FileCacheRecord(path: resultPath, hash: resultHash.description))
try self.resultsCache.set(key, to: result)

return resultPath
return result
}
}
}
81 changes: 13 additions & 68 deletions Sources/SwiftSDKGenerator/Artifacts/DownloadableArtifacts.swift
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
//===----------------------------------------------------------------------===//

import struct Foundation.URL
import GeneratorEngine
import struct SystemPackage.FilePath

/// Information about the OS for which the artifact is built, if it's downloaded as prebuilt.
Expand Down Expand Up @@ -45,74 +46,26 @@ enum ArtifactOS: Hashable {

typealias CPUMapping = [Triple.CPU: String]

/// SHA256 hashes of binary LLVM artifacts known to the generator.
private let knownLLVMBinariesVersions: [ArtifactOS: [String: CPUMapping]] = [
.macOS: [
"15.0.7": [
Triple.CPU.arm64: "867c6afd41158c132ef05a8f1ddaecf476a26b91c85def8e124414f9a9ba188d",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A side effect of using these hashes was that we also verified that the upstream tarballs hadn't changed from the known versions. LLVM signs its binaries using GPG, so in future we could have an option to verify those signatures. This would require the user to download and validate LLVM's public key.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree, checking signatures is the right way to do it. But code on main that handled checksums was inconsistent and didn't warn users on checksums mismatch. It would only redownload new archives from remote URLs on a mismatch or if a checksum was absent. From that perspective, we didn't regress, and the caching behaviour is actually improved.

],
"16.0.0": [
Triple.CPU.arm64: "2041587b90626a4a87f0de14a5842c14c6c3374f42c8ed12726ef017416409d9",
],
"16.0.1": [
Triple.CPU.arm64: "cb487fa991f047dc79ae36430cbb9ef14621c1262075373955b1d97215c75879",
],
"16.0.4": [
Triple.CPU.arm64: "429b8061d620108fee636313df55a0602ea0d14458c6d3873989e6b130a074bd",
],
"16.0.5": [
Triple.CPU.arm64: "1aed0787417dd915f0101503ce1d2719c8820a2c92d4a517bfc4044f72035bcc",
],
],
]

/// SHA256 hashes of binary Swift artifacts known to the generator.
private let knownSwiftBinariesVersions: [ArtifactOS: [String: CPUMapping]] = [
.linux(.ubuntu(.jammy)): [
"5.7.3-RELEASE": [
.arm64: "75003d5a995292ae3f858b767fbb89bc3edee99488f4574468a0e44341aec55b",
],
"5.8-RELEASE": [
.arm64: "12ea2df36f9af0aefa74f0989009683600978f62223e7dd73b627c90c7fe9273",
],
"5.9-RELEASE": [
.arm64: "30b289e02f7e03c380744ea97fdf0e96985dff504b0f09de23e098fdaf6513f3",
.x86_64: "bca015e9d727ca39385d7e5b5399f46302d54a02218d40d1c3063662ffc6b42f",
],
],
.macOS: [
"5.7.3-RELEASE": [
.arm64: "ba3516845eb8f4469a8bb06a273687f05791187324a3843996af32a73a2a687d",
.x86_64: "ba3516845eb8f4469a8bb06a273687f05791187324a3843996af32a73a2a687d",
],
"5.8-RELEASE": [
.arm64: "9b6cc56993652ca222c86a2d6b7b66abbd50bb92cc526efc2b23d47d40002097",
.x86_64: "9b6cc56993652ca222c86a2d6b7b66abbd50bb92cc526efc2b23d47d40002097",
],
"5.9-RELEASE": [
.arm64: "3cf7a4b2f3efcfcb4fef42b6588a7b1c54f7b0f2d0a479f41c3e1620b045f48e",
.x86_64: "3cf7a4b2f3efcfcb4fef42b6588a7b1c54f7b0f2d0a479f41c3e1620b045f48e",
],
],
]

private let knownLLVMSourcesVersions: [String: String] = [
"16.0.5": "37f540124b9cfd4680666e649f557077f9937c9178489cea285a672e714b2863",
]

public struct DownloadableArtifacts: Sendable {
public struct Item: Sendable {
struct DownloadableArtifacts: Sendable {
@CacheKey
struct Item: Sendable {
let remoteURL: URL
var localPath: FilePath
let checksum: String?
let isPrebuilt: Bool
}

let hostSwift: Item
private(set) var hostLLVM: Item
let targetSwift: Item

let allItems: [Item]
private let shouldUseDocker: Bool
var allItems: [Item] {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixing a bug here, where allItems wasn't recomputed after running useLLVMSources.

if self.shouldUseDocker {
[self.hostSwift, self.hostLLVM]
} else {
[self.hostSwift, self.hostLLVM, self.targetSwift]
}
}

private let versions: VersionsConfiguration
private let paths: PathsConfiguration
Expand All @@ -136,7 +89,6 @@ public struct DownloadableArtifacts: Sendable {
),
localPath: paths.artifactsCachePath
.appending("host_swift_\(versions.swiftVersion)_\(hostTriple).pkg"),
checksum: knownSwiftBinariesVersions[hostArtifactsOS]?[versions.swiftVersion]?[hostTriple.cpu],
isPrebuilt: true
)

Expand All @@ -152,7 +104,6 @@ public struct DownloadableArtifacts: Sendable {
)!,
localPath: paths.artifactsCachePath
.appending("host_llvm_\(versions.lldVersion)_\(hostTriple).tar.xz"),
checksum: knownLLVMBinariesVersions[hostArtifactsOS]?[versions.lldVersion]?[hostTriple.cpu],
isPrebuilt: true
)

Expand All @@ -161,15 +112,10 @@ public struct DownloadableArtifacts: Sendable {
remoteURL: versions.swiftDownloadURL(),
localPath: paths.artifactsCachePath
.appending("target_swift_\(versions.swiftVersion)_\(targetTriple).tar.gz"),
checksum: knownSwiftBinariesVersions[targetArtifactsOS]?[versions.swiftVersion]?[targetTriple.cpu],
isPrebuilt: true
)

self.allItems = if shouldUseDocker {
[self.hostSwift, self.hostLLVM]
} else {
[self.hostSwift, self.hostLLVM, self.targetSwift]
}
self.shouldUseDocker = shouldUseDocker
}

mutating func useLLVMSources() {
Expand All @@ -185,7 +131,6 @@ public struct DownloadableArtifacts: Sendable {
)!,
localPath: self.paths.artifactsCachePath
.appending("llvm_\(self.versions.lldVersion).src.tar.xz"),
checksum: knownLLVMSourcesVersions[self.versions.lldVersion],
isPrebuilt: false
)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,16 +12,36 @@

import AsyncAlgorithms
import AsyncHTTPClient
import GeneratorEngine
import RegexBuilder

import class Foundation.ByteCountFormatter
import struct Foundation.URL

import struct SystemPackage.FilePath

private let ubuntuAMD64Mirror = "http://gb.archive.ubuntu.com/ubuntu"
private let ubuntuARM64Mirror = "http://ports.ubuntu.com/ubuntu-ports"

private let byteCountFormatter = ByteCountFormatter()

@Query
struct DownloadQuery {
let artifact: DownloadableArtifacts.Item

func run(engine: Engine) async throws -> FilePath {
print("Downloading remote artifact not available in local cache: \(self.artifact.remoteURL)")
let stream = await engine.httpClient.streamDownloadProgress(for: self.artifact)
.removeDuplicates(by: didProgressChangeSignificantly)
.throttle(for: .seconds(1))

for try await item in stream {
report(progress: item.progress, for: item.artifact)
}
return self.artifact.localPath
}
}

extension SwiftSDKGenerator {
func downloadArtifacts(_ client: HTTPClient) async throws {
logGenerationStep("Downloading required toolchain packages...")
Expand All @@ -35,42 +55,19 @@ extension SwiftSDKGenerator {
downloadableArtifacts.useLLVMSources()
}

let hostSwiftProgressStream = client.streamDownloadProgress(for: downloadableArtifacts.hostSwift)
.removeDuplicates(by: didProgressChangeSignificantly)
let hostLLVMProgressStream = client.streamDownloadProgress(for: downloadableArtifacts.hostLLVM)
.removeDuplicates(by: didProgressChangeSignificantly)

print("Using these URLs for downloads:")

for artifact in downloadableArtifacts.allItems {
print(artifact.remoteURL)
}

// FIXME: some code duplication is necessary due to https://github.com/apple/swift-async-algorithms/issues/226
if shouldUseDocker {
let stream = combineLatest(hostSwiftProgressStream, hostLLVMProgressStream)
.throttle(for: .seconds(1))

for try await (swiftProgress, llvmProgress) in stream {
report(progress: swiftProgress, for: downloadableArtifacts.hostSwift)
report(progress: llvmProgress, for: downloadableArtifacts.hostLLVM)
let results = try await withThrowingTaskGroup(of: FileCacheRecord.self) { group in
for item in self.downloadableArtifacts.allItems {
print(item.remoteURL)
group.addTask {
try await self.engine[DownloadQuery(artifact: item)]
}
}
} else {
let targetSwiftProgressStream = client.streamDownloadProgress(for: downloadableArtifacts.targetSwift)
.removeDuplicates(by: didProgressChangeSignificantly)

let stream = combineLatest(
hostSwiftProgressStream,
hostLLVMProgressStream,
targetSwiftProgressStream
)
.throttle(for: .seconds(1))

for try await (hostSwiftProgress, hostLLVMProgress, targetSwiftProgress) in stream {
report(progress: hostSwiftProgress, for: downloadableArtifacts.hostSwift)
report(progress: hostLLVMProgress, for: downloadableArtifacts.hostLLVM)
report(progress: targetSwiftProgress, for: downloadableArtifacts.targetSwift)
var result = [FileCacheRecord]()
for try await file in group {
result.append(file)
}
return result
}
}

Expand Down Expand Up @@ -224,14 +221,14 @@ extension HTTPClient {
/// larger than 1MiB. Returns `false` otherwise.
@Sendable
private func didProgressChangeSignificantly(
previous: FileDownloadDelegate.Progress,
current: FileDownloadDelegate.Progress
previous: ArtifactDownloadProgress,
current: ArtifactDownloadProgress
) -> Bool {
guard previous.totalBytes == current.totalBytes else {
guard previous.progress.totalBytes == current.progress.totalBytes else {
return true
}

return current.receivedBytes - previous.receivedBytes > 1024 * 1024 * 1024
return current.progress.receivedBytes - previous.progress.receivedBytes > 1024 * 1024 * 1024
}

private func report(progress: FileDownloadDelegate.Progress, for artifact: DownloadableArtifacts.Item) {
Expand Down
Loading