From 47719010e0fe22ca7d5078a7c2f7f68fc3a1d646 Mon Sep 17 00:00:00 2001 From: Valeriy Van Date: Wed, 11 Sep 2024 16:00:46 +0300 Subject: [PATCH 1/3] Use task group instead of semaphore --- .../Commands/PackageCommands/APIDiff.swift | 53 ++++++++++--------- 1 file changed, 28 insertions(+), 25 deletions(-) diff --git a/Sources/Commands/PackageCommands/APIDiff.swift b/Sources/Commands/PackageCommands/APIDiff.swift index 6f8eb05b53f..1975a4e50ee 100644 --- a/Sources/Commands/PackageCommands/APIDiff.swift +++ b/Sources/Commands/PackageCommands/APIDiff.swift @@ -119,38 +119,41 @@ struct APIDiff: AsyncSwiftCommand { swiftCommandState: swiftCommandState ) - let results = ThreadSafeArrayStore() - let group = DispatchGroup() - let semaphore = DispatchSemaphore(value: Int(try buildSystem.buildPlan.destinationBuildParameters.workers)) var skippedModules: Set = [] - for module in modulesToDiff { - let moduleBaselinePath = baselineDir.appending("\(module).json") - guard swiftCommandState.fileSystem.exists(moduleBaselinePath) else { - print("\nSkipping \(module) because it does not exist in the baseline") - skippedModules.insert(module) - continue - } - semaphore.wait() - DispatchQueue.sharedConcurrent.async(group: group) { - do { - if let comparisonResult = try apiDigesterTool.compareAPIToBaseline( - at: moduleBaselinePath, - for: module, - buildPlan: try buildSystem.buildPlan, - except: breakageAllowlistPath - ) { - results.append(comparisonResult) + let results = await withTaskGroup(of: SwiftAPIDigester.ComparisonResult?.self, returning: ThreadSafeArrayStore.self) { taskGroup in + + for module in modulesToDiff { + let moduleBaselinePath = baselineDir.appending("\(module).json") + guard swiftCommandState.fileSystem.exists(moduleBaselinePath) else { + print("\nSkipping \(module) because it does not exist in the baseline") + skippedModules.insert(module) + continue + } + taskGroup.addTask { + do { + if let comparisonResult = try apiDigesterTool.compareAPIToBaseline( + at: moduleBaselinePath, + for: module, + buildPlan: try buildSystem.buildPlan, + except: breakageAllowlistPath + ) { + return comparisonResult + } + } catch { + swiftCommandState.observabilityScope.emit(error: "failed to compare API to baseline", underlyingError: error) } - } catch { - swiftCommandState.observabilityScope.emit(error: "failed to compare API to baseline", underlyingError: error) + return nil } - semaphore.signal() } + let results = ThreadSafeArrayStore() + for await result in taskGroup { + guard let result else { continue } + results.append(result) + } + return results } - group.wait() - let failedModules = modulesToDiff .subtracting(skippedModules) .subtracting(results.map(\.moduleName)) From de7ecfa11a029252a00976ac1053cafb87c6ed0b Mon Sep 17 00:00:00 2001 From: Valeriy Van Date: Wed, 11 Sep 2024 17:52:09 +0300 Subject: [PATCH 2/3] Use regular Array instead of ThreadSafeArrayStore --- Sources/Commands/PackageCommands/APIDiff.swift | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/Sources/Commands/PackageCommands/APIDiff.swift b/Sources/Commands/PackageCommands/APIDiff.swift index 1975a4e50ee..9bfe1121a04 100644 --- a/Sources/Commands/PackageCommands/APIDiff.swift +++ b/Sources/Commands/PackageCommands/APIDiff.swift @@ -121,7 +121,7 @@ struct APIDiff: AsyncSwiftCommand { var skippedModules: Set = [] - let results = await withTaskGroup(of: SwiftAPIDigester.ComparisonResult?.self, returning: ThreadSafeArrayStore.self) { taskGroup in + let results = await withTaskGroup(of: SwiftAPIDigester.ComparisonResult?.self, returning: [SwiftAPIDigester.ComparisonResult].self) { taskGroup in for module in modulesToDiff { let moduleBaselinePath = baselineDir.appending("\(module).json") @@ -146,7 +146,7 @@ struct APIDiff: AsyncSwiftCommand { return nil } } - let results = ThreadSafeArrayStore() + var results = [SwiftAPIDigester.ComparisonResult]() for await result in taskGroup { guard let result else { continue } results.append(result) @@ -161,11 +161,11 @@ struct APIDiff: AsyncSwiftCommand { swiftCommandState.observabilityScope.emit(error: "failed to read API digester output for \(failedModule)") } - for result in results.get() { + for result in results { try self.printComparisonResult(result, observabilityScope: swiftCommandState.observabilityScope) } - guard failedModules.isEmpty && results.get().allSatisfy(\.hasNoAPIBreakingChanges) else { + guard failedModules.isEmpty && results.allSatisfy(\.hasNoAPIBreakingChanges) else { throw ExitCode.failure } } From d8f1df06fd7d7e274ec68ac6a1bbf8fb3fc9313a Mon Sep 17 00:00:00 2001 From: Valeriy Van Date: Thu, 12 Sep 2024 19:27:54 +0300 Subject: [PATCH 3/3] Add import _Concurrency to please outdated CI --- Sources/Commands/PackageCommands/APIDiff.swift | 1 + 1 file changed, 1 insertion(+) diff --git a/Sources/Commands/PackageCommands/APIDiff.swift b/Sources/Commands/PackageCommands/APIDiff.swift index 9bfe1121a04..416c7a40057 100644 --- a/Sources/Commands/PackageCommands/APIDiff.swift +++ b/Sources/Commands/PackageCommands/APIDiff.swift @@ -17,6 +17,7 @@ import Dispatch import PackageGraph import PackageModel import SourceControl +import _Concurrency struct DeprecatedAPIDiff: ParsableCommand { static let configuration = CommandConfiguration(commandName: "experimental-api-diff",