Improve resolveBuildProducts performance

giginet · Jul 7, 2024 · 8e28a73 · 8e28a73
1 parent 2d2f361
commit 8e28a73
Showing 1 changed file with 74 additions and 53 deletions.
diff --git a/Sources/ScipioKit/DescriptionPackage.swift b/Sources/ScipioKit/DescriptionPackage.swift
@@ -149,10 +149,58 @@ struct DescriptionPackage {
 }
 
 extension DescriptionPackage {
+    func resolveBuildProducts() throws -> [BuildProduct] {
+        let resolver = BuildProductsResolver(descriptionPackage: self)
+        return try resolver.resolveBuildProducts()
+    }
+}
+
+struct BuildProduct: Hashable, Sendable {
+    var package: ResolvedPackage
+    var target: ResolvedTarget
+
+    var frameworkName: String {
+        "\(target.name.packageNamed()).xcframework"
+    }
+
+    var binaryTarget: BinaryTarget? {
+        target.underlyingTarget as? BinaryTarget
+    }
+
+    func hash(into hasher: inout Hasher) {
+        // Important: Relevant for swift-6.0+ toolchain versions. For the versions below
+        // this change has no effect as SwiftPM provides its own proper `Hashable`
+        // implementations for both `ResolvedPackage` and `ResolvedTarget`.
+        //
+        // We cannot directly use `ResolvedModule.id` here as `id` also includes `BuildTriple`.
+        // The reason for this is that `ResolvedModule.buildTriple` is parent-dependent; more
+        // specifically, the same `ResolvedModule` will have a different build triple depending
+        // on whether it is in a root or dependency position.
+        // For more context, see `ResolvedModule.updateBuildTriplesOfDependencies`.
+        //
+        // At the same time, build triples remain irrelevant for the `Scipio` use case where the
+        // build product must be the same regardless of the triple. Meanwhile, the target name and
+        // package identity remain relevant and unambiguously identify the build product.
+        hasher.combine(target.name)
+        hasher.combine(package.identity)
+    }
+}
+
+
+private final class BuildProductsResolver {
+    private var visitedTargets: Set<ResolvedTarget> = []
+    let descriptionPackage: DescriptionPackage
+
+    init(descriptionPackage: DescriptionPackage) {
+        self.descriptionPackage = descriptionPackage
+    }
+
     func resolveBuildProducts() throws -> [BuildProduct] {
         let targetsToBuild = try targetsToBuild()
         var products = try targetsToBuild.flatMap(resolveBuildProduct(from:))
 
+        visitedTargets.removeAll()
+
         let productMap: [String: BuildProduct] = Dictionary(products.map { ($0.target.name, $0) }) { $1 }
         func resolvedTargetToBuildProduct(_ target: ResolvedTarget) -> BuildProduct {
             guard let product = productMap[target.name] else {
@@ -174,41 +222,18 @@ extension DescriptionPackage {
             }
         } catch {
             switch error {
-            case GraphError.unexpectedCycle: throw Error.cycleDetected
+            case GraphError.unexpectedCycle: throw DescriptionPackage.Error.cycleDetected
             default: throw error
             }
         }
 
         return products.reversed()
     }
 
-    private func targetsToBuild() throws -> Set<ResolvedTarget> {
-        switch mode {
-        case .createPackage:
-            // In create mode, all products should be built
-            // In future update, users will be enable to specify products want to build
-            let rootPackage = try fetchRootPackage()
-            let productNamesToBuild = rootPackage.manifest.products.map { $0.name }
-            let productsToBuild = rootPackage.products.filter { productNamesToBuild.contains($0.name) }
-            return Set(productsToBuild.flatMap(\.targets))
-        case .prepareDependencies:
-            // In prepare mode, all targets should be built
-            // In future update, users will be enable to specify targets want to build
-            return Set(try fetchRootPackage().targets)
-        }
-    }
-
-    private func fetchRootPackage() throws -> ResolvedPackage {
-        guard let rootPackage = graph.rootPackages.first else {
-            throw Error.packageNotDefined
-        }
-        return rootPackage
-    }
-
     private func resolveBuildProduct(from rootTarget: ResolvedTarget) throws -> Set<BuildProduct> {
         let dependencyProducts = Set(try rootTarget.recursiveTargetDependencies().flatMap(buildProducts(from:)))
 
-        switch mode {
+        switch descriptionPackage.mode {
         case .createPackage:
             // In create mode, rootTarget should be built
             let rootTargetProducts = try buildProducts(from: rootTarget)
@@ -220,43 +245,39 @@ extension DescriptionPackage {
     }
 
     private func buildProducts(from target: ResolvedTarget) throws -> Set<BuildProduct> {
-        guard let package = graph.package(for: target) else {
+        guard let package = descriptionPackage.graph.package(for: target),
+              !visitedTargets.contains(target)
+        else {
             return []
         }
 
+        visitedTargets.insert(target)
+
         let rootTargetProduct = BuildProduct(package: package, target: target)
         let dependencyProducts = try target.recursiveDependencies().compactMap(\.target).flatMap(buildProducts(from:))
         return Set([rootTargetProduct] + dependencyProducts)
     }
-}
-
-struct BuildProduct: Hashable, Sendable {
-    var package: ResolvedPackage
-    var target: ResolvedTarget
-
-    var frameworkName: String {
-        "\(target.name.packageNamed()).xcframework"
-    }
 
-    var binaryTarget: BinaryTarget? {
-        target.underlyingTarget as? BinaryTarget
+    private func targetsToBuild() throws -> Set<ResolvedTarget> {
+        switch descriptionPackage.mode {
+        case .createPackage:
+            // In create mode, all products should be built
+            // In future update, users will be enable to specify products want to build
+            let rootPackage = try fetchRootPackage()
+            let productNamesToBuild = rootPackage.manifest.products.map { $0.name }
+            let productsToBuild = rootPackage.products.filter { productNamesToBuild.contains($0.name) }
+            return Set(productsToBuild.flatMap(\.targets))
+        case .prepareDependencies:
+            // In prepare mode, all targets should be built
+            // In future update, users will be enable to specify targets want to build
+            return Set(try fetchRootPackage().targets)
+        }
     }
 
-    func hash(into hasher: inout Hasher) {
-        // Important: Relevant for swift-6.0+ toolchain versions. For the versions below
-        // this change has no effect as SwiftPM provides its own proper `Hashable`
-        // implementations for both `ResolvedPackage` and `ResolvedTarget`.
-        //
-        // We cannot directly use `ResolvedModule.id` here as `id` also includes `BuildTriple`.
-        // The reason for this is that `ResolvedModule.buildTriple` is parent-dependent; more
-        // specifically, the same `ResolvedModule` will have a different build triple depending
-        // on whether it is in a root or dependency position.
-        // For more context, see `ResolvedModule.updateBuildTriplesOfDependencies`.
-        //
-        // At the same time, build triples remain irrelevant for the `Scipio` use case where the 
-        // build product must be the same regardless of the triple. Meanwhile, the target name and
-        // package identity remain relevant and unambiguously identify the build product.
-        hasher.combine(target.name)
-        hasher.combine(package.identity)
+    private func fetchRootPackage() throws -> ResolvedPackage {
+        guard let rootPackage = descriptionPackage.graph.rootPackages.first else {
+            throw DescriptionPackage.Error.packageNotDefined
+        }
+        return rootPackage
     }
 }