Fix PackageGraph traversal performance regressions

CMakeLists.txt

@@ -51,5 +51,8 @@ find_package(dispatch QUIET)
 find_package(Foundation QUIET)
 find_package(SQLite3 REQUIRED)
 
+# Enable `package` modifier for the whole package.
+add_compile_options("$<$<COMPILE_LANGUAGE:Swift>:-package-name;SwiftPM>")
+
 add_subdirectory(Sources)
 add_subdirectory(cmake/modules)

Sources/Basics/CMakeLists.txt

@@ -12,16 +12,18 @@ add_library(Basics
   Archiver/ZipArchiver.swift
   Archiver/UniversalArchiver.swift
   AuthorizationProvider.swift
-  ByteString+Extensions.swift
   Cancellator.swift
+  Collections/ByteString+Extensions.swift
+  Collections/Dictionary+Extensions.swift
+  Collections/IdentifiableSet.swift
+  Collections/String+Extensions.swift
   Concurrency/ConcurrencyHelpers.swift
   Concurrency/NSLock+Extensions.swift
   Concurrency/SendableBox.swift
   Concurrency/ThreadSafeArrayStore.swift
   Concurrency/ThreadSafeBox.swift
   Concurrency/ThreadSafeKeyValueStore.swift
   Concurrency/TokenBucket.swift
-  Dictionary+Extensions.swift
   DispatchTimeInterval+Extensions.swift
   EnvironmentVariables.swift
   Errors.swift
@@ -53,7 +55,6 @@ add_library(Basics
   Sandbox.swift
   SendableTimeInterval.swift
   Serialization/SerializedJSON.swift
-  String+Extensions.swift
   SwiftVersion.swift
   SQLiteBackedCache.swift
   Triple+Basics.swift

Sources/Basics/Collections/IdentifiableSet.swift

@@ -0,0 +1,109 @@
+//===----------------------------------------------------------------------===//
+//
+// This source file is part of the Swift open source project
+//
+// Copyright (c) 2024 Apple Inc. and the Swift project authors
+// Licensed under Apache License v2.0 with Runtime Library Exception
+//
+// See http://swift.org/LICENSE.txt for license information
+// See http://swift.org/CONTRIBUTORS.txt for the list of Swift project authors
+//
+//===----------------------------------------------------------------------===//
+
+/// Replacement for `Set` elements that can't be `Hashable`, but can be `Identifiable`.
+public struct IdentifiableSet<Element: Identifiable>: Collection {
+    public init() {
+        self.storage = [:]
+    }
+
+    public init(_ sequence: some Sequence<Element>) {
+        self.storage = .init(pickLastWhenDuplicateFound: sequence)
+    }
+
+    fileprivate typealias Storage = [Element.ID: Element]
+
+    public struct Index: Comparable {
+        public static func < (lhs: IdentifiableSet<Element>.Index, rhs: IdentifiableSet<Element>.Index) -> Bool {
+            lhs.storageIndex < rhs.storageIndex
+        }
+
+        fileprivate let storageIndex: Storage.Index
+    }
+
+    private var storage: Storage
+
+    public var startIndex: Index {
+        Index(storageIndex: storage.startIndex)
+    }
+
+    public var endIndex: Index {
+        Index(storageIndex: storage.endIndex)
+    }
+
+    public subscript(position: Index) -> Element {
+        self.storage[position.storageIndex].value
+    }
+
+    public subscript(id: Element.ID) -> Element? {
+        self.storage[id]
+    }
+
+    public func index(after i: Index) -> Index {
+        Index(storageIndex: self.storage.index(after: i.storageIndex))
+    }
+
+    public func union(_ otherSequence: some Sequence<Element>) -> Self {
+        var result = self
+        for element in otherSequence {
+            result.storage[element.id] = element
+        }
+        return result
+    }
+
+    public mutating func formUnion(_ otherSequence: some Sequence<Element>) {
+        for element in otherSequence {
+            self.storage[element.id] = element
+        }
+    }
+
+    public func intersection(_ otherSequence: some Sequence<Element>) -> Self {
+        var keysToRemove = Set(self.storage.keys).subtracting(otherSequence.map(\.id))
+        var result = Self()
+        for key in keysToRemove {
+            result.storage.removeValue(forKey: key)
+        }
+        return result
+    }
+
+    public func subtracting(_ otherSequence: some Sequence<Element>) -> Self {
+        var result = self
+        for element in otherSequence {
+            result.storage.removeValue(forKey: element.id)
+        }
+        return result
+    }
+
+    public func contains(id: Element.ID) -> Bool {
+        self.storage.keys.contains(id)
+    }
+}
+
+extension Dictionary where Value: Identifiable, Key == Value.ID {
+    fileprivate init(pickLastWhenDuplicateFound sequence: some Sequence<Value>) {
+        self.init(sequence.map { ($0.id, $0) }, uniquingKeysWith: { $1 })
+    }
+}
+
+extension IdentifiableSet: Equatable {
+    public static func ==(_ lhs: Self, _ rhs: Self) -> Bool {
+        lhs.storage.keys == rhs.storage.keys
+    }
+}
+
+extension IdentifiableSet: Hashable {
+    public func hash(into hasher: inout Hasher) {
+        for key in self.storage.keys {
+            hasher.combine(key)
+        }
+    }
+}

Sources/Build/BuildDescription/ProductBuildDescription.swift

@@ -311,7 +311,7 @@ public final class ProductBuildDescription: SPMBuildCore.ProductBuildDescription
         // setting is the package-level right now. We might need to figure out a better
         // answer for libraries if/when we support specifying deployment target at the
         // target-level.
-        args += try self.buildParameters.targetTripleArgs(for: self.product.targets[0])
+        args += try self.buildParameters.targetTripleArgs(for: self.product.targets[self.product.targets.startIndex])
 
         // Add arguments from declared build settings.
         args += self.buildSettingsFlags

Sources/Build/BuildManifest/LLBuildManifestBuilder+Clang.swift

@@ -33,7 +33,7 @@ extension LLBuildManifestBuilder {
         }
 
         func addStaticTargetInputs(_ target: ResolvedTarget) {
-            if case .swift(let desc)? = self.plan.targetMap[target], target.type == .library {
+            if case .swift(let desc)? = self.plan.targetMap[target.id], target.type == .library {
                 inputs.append(file: desc.moduleOutputPath)
             }
         }
@@ -46,7 +46,7 @@ extension LLBuildManifestBuilder {
             case .product(let product, _):
                 switch product.type {
                 case .executable, .snippet, .library(.dynamic), .macro:
-                    guard let planProduct = plan.productMap[product] else {
+                    guard let planProduct = plan.productMap[product.id] else {
                         throw InternalError("unknown product \(product)")
                     }
                     // Establish a dependency on binary of the product.

Sources/Build/BuildManifest/LLBuildManifestBuilder+Product.swift

@@ -26,7 +26,7 @@ extension LLBuildManifestBuilder {
             testInputs = [testBundleInfoPlistPath]
 
             self.manifest.addWriteInfoPlistCommand(
-                principalClass: "\(buildProduct.product.targets[0].c99name).SwiftPMXCTestObserver",
+                principalClass: "\(buildProduct.product.targets[buildProduct.product.targets.startIndex].c99name).SwiftPMXCTestObserver",
                 outputPath: testBundleInfoPlistPath
             )
         } else {
@@ -107,7 +107,7 @@ extension LLBuildManifestBuilder {
             outputs: [output]
         )
 
-        if self.plan.graph.reachableProducts.contains(buildProduct.product) {
+        if self.plan.graph.reachableProducts.contains(id: buildProduct.product.id) {
             if buildProduct.product.type != .test {
                 self.addNode(output, toTarget: .main)
             }

Sources/Build/BuildManifest/LLBuildManifestBuilder+Swift.swift

@@ -191,8 +191,9 @@ extension LLBuildManifestBuilder {
     public func addTargetsToExplicitBuildManifest() throws {
         // Sort the product targets in topological order in order to collect and "bubble up"
         // their respective dependency graphs to the depending targets.
-        let nodes: [ResolvedTarget.Dependency] = self.plan.targetMap.keys.map {
-            ResolvedTarget.Dependency.target($0, conditions: [])
+        let nodes: [ResolvedTarget.Dependency] = try self.plan.targetMap.keys.compactMap {
+            guard let target = self.plan.graph.allTargets[$0] else { throw InternalError("unknown target \($0)") }
+            return ResolvedTarget.Dependency.target(target, conditions: [])
         }
         let allPackageDependencies = try topologicalSort(nodes, successors: { $0.dependencies })
         // Instantiate the inter-module dependency oracle which will cache commonly-scanned
@@ -225,7 +226,7 @@ extension LLBuildManifestBuilder {
                 // be able to detect such targets' modules.
                 continue
             }
-            guard let description = plan.targetMap[target] else {
+            guard let description = plan.targetMap[target.id] else {
                 throw InternalError("Expected description for target \(target)")
             }
             switch description {
@@ -327,7 +328,7 @@ extension LLBuildManifestBuilder {
                     throw InternalError("unknown dependency product for \(dependency)")
                 }
                 for dependencyProductTarget in dependencyProduct.targets {
-                    guard let dependencyTargetDescription = self.plan.targetMap[dependencyProductTarget] else {
+                    guard let dependencyTargetDescription = self.plan.targetMap[dependencyProductTarget.id] else {
                         throw InternalError("unknown dependency target for \(dependencyProductTarget)")
                     }
                     try self.addTargetDependencyInfo(
@@ -339,7 +340,7 @@ extension LLBuildManifestBuilder {
                 // Product dependencies are broken down into the targets that make them up.
                 guard
                     let dependencyTarget = dependency.target,
-                    let dependencyTargetDescription = self.plan.targetMap[dependencyTarget]
+                    let dependencyTargetDescription = self.plan.targetMap[dependencyTarget.id]
                 else {
                     throw InternalError("unknown dependency target for \(dependency)")
                 }
@@ -426,18 +427,18 @@ extension LLBuildManifestBuilder {
             if target.type == .executable {
                 // FIXME: Optimize.
                 let product = try plan.graph.allProducts.first {
-                    try $0.type == .executable && $0.executableTarget == target
+                    try $0.type == .executable && $0.executableTarget.id == target.id
                 }
                 if let product {
-                    guard let planProduct = plan.productMap[product] else {
+                    guard let planProduct = plan.productMap[product.id] else {
                         throw InternalError("unknown product \(product)")
                     }
                     try inputs.append(file: planProduct.binaryPath)
                 }
                 return
             }
 
-            switch self.plan.targetMap[target] {
+            switch self.plan.targetMap[target.id] {
             case .swift(let target)?:
                 inputs.append(file: target.moduleOutputPath)
             case .clang(let target)?:
@@ -457,7 +458,7 @@ extension LLBuildManifestBuilder {
             case .product(let product, _):
                 switch product.type {
                 case .executable, .snippet, .library(.dynamic), .macro:
-                    guard let planProduct = plan.productMap[product] else {
+                    guard let planProduct = plan.productMap[product.id] else {
                         throw InternalError("unknown product \(product)")
                     }
                     // Establish a dependency on binary of the product.

Sources/Build/BuildManifest/LLBuildManifestBuilder.swift

@@ -253,7 +253,7 @@ extension LLBuildManifestBuilder {
     private func addTestDiscoveryGenerationCommand() throws {
         for testDiscoveryTarget in self.plan.targets.compactMap(\.testDiscoveryTargetBuildDescription) {
             let testTargets = testDiscoveryTarget.target.dependencies
-                .compactMap(\.target).compactMap { self.plan.targetMap[$0] }
+                .compactMap(\.target).compactMap { self.plan.targetMap[$0.id] }
             let objectFiles = try testTargets.flatMap { try $0.objects }.sorted().map(Node.file)
             let outputs = testDiscoveryTarget.target.sources.paths
 
@@ -280,7 +280,7 @@ extension LLBuildManifestBuilder {
             // Get the Swift target build descriptions of all discovery targets this synthesized entry point target
             // depends on.
             let discoveredTargetDependencyBuildDescriptions = testEntryPointTarget.target.dependencies
-                .compactMap(\.target)
+                .compactMap(\.target?.id)
                 .compactMap { self.plan.targetMap[$0] }
                 .compactMap(\.testDiscoveryTargetBuildDescription)
 

Sources/Build/BuildOperation.swift

@@ -451,8 +451,8 @@ public final class BuildOperation: PackageStructureDelegate, SPMBuildCore.BuildS
         // Load the package graph.
         let graph = try getPackageGraph()
 
-        let buildToolPluginInvocationResults: [ResolvedTarget: [BuildToolPluginInvocationResult]]
-        let prebuildCommandResults: [ResolvedTarget: [PrebuildCommandResult]]
+        let buildToolPluginInvocationResults: [ResolvedTarget.ID: (target: ResolvedTarget, results: [BuildToolPluginInvocationResult])]
+        let prebuildCommandResults: [ResolvedTarget.ID: [PrebuildCommandResult]]
         // Invoke any build tool plugins in the graph to generate prebuild commands and build commands.
         if let pluginConfiguration, !self.productsBuildParameters.shouldSkipBuilding {
             let buildOperationForPluginDependencies = BuildOperation(
@@ -488,7 +488,7 @@ public final class BuildOperation: PackageStructureDelegate, SPMBuildCore.BuildS
 
             // Surface any diagnostics from build tool plugins.
             var succeeded = true
-            for (target, results) in buildToolPluginInvocationResults {
+            for (_, (target, results)) in buildToolPluginInvocationResults {
                 // There is one result for each plugin that gets applied to a target.
                 for result in results {
                     let diagnosticsEmitter = self.observabilityScope.makeDiagnosticsEmitter {
@@ -513,7 +513,9 @@ public final class BuildOperation: PackageStructureDelegate, SPMBuildCore.BuildS
 
             // Run any prebuild commands provided by build tool plugins. Any failure stops the build.
             prebuildCommandResults = try graph.reachableTargets.reduce(into: [:], { partial, target in
-                partial[target] = try buildToolPluginInvocationResults[target].map { try self.runPrebuildCommands(for: $0) }
+                partial[target.id] = try buildToolPluginInvocationResults[target.id].map {
+                    try self.runPrebuildCommands(for: $0.results)
+                }
             })
         } else {
             buildToolPluginInvocationResults = [:]
@@ -528,8 +530,8 @@ public final class BuildOperation: PackageStructureDelegate, SPMBuildCore.BuildS
                 if unhandledFiles.isEmpty { continue }
 
                 // Subtract out any that were inputs to any commands generated by plugins.
-                if let result = buildToolPluginInvocationResults[target] {
-                    let handledFiles = result.flatMap{ $0.buildCommands.flatMap{ $0.inputFiles } }
+                if let result = buildToolPluginInvocationResults[target.id]?.results {
+                    let handledFiles = result.flatMap { $0.buildCommands.flatMap { $0.inputFiles } }
                     unhandledFiles.subtract(handledFiles)
                 }
                 if unhandledFiles.isEmpty { continue }
@@ -556,7 +558,7 @@ public final class BuildOperation: PackageStructureDelegate, SPMBuildCore.BuildS
             toolsBuildParameters: self.toolsBuildParameters,
             graph: graph,
             additionalFileRules: additionalFileRules,
-            buildToolPluginInvocationResults: buildToolPluginInvocationResults,
+            buildToolPluginInvocationResults: buildToolPluginInvocationResults.mapValues(\.results),
             prebuildCommandResults: prebuildCommandResults,
             disableSandbox: self.pluginConfiguration?.disableSandbox ?? false,
             fileSystem: self.fileSystem,

Sources/Build/BuildOperationBuildSystemDelegateHandler.swift

@@ -382,8 +382,8 @@ public struct BuildDescription: Codable {
         }
         var targetCommandLines: [TargetName: [CommandLineFlag]] = [:]
         var generatedSourceTargets: [TargetName] = []
-        for (target, description) in plan.targetMap {
-            guard case .swift(let desc) = description else {
+        for (targetID, description) in plan.targetMap {
+            guard case .swift(let desc) = description, let target = plan.graph.allTargets[targetID] else {
                 continue
             }
             let buildParameters = plan.buildParameters(for: target)

Sources/Build/BuildPlan/BuildPlan+Clang.swift

@@ -23,7 +23,7 @@ extension BuildPlan {
         for case .target(let dependency, _) in dependencies {
             switch dependency.underlying {
             case is SwiftTarget:
-                if case let .swift(dependencyTargetDescription)? = targetMap[dependency] {
+                if case let .swift(dependencyTargetDescription)? = targetMap[dependency.id] {
                     if let moduleMap = dependencyTargetDescription.moduleMap {
                         clangTarget.additionalFlags += ["-fmodule-map-file=\(moduleMap.pathString)"]
                     }
@@ -34,7 +34,7 @@ extension BuildPlan {
                 clangTarget.additionalFlags += ["-I", target.includeDir.pathString]
 
                 // Add the modulemap of the dependency if it has one.
-                if case let .clang(dependencyTargetDescription)? = targetMap[dependency] {
+                if case let .clang(dependencyTargetDescription)? = targetMap[dependency.id] {
                     if let moduleMap = dependencyTargetDescription.moduleMap {
                         clangTarget.additionalFlags += ["-fmodule-map-file=\(moduleMap.pathString)"]
                     }