Skip to content
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

Add multiple cache storages feature #156

Merged
merged 32 commits into from
Nov 18, 2024
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
32 commits
Select commit Hold shift + click to select a range
6d35c14
Add multiple cache storages feature
ikesyo Nov 7, 2024
5953d6c
Pass storages to `CacheSystem.cacheFrameworks`
ikesyo Nov 8, 2024
a24f7c9
Refactor to remove isProducingCacheEnabled
ikesyo Nov 8, 2024
d7bd9ff
Refactor to remove isConsumingCacheEnabled
ikesyo Nov 8, 2024
e7b20c5
Improve logging
ikesyo Nov 8, 2024
0f1c98a
debugging
ikesyo Nov 8, 2024
8750153
Add test case for storages cache mode
ikesyo Nov 11, 2024
8c7c16e
Update `build-pipeline.md` for `.storages` cache mode
ikesyo Nov 11, 2024
f6cbef1
debugging
ikesyo Nov 11, 2024
413bb12
Add StorageConfig struct
ikesyo Nov 12, 2024
c3d9efa
Address naming convention issues
ikesyo Nov 12, 2024
25f5fcb
Replace `case storage` with static func
ikesyo Nov 12, 2024
82ec106
Replace `case disabled` with static func
ikesyo Nov 12, 2024
086970f
Merge branch 'main' into multiple-cache-storages
ikesyo Nov 12, 2024
7434f6b
Rename to LocalDiskCacheStorage
ikesyo Nov 12, 2024
a451614
Introduce ProjectCacheStorage
ikesyo Nov 12, 2024
c22ed6f
Replace `case project` with static func (using ProjectCacheStorage)
ikesyo Nov 12, 2024
0006fbb
Introduce CachePolicy and replace CacheMode with an array of CachePolicy
ikesyo Nov 12, 2024
4d8abab
Update documentation
ikesyo Nov 12, 2024
76ae5af
Avoid direct usages of `type(of: storage)`
ikesyo Nov 12, 2024
ff1d483
Make LocalDiskCacheStorage and ProjectCacheStorage internal
ikesyo Nov 14, 2024
4712f89
Use some over any for arguments
ikesyo Nov 14, 2024
3d333e6
[CI] macos-15 image is required to use Xcode 16
ikesyo Nov 14, 2024
0c81a2e
Add info log when a cache is not found when restoring as well as `.su…
ikesyo Nov 14, 2024
d71053b
Fix format
ikesyo Nov 14, 2024
ed52625
Use some over any for arguments
ikesyo Nov 14, 2024
4de26b5
Add inline comment
ikesyo Nov 14, 2024
bced0e2
Reduce duplication
ikesyo Nov 14, 2024
dfe443c
Revert "Reduce duplication"
ikesyo Nov 14, 2024
acdd251
Reduce duplication
ikesyo Nov 14, 2024
afafa8d
Revert "debugging"
ikesyo Nov 14, 2024
2720cde
Add `[Runner.Options.CachePolicy].disabled`
ikesyo Nov 15, 2024
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
34 changes: 16 additions & 18 deletions Sources/ScipioKit/Producer/Cache/CacheSystem.swift
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,6 @@ struct CacheSystem: Sendable {
static let defaultParalellNumber = 8
private let pinsStore: PinsStore
private let outputDirectory: URL
private let storage: (any CacheStorage)?
Copy link
Collaborator Author

@ikesyo ikesyo Nov 11, 2024

Choose a reason for hiding this comment

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

Due to the introduction of cache policies, storages may be different on when saving caches and when restoring caches. So removed the property and passes by arguments instead.

private let fileSystem: any FileSystem

struct CacheTarget: Hashable, Sendable {
Expand Down Expand Up @@ -139,18 +138,23 @@ struct CacheSystem: Sendable {
init(
pinsStore: PinsStore,
outputDirectory: URL,
storage: (any CacheStorage)?,
fileSystem: any FileSystem = localFileSystem
) {
self.pinsStore = pinsStore
self.outputDirectory = outputDirectory
self.storage = storage
self.fileSystem = fileSystem
}

func cacheFrameworks(_ targets: Set<CacheTarget>) async {
let chunked = targets.chunks(ofCount: storage?.parallelNumber ?? CacheSystem.defaultParalellNumber)
func cacheFrameworks(_ targets: Set<CacheTarget>, storages: [any CacheStorage]) async {
for storage in storages {
await cacheFrameworks(targets, storage: storage)
}
}

private func cacheFrameworks(_ targets: Set<CacheTarget>, storage: any CacheStorage) async {
ikesyo marked this conversation as resolved.
Show resolved Hide resolved
let chunked = targets.chunks(ofCount: storage.parallelNumber ?? CacheSystem.defaultParalellNumber)

let storageType = type(of: storage)
ikesyo marked this conversation as resolved.
Show resolved Hide resolved
for chunk in chunked {
await withTaskGroup(of: Void.self) { group in
for target in chunk {
Expand All @@ -159,10 +163,10 @@ struct CacheSystem: Sendable {
let frameworkPath = outputDirectory.appendingPathComponent(frameworkName)
do {
logger.info(
"🚀 Cache \(frameworkName) to cache storage",
"🚀 Cache \(frameworkName) to cache storage: \(storageType)",
metadata: .color(.green)
)
try await cacheFramework(target, at: frameworkPath)
try await cacheFramework(target, at: frameworkPath, storage: storage)
} catch {
logger.warning("⚠️ Can't create caches for \(frameworkPath.path)")
}
Expand All @@ -173,10 +177,10 @@ struct CacheSystem: Sendable {
}
}

private func cacheFramework(_ target: CacheTarget, at frameworkPath: URL) async throws {
private func cacheFramework(_ target: CacheTarget, at frameworkPath: URL, storage: any CacheStorage) async throws {
let cacheKey = try await calculateCacheKey(of: target)

try await storage?.cacheFramework(frameworkPath, for: cacheKey)
try await storage.cacheFramework(frameworkPath, for: cacheKey)
}

func generateVersionFile(for target: CacheTarget) async throws {
Expand Down Expand Up @@ -210,8 +214,8 @@ struct CacheSystem: Sendable {
case failed(LocalizedError?)
case noCache
}
func restoreCacheIfPossible(target: CacheTarget) async -> RestoreResult {
guard let storage = storage else { return .noCache }

func restoreCacheIfPossible(target: CacheTarget, storage: any CacheStorage) async -> RestoreResult {
ikesyo marked this conversation as resolved.
Show resolved Hide resolved
do {
let cacheKey = try await calculateCacheKey(of: target)
if try await storage.existsValidCache(for: cacheKey) {
Expand All @@ -225,12 +229,6 @@ struct CacheSystem: Sendable {
}
}

private func fetchArtifacts(target: CacheTarget, to destination: URL) async throws {
Copy link
Collaborator Author

@ikesyo ikesyo Nov 11, 2024

Choose a reason for hiding this comment

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

This was unused.

guard let storage = storage else { return }
let cacheKey = try await calculateCacheKey(of: target)
try await storage.fetchArtifacts(for: cacheKey, to: destination)
}

func calculateCacheKey(of target: CacheTarget) async throws -> SwiftPMCacheKey {
let targetName = target.buildProduct.target.name
let pin = try retrievePin(package: target.buildProduct.package)
Expand All @@ -247,7 +245,7 @@ struct CacheSystem: Sendable {
buildOptions: buildOptions,
clangVersion: clangVersion,
xcodeVersion: xcodeVersion,
scipioVersion: currentScipioVersion
scipioVersion: "578ce98d236e79dad3e473cb11153e867be07174" // TODO: revert
)
}

Expand Down
169 changes: 117 additions & 52 deletions Sources/ScipioKit/Producer/FrameworkProducer.swift
Original file line number Diff line number Diff line change
Expand Up @@ -16,22 +16,6 @@ struct FrameworkProducer {
private let fileSystem: any FileSystem
private let toolchainEnvironment: [String: String]?

private var cacheStorage: (any CacheStorage)? {
switch cacheMode {
case .disabled, .project: return nil
case .storage(let storage, _): return storage
}
}

private var isProducingCacheEnabled: Bool {
switch cacheMode {
case .disabled: return false
case .project: return true
case .storage(_, let actors):
return actors.contains(.producer)
}
}

private var shouldGenerateVersionFile: Bool {
// cacheMode is not disabled
if case .disabled = cacheMode {
Expand Down Expand Up @@ -104,20 +88,17 @@ struct FrameworkProducer {
buildOptions: buildOptionsForProduct
)
})

let pinsStore = try descriptionPackage.workspace.pinsStore.load()
let cacheSystem = CacheSystem(
pinsStore: pinsStore,
outputDirectory: outputDir
)

let cacheSystem = CacheSystem(pinsStore: pinsStore,
outputDirectory: outputDir,
storage: cacheStorage)
let cacheEnabledTargets: Set<CacheSystem.CacheTarget>
if cacheMode.isConsumingCacheEnabled {
cacheEnabledTargets = await restoreAllAvailableCaches(
availableTargets: Set(allTargets),
cacheSystem: cacheSystem
)
} else {
cacheEnabledTargets = []
}
let cacheEnabledTargets = await restoreAllAvailableCachesIfNeeded(
availableTargets: Set(allTargets),
cacheSystem: cacheSystem
)

let targetsToBuild = allTargets.subtracting(cacheEnabledTargets)

Expand All @@ -129,9 +110,7 @@ struct FrameworkProducer {
)
}

if isProducingCacheEnabled {
await cacheSystem.cacheFrameworks(Set(targetsToBuild))
}
await cacheFrameworksIfNeeded(Set(targetsToBuild), cacheSystem: cacheSystem)

if shouldGenerateVersionFile {
// Versionfiles should be generate for all targets
Expand All @@ -141,22 +120,93 @@ struct FrameworkProducer {
}
}

private func restoreAllAvailableCaches(
private func restoreAllAvailableCachesIfNeeded(
availableTargets: Set<CacheSystem.CacheTarget>,
cacheSystem: CacheSystem
) async -> Set<CacheSystem.CacheTarget> {
let chunked = availableTargets.chunks(ofCount: cacheStorage?.parallelNumber ?? CacheSystem.defaultParalellNumber)
let cacheStorages: [any CacheStorage]

switch cacheMode {
case .disabled:
return []
case .project:
ikesyo marked this conversation as resolved.
Show resolved Hide resolved
// For `.project`, just checking whether the valid caches (already built frameworks under the project)
// exist or not (not restoring anything from external locations).
return await restoreCachesForTargets(
availableTargets,
cacheSystem: cacheSystem,
cacheStorage: nil
)
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

here:

if isValidCache && exists {
logger.info(
"✅ Valid \(product.target.name).xcframework (\(expectedCacheKeyHash)) is exists. Skip building.", metadata: .color(.green)
)
return true
} else {
if exists {
logger.warning("⚠️ Existing \(frameworkName) is outdated.", metadata: .color(.yellow))
logger.info("🗑️ Delete \(frameworkName)", metadata: .color(.red))
try fileSystem.removeFileTree(outputPath.absolutePath)
}
guard let cacheStorage else {
return false
}

case .storage(let storage, let actors):
guard actors.contains(.consumer) else { return [] }
cacheStorages = [storage]
case .storages(let storages):
let storagesWithConsumer = storages.compactMap { storage, actors in
actors.contains(.consumer) ? storage : nil
}
guard !storagesWithConsumer.isEmpty else { return [] }
cacheStorages = storagesWithConsumer
}

var remainingTargets = availableTargets
var restored: Set<CacheSystem.CacheTarget> = []

for index in cacheStorages.indices {
let storage = cacheStorages[index]

let logSuffix = "[\(index)] \(type(of: storage))"
if index == cacheStorages.startIndex {
logger.info(
"▶️ Starting restoration with cache storage: \(logSuffix)",
metadata: .color(.green)
)
} else {
logger.info(
"⏭️ Falling back to next cache storage: \(logSuffix)",
metadata: .color(.green)
)
}

let restoredPerStorage = await restoreCachesForTargets(
remainingTargets,
cacheSystem: cacheSystem,
cacheStorage: storage
)
restored.formUnion(restoredPerStorage)

logger.info(
"⏸️ Restoration finished with cache storage: \(logSuffix)",
metadata: .color(.green)
)

remainingTargets.subtract(restoredPerStorage)
if remainingTargets.isEmpty {
ikesyo marked this conversation as resolved.
Show resolved Hide resolved
break
}
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

if all frameworks are successfully restored, we don't need to proceed to next cache storage.

Copy link
Owner

Choose a reason for hiding this comment

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

[not mandatory] it's kindly to comment this

}

logger.info("⏹️ Restoration finished", metadata: .color(.green))
return restored
}

private func restoreCachesForTargets(
_ targets: Set<CacheSystem.CacheTarget>,
cacheSystem: CacheSystem,
cacheStorage: (any CacheStorage)?
) async -> Set<CacheSystem.CacheTarget> {
ikesyo marked this conversation as resolved.
Show resolved Hide resolved
let chunked = targets.chunks(ofCount: cacheStorage?.parallelNumber ?? CacheSystem.defaultParalellNumber)

var restored: Set<CacheSystem.CacheTarget> = []
for chunk in chunked {
let restorer = Restorer(outputDir: outputDir, cacheMode: cacheMode, fileSystem: fileSystem)
let restorer = Restorer(outputDir: outputDir, fileSystem: fileSystem)
await withTaskGroup(of: CacheSystem.CacheTarget?.self) { group in
for target in chunk {
group.addTask {
do {
let restored = try await restorer.restore(
target: target,
cacheSystem: cacheSystem
cacheSystem: cacheSystem,
cacheStorage: cacheStorage
)
return restored ? target : nil
} catch {
Expand All @@ -175,25 +225,23 @@ struct FrameworkProducer {
/// Sendable interface to provide restore caches
private struct Restorer: Sendable {
let outputDir: URL
let cacheMode: Runner.Options.CacheMode
let fileSystem: any FileSystem

// Return true if pre-built artifact is available (already existing or restored from cache)
func restore(
target: CacheSystem.CacheTarget,
cacheSystem: CacheSystem
cacheSystem: CacheSystem,
cacheStorage: (any CacheStorage)?
) async throws -> Bool {
let product = target.buildProduct
let frameworkName = product.frameworkName
let outputPath = outputDir.appendingPathComponent(product.frameworkName)
let exists = fileSystem.exists(outputPath.absolutePath)

guard cacheMode.isConsumingCacheEnabled else {
return false
}
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is checked within restoreAllAvailableCachesIfNeeded so should not be needed.

let expectedCacheKey = try await cacheSystem.calculateCacheKey(of: target)
let isValidCache = await cacheSystem.existsValidCache(cacheKey: expectedCacheKey)
let expectedCacheKeyHash = try expectedCacheKey.calculateChecksum()

if isValidCache && exists {
logger.info(
"✅ Valid \(product.target.name).xcframework (\(expectedCacheKeyHash)) is exists. Skip building.", metadata: .color(.green)
Expand All @@ -205,7 +253,12 @@ struct FrameworkProducer {
logger.info("🗑️ Delete \(frameworkName)", metadata: .color(.red))
try fileSystem.removeFileTree(outputPath.absolutePath)
}
let restoreResult = await cacheSystem.restoreCacheIfPossible(target: target)

guard let cacheStorage else {
return false
}

let restoreResult = await cacheSystem.restoreCacheIfPossible(target: target, storage: cacheStorage)
switch restoreResult {
case .succeeded:
logger.info("✅ Restore \(frameworkName) (\(expectedCacheKeyHash)) from cache storage.", metadata: .color(.green))
Expand Down Expand Up @@ -260,6 +313,29 @@ struct FrameworkProducer {
return []
}

private func cacheFrameworksIfNeeded(_ targets: Set<CacheSystem.CacheTarget>, cacheSystem: CacheSystem) async {
switch cacheMode {
case .disabled:
// no-op
break
case .project:
// For `.project` which is not tied to any (external) storages, we don't need to do anything.
// The built frameworks under the project themselves are treated as valid caches.
break
case .storage(let storage, let actors):
if actors.contains(.producer) {
await cacheSystem.cacheFrameworks(targets, storages: [storage])
}
case .storages(let storages):
let storagesWithProducer = storages.compactMap { storage, actors in
actors.contains(.producer) ? storage : nil
}
if !storagesWithProducer.isEmpty {
await cacheSystem.cacheFrameworks(targets, storages: storagesWithProducer)
}
}
}

private func generateVersionFile(for target: CacheSystem.CacheTarget, using cacheSystem: CacheSystem) async {
do {
try await cacheSystem.generateVersionFile(for: target)
Expand All @@ -268,14 +344,3 @@ struct FrameworkProducer {
}
}
}

extension Runner.Options.CacheMode {
fileprivate var isConsumingCacheEnabled: Bool {
switch self {
case .disabled: return false
case .project: return true
case .storage(_, let actors):
return actors.contains(.consumer)
}
}
}
1 change: 1 addition & 0 deletions Sources/ScipioKit/Runner.swift
Original file line number Diff line number Diff line change
Expand Up @@ -219,6 +219,7 @@ extension Runner {
case disabled
case project
case storage(any CacheStorage, Set<CacheActorKind>)
ikesyo marked this conversation as resolved.
Show resolved Hide resolved
ikesyo marked this conversation as resolved.
Show resolved Hide resolved
case storages([(storage: any CacheStorage, actors: Set<CacheActorKind>)])
}

public enum PlatformSpecifier: Equatable {
Expand Down
3 changes: 1 addition & 2 deletions Tests/ScipioKitTests/CacheSystemTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -95,8 +95,7 @@ final class CacheSystemTests: XCTestCase {
)
let cacheSystem = CacheSystem(
pinsStore: try descriptionPackage.workspace.pinsStore.load(),
outputDirectory: FileManager.default.temporaryDirectory.appendingPathComponent("XCFrameworks"),
storage: nil
outputDirectory: FileManager.default.temporaryDirectory.appendingPathComponent("XCFrameworks")
)
let testingPackage = descriptionPackage
.graph
Expand Down
Loading