Skip to content

Commit 394b430

Browse files
authored
[GH:swiftlang#9304] Explicit source file declaration in Target's sources causes a performance degradation in TargetSourcesBuilder (swiftlang#9385)
Avoid expensive string array operations when performing target source evaluation. Local timings for target source evaluation of protoc before and after: **Before:** 'swift-protobuf': TargetSourcesBuilder.run() for target 'protoc' took 11.544s **After:** 'swift-protobuf': TargetSourcesBuilder.run() for target 'protoc' took 0.190s CPU cycles for full build of swift-protobuf: **Before:** 33.43 G **After:** 5.72 G Motivation: Fixes: swiftlang#9304
1 parent a5a83d1 commit 394b430

File tree

1 file changed

+38
-18
lines changed

1 file changed

+38
-18
lines changed

Sources/PackageLoading/TargetSourcesBuilder.swift

Lines changed: 38 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212

1313
import Basics
1414
import Foundation
15+
import OrderedCollections
1516
import PackageModel
1617
import TSCBasic
1718

@@ -33,7 +34,7 @@ public struct TargetSourcesBuilder {
3334
public let targetPath: Basics.AbsolutePath
3435

3536
/// The list of declared sources in the package manifest.
36-
public let declaredSources: [Basics.AbsolutePath]?
37+
public let declaredSources: OrderedCollections.OrderedSet<Basics.AbsolutePath>?
3738

3839
/// The list of declared resources in the package manifest.
3940
public let declaredResources: [(path: Basics.AbsolutePath, rule: TargetDescription.Resource.Rule)]
@@ -89,23 +90,28 @@ public struct TargetSourcesBuilder {
8990
)
9091
self.fileSystem = fileSystem
9192

92-
self.observabilityScope = observabilityScope.makeChildScope(description: "TargetSourcesBuilder") {
93+
let childObservabilityScope = observabilityScope.makeChildScope(description: "TargetSourcesBuilder") {
9394
var metadata = ObservabilityMetadata.packageMetadata(identity: packageIdentity, kind: packageKind)
9495
metadata.moduleName = target.name
9596
return metadata
9697
}
98+
self.observabilityScope = childObservabilityScope
9799

98-
let declaredSources = target.sources?.compactMap { try? AbsolutePath(validating: $0, relativeTo: path) }
99-
if let declaredSources {
100-
// Diagnose duplicate entries.
101-
let duplicates = declaredSources.spm_findDuplicateElements()
102-
if !duplicates.isEmpty {
103-
for duplicate in duplicates {
104-
self.observabilityScope.emit(warning: "found duplicate sources declaration in the package manifest: \(duplicate.map{ $0.pathString }[0])")
100+
var declaredSources: OrderedCollections.OrderedSet<Basics.AbsolutePath>? = nil
101+
102+
if let targetSources = target.sources {
103+
for targetSource in targetSources {
104+
if let targetSourcePath = try? AbsolutePath(validating: targetSource, relativeTo: path) {
105+
declaredSources = declaredSources ?? OrderedCollections.OrderedSet<Basics.AbsolutePath>()
106+
if declaredSources?.updateOrAppend(targetSourcePath) != nil {
107+
childObservabilityScope.emit(warning: "found duplicate sources declaration in the package manifest: \(targetSourcePath.pathString)")
108+
109+
}
105110
}
106111
}
107112
}
108-
self.declaredSources = declaredSources?.spm_uniqueElements()
113+
114+
self.declaredSources = declaredSources
109115

110116
self.declaredResources = (try? target.resources.map {
111117
(path: try AbsolutePath(validating: $0.path, relativeTo: path), rule: $0.rule)
@@ -237,7 +243,7 @@ public struct TargetSourcesBuilder {
237243
toolsVersion: ToolsVersion,
238244
rules: [FileRuleDescription],
239245
declaredResources: [(path: Basics.AbsolutePath, rule: TargetDescription.Resource.Rule)],
240-
declaredSources: [Basics.AbsolutePath]?,
246+
declaredSources: OrderedCollections.OrderedSet<Basics.AbsolutePath>?,
241247
matchingResourceRuleHandler: (Basics.AbsolutePath) -> () = { _ in },
242248
observabilityScope: ObservabilityScope
243249
) -> FileRuleDescription.Rule {
@@ -257,11 +263,10 @@ public struct TargetSourcesBuilder {
257263

258264
// Match any sources explicitly declared in the manifest file.
259265
if let declaredSources {
260-
for sourcePath in declaredSources {
261-
if path.isDescendantOfOrEqual(to: sourcePath) {
262-
if matchedRule != .none {
263-
observabilityScope.emit(error: "duplicate rule found for file at '\(path)'")
264-
}
266+
if isDescendantOfOrEqualToAny(path, declaredSources) {
267+
if matchedRule != .none {
268+
observabilityScope.emit(error: "duplicate rule found for file at '\(path)'")
269+
}
265270

266271
// Check for header files as they're allowed to be mixed with sources.
267272
if let ext = path.extension,
@@ -276,10 +281,8 @@ public struct TargetSourcesBuilder {
276281
// The source file might have been declared twice so
277282
// exit on first match.
278283
// FIXME: We should emitting warnings for duplicate// declarations.
279-
break
280284
}
281285
}
282-
}
283286

284287
// We haven't found a rule using that's explicitly declared in the manifest
285288
// so try doing an automatic match.
@@ -302,6 +305,23 @@ public struct TargetSourcesBuilder {
302305

303306
return matchedRule
304307
}
308+
309+
private static func isDescendantOfOrEqualToAny(_ path: Basics.AbsolutePath, _ ancestorPaths: OrderedCollections.OrderedSet<Basics.AbsolutePath>) -> Bool {
310+
var currentPath = path
311+
while true {
312+
if ancestorPaths.contains(currentPath) {
313+
return true
314+
}
315+
316+
let parentPath = currentPath.parentDirectory
317+
if parentPath == currentPath {
318+
break
319+
}
320+
currentPath = parentPath
321+
}
322+
323+
return false
324+
}
305325

306326
/// Returns the `Resource` file associated with a file and a particular rule, if there is one.
307327
private static func resource(for path: Basics.AbsolutePath, with rule: FileRuleDescription.Rule, defaultLocalization: String?, targetName: String, targetPath: Basics.AbsolutePath, observabilityScope: ObservabilityScope) -> Resource? {

0 commit comments

Comments
 (0)