Skip to content

Commit 3baeee5

Browse files
authored
Fix PackageGraph traversal performance regressions (#7248)
This restores the performance of traversal of deep package graphs previously available when `ResolvedTarget`, `ResolvedProduct`, and `ResolvedPackage` were reference types. Now instead of recursively applying `Hashable` and `Equatable` conformances to all properties in graph's node, we only hash and compare for equality their identities defined by `Identifiable`. Uses of `Set` on these types are now replaced with the new `IdentifiableSet` type, which uses a dictionary of `[Element.ID: Element]` as its storage. The main benefit of this approach is that build triple of `ResolvedProduct` and `ResolvedTarget` is now a part of their identity. Thus we can still use value types, while the same `Target` from `PackageModel` instantiated as two different `ResolvedTarget`s with different `BuildTriples` will have different identities. Resolves rdar://120861833 Resolves rdar://120711211
1 parent 0d1f370 commit 3baeee5

39 files changed

+401
-193
lines changed

CMakeLists.txt

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,5 +51,8 @@ find_package(dispatch QUIET)
5151
find_package(Foundation QUIET)
5252
find_package(SQLite3 REQUIRED)
5353

54+
# Enable `package` modifier for the whole package.
55+
add_compile_options("$<$<COMPILE_LANGUAGE:Swift>:-package-name;SwiftPM>")
56+
5457
add_subdirectory(Sources)
5558
add_subdirectory(cmake/modules)

Sources/Basics/CMakeLists.txt

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -12,16 +12,18 @@ add_library(Basics
1212
Archiver/ZipArchiver.swift
1313
Archiver/UniversalArchiver.swift
1414
AuthorizationProvider.swift
15-
ByteString+Extensions.swift
1615
Cancellator.swift
16+
Collections/ByteString+Extensions.swift
17+
Collections/Dictionary+Extensions.swift
18+
Collections/IdentifiableSet.swift
19+
Collections/String+Extensions.swift
1720
Concurrency/ConcurrencyHelpers.swift
1821
Concurrency/NSLock+Extensions.swift
1922
Concurrency/SendableBox.swift
2023
Concurrency/ThreadSafeArrayStore.swift
2124
Concurrency/ThreadSafeBox.swift
2225
Concurrency/ThreadSafeKeyValueStore.swift
2326
Concurrency/TokenBucket.swift
24-
Dictionary+Extensions.swift
2527
DispatchTimeInterval+Extensions.swift
2628
EnvironmentVariables.swift
2729
Errors.swift
@@ -53,7 +55,6 @@ add_library(Basics
5355
Sandbox.swift
5456
SendableTimeInterval.swift
5557
Serialization/SerializedJSON.swift
56-
String+Extensions.swift
5758
SwiftVersion.swift
5859
SQLiteBackedCache.swift
5960
Triple+Basics.swift
File renamed without changes.
File renamed without changes.
Lines changed: 109 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,109 @@
1+
//===----------------------------------------------------------------------===//
2+
//
3+
// This source file is part of the Swift open source project
4+
//
5+
// Copyright (c) 2024 Apple Inc. and the Swift project authors
6+
// Licensed under Apache License v2.0 with Runtime Library Exception
7+
//
8+
// See http://swift.org/LICENSE.txt for license information
9+
// See http://swift.org/CONTRIBUTORS.txt for the list of Swift project authors
10+
//
11+
//===----------------------------------------------------------------------===//
12+
13+
/// Replacement for `Set` elements that can't be `Hashable`, but can be `Identifiable`.
14+
public struct IdentifiableSet<Element: Identifiable>: Collection {
15+
public init() {
16+
self.storage = [:]
17+
}
18+
19+
public init(_ sequence: some Sequence<Element>) {
20+
self.storage = .init(pickLastWhenDuplicateFound: sequence)
21+
}
22+
23+
fileprivate typealias Storage = [Element.ID: Element]
24+
25+
public struct Index: Comparable {
26+
public static func < (lhs: IdentifiableSet<Element>.Index, rhs: IdentifiableSet<Element>.Index) -> Bool {
27+
lhs.storageIndex < rhs.storageIndex
28+
}
29+
30+
fileprivate let storageIndex: Storage.Index
31+
}
32+
33+
private var storage: Storage
34+
35+
public var startIndex: Index {
36+
Index(storageIndex: storage.startIndex)
37+
}
38+
39+
public var endIndex: Index {
40+
Index(storageIndex: storage.endIndex)
41+
}
42+
43+
public subscript(position: Index) -> Element {
44+
self.storage[position.storageIndex].value
45+
}
46+
47+
public subscript(id: Element.ID) -> Element? {
48+
self.storage[id]
49+
}
50+
51+
public func index(after i: Index) -> Index {
52+
Index(storageIndex: self.storage.index(after: i.storageIndex))
53+
}
54+
55+
public func union(_ otherSequence: some Sequence<Element>) -> Self {
56+
var result = self
57+
for element in otherSequence {
58+
result.storage[element.id] = element
59+
}
60+
return result
61+
}
62+
63+
public mutating func formUnion(_ otherSequence: some Sequence<Element>) {
64+
for element in otherSequence {
65+
self.storage[element.id] = element
66+
}
67+
}
68+
69+
public func intersection(_ otherSequence: some Sequence<Element>) -> Self {
70+
var keysToRemove = Set(self.storage.keys).subtracting(otherSequence.map(\.id))
71+
var result = Self()
72+
for key in keysToRemove {
73+
result.storage.removeValue(forKey: key)
74+
}
75+
return result
76+
}
77+
78+
public func subtracting(_ otherSequence: some Sequence<Element>) -> Self {
79+
var result = self
80+
for element in otherSequence {
81+
result.storage.removeValue(forKey: element.id)
82+
}
83+
return result
84+
}
85+
86+
public func contains(id: Element.ID) -> Bool {
87+
self.storage.keys.contains(id)
88+
}
89+
}
90+
91+
extension Dictionary where Value: Identifiable, Key == Value.ID {
92+
fileprivate init(pickLastWhenDuplicateFound sequence: some Sequence<Value>) {
93+
self.init(sequence.map { ($0.id, $0) }, uniquingKeysWith: { $1 })
94+
}
95+
}
96+
97+
extension IdentifiableSet: Equatable {
98+
public static func ==(_ lhs: Self, _ rhs: Self) -> Bool {
99+
lhs.storage.keys == rhs.storage.keys
100+
}
101+
}
102+
103+
extension IdentifiableSet: Hashable {
104+
public func hash(into hasher: inout Hasher) {
105+
for key in self.storage.keys {
106+
hasher.combine(key)
107+
}
108+
}
109+
}
File renamed without changes.

Sources/Build/BuildDescription/ProductBuildDescription.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -311,7 +311,7 @@ public final class ProductBuildDescription: SPMBuildCore.ProductBuildDescription
311311
// setting is the package-level right now. We might need to figure out a better
312312
// answer for libraries if/when we support specifying deployment target at the
313313
// target-level.
314-
args += try self.buildParameters.targetTripleArgs(for: self.product.targets[0])
314+
args += try self.buildParameters.targetTripleArgs(for: self.product.targets[self.product.targets.startIndex])
315315

316316
// Add arguments from declared build settings.
317317
args += self.buildSettingsFlags

Sources/Build/BuildManifest/LLBuildManifestBuilder+Clang.swift

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ extension LLBuildManifestBuilder {
3333
}
3434

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

Sources/Build/BuildManifest/LLBuildManifestBuilder+Product.swift

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ extension LLBuildManifestBuilder {
2626
testInputs = [testBundleInfoPlistPath]
2727

2828
self.manifest.addWriteInfoPlistCommand(
29-
principalClass: "\(buildProduct.product.targets[0].c99name).SwiftPMXCTestObserver",
29+
principalClass: "\(buildProduct.product.targets[buildProduct.product.targets.startIndex].c99name).SwiftPMXCTestObserver",
3030
outputPath: testBundleInfoPlistPath
3131
)
3232
} else {
@@ -107,7 +107,7 @@ extension LLBuildManifestBuilder {
107107
outputs: [output]
108108
)
109109

110-
if self.plan.graph.reachableProducts.contains(buildProduct.product) {
110+
if self.plan.graph.reachableProducts.contains(id: buildProduct.product.id) {
111111
if buildProduct.product.type != .test {
112112
self.addNode(output, toTarget: .main)
113113
}

Sources/Build/BuildManifest/LLBuildManifestBuilder+Swift.swift

Lines changed: 10 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -191,8 +191,9 @@ extension LLBuildManifestBuilder {
191191
public func addTargetsToExplicitBuildManifest() throws {
192192
// Sort the product targets in topological order in order to collect and "bubble up"
193193
// their respective dependency graphs to the depending targets.
194-
let nodes: [ResolvedTarget.Dependency] = self.plan.targetMap.keys.map {
195-
ResolvedTarget.Dependency.target($0, conditions: [])
194+
let nodes: [ResolvedTarget.Dependency] = try self.plan.targetMap.keys.compactMap {
195+
guard let target = self.plan.graph.allTargets[$0] else { throw InternalError("unknown target \($0)") }
196+
return ResolvedTarget.Dependency.target(target, conditions: [])
196197
}
197198
let allPackageDependencies = try topologicalSort(nodes, successors: { $0.dependencies })
198199
// Instantiate the inter-module dependency oracle which will cache commonly-scanned
@@ -225,7 +226,7 @@ extension LLBuildManifestBuilder {
225226
// be able to detect such targets' modules.
226227
continue
227228
}
228-
guard let description = plan.targetMap[target] else {
229+
guard let description = plan.targetMap[target.id] else {
229230
throw InternalError("Expected description for target \(target)")
230231
}
231232
switch description {
@@ -327,7 +328,7 @@ extension LLBuildManifestBuilder {
327328
throw InternalError("unknown dependency product for \(dependency)")
328329
}
329330
for dependencyProductTarget in dependencyProduct.targets {
330-
guard let dependencyTargetDescription = self.plan.targetMap[dependencyProductTarget] else {
331+
guard let dependencyTargetDescription = self.plan.targetMap[dependencyProductTarget.id] else {
331332
throw InternalError("unknown dependency target for \(dependencyProductTarget)")
332333
}
333334
try self.addTargetDependencyInfo(
@@ -339,7 +340,7 @@ extension LLBuildManifestBuilder {
339340
// Product dependencies are broken down into the targets that make them up.
340341
guard
341342
let dependencyTarget = dependency.target,
342-
let dependencyTargetDescription = self.plan.targetMap[dependencyTarget]
343+
let dependencyTargetDescription = self.plan.targetMap[dependencyTarget.id]
343344
else {
344345
throw InternalError("unknown dependency target for \(dependency)")
345346
}
@@ -426,18 +427,18 @@ extension LLBuildManifestBuilder {
426427
if target.type == .executable {
427428
// FIXME: Optimize.
428429
let product = try plan.graph.allProducts.first {
429-
try $0.type == .executable && $0.executableTarget == target
430+
try $0.type == .executable && $0.executableTarget.id == target.id
430431
}
431432
if let product {
432-
guard let planProduct = plan.productMap[product] else {
433+
guard let planProduct = plan.productMap[product.id] else {
433434
throw InternalError("unknown product \(product)")
434435
}
435436
try inputs.append(file: planProduct.binaryPath)
436437
}
437438
return
438439
}
439440

440-
switch self.plan.targetMap[target] {
441+
switch self.plan.targetMap[target.id] {
441442
case .swift(let target)?:
442443
inputs.append(file: target.moduleOutputPath)
443444
case .clang(let target)?:
@@ -457,7 +458,7 @@ extension LLBuildManifestBuilder {
457458
case .product(let product, _):
458459
switch product.type {
459460
case .executable, .snippet, .library(.dynamic), .macro:
460-
guard let planProduct = plan.productMap[product] else {
461+
guard let planProduct = plan.productMap[product.id] else {
461462
throw InternalError("unknown product \(product)")
462463
}
463464
// Establish a dependency on binary of the product.

0 commit comments

Comments
 (0)