Skip to content

Commit 559f239

Browse files
committed
Improve performance of sourceFilesAndDirectories
`SourceFilesAndDirectoriesKey` contained all source files in the project and computing its hash value was pretty expensive. The key didn’t really provide any value here because the only way it changes is if the build targets change and if that’s the case, we already clear `cachedSourceFilesAndDirectories`, so we can just avoid the hash value computation.
1 parent 7a06eb0 commit 559f239

File tree

1 file changed

+31
-32
lines changed

1 file changed

+31
-32
lines changed

Sources/BuildSystemIntegration/BuildSystemManager.swift

Lines changed: 31 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -54,14 +54,21 @@ package struct SourceFileInfo: Sendable {
5454
/// from non-test targets or files that don't actually contain any tests.
5555
package var mayContainTests: Bool
5656

57+
/// Source files returned here fall into two categories:
58+
/// - Buildable source files are files that can be built by the build system and that make sense to background index
59+
/// - Non-buildable source files include eg. the SwiftPM package manifest or header files. We have sufficient
60+
/// compiler arguments for these files to provide semantic editor functionality but we can't build them.
61+
package var isBuildable: Bool
62+
5763
fileprivate func merging(_ other: SourceFileInfo?) -> SourceFileInfo {
5864
guard let other else {
5965
return self
6066
}
6167
return SourceFileInfo(
6268
targets: targets.union(other.targets),
6369
isPartOfRootProject: other.isPartOfRootProject || isPartOfRootProject,
64-
mayContainTests: other.mayContainTests || mayContainTests
70+
mayContainTests: other.mayContainTests || mayContainTests,
71+
isBuildable: other.isBuildable || isBuildable
6572
)
6673
}
6774
}
@@ -327,11 +334,9 @@ package actor BuildSystemManager: QueueBasedMessageHandler {
327334

328335
private var cachedTargetSources = RequestCache<BuildTargetSourcesRequest>()
329336

330-
/// The parameters with which `SourceFilesAndDirectories` can be cached in `cachedSourceFilesAndDirectories`.
331-
private struct SourceFilesAndDirectoriesKey: Hashable {
332-
let includeNonBuildableFiles: Bool
333-
let sourcesItems: [SourcesItem]
334-
}
337+
/// `SourceFilesAndDirectories` is a global property that only gets reset when the build targets change and thus
338+
/// has no real key.
339+
private struct SourceFilesAndDirectoriesKey: Hashable {}
335340

336341
private struct SourceFilesAndDirectories {
337342
/// The source files in the workspace, ie. all `SourceItem`s that have `kind == .file`.
@@ -678,7 +683,7 @@ package actor BuildSystemManager: QueueBasedMessageHandler {
678683
package func targets(for document: DocumentURI) async -> Set<BuildTargetIdentifier> {
679684
return await orLog("Getting targets for source file") {
680685
var result: Set<BuildTargetIdentifier> = []
681-
let filesAndDirectories = try await sourceFilesAndDirectories(includeNonBuildableFiles: true)
686+
let filesAndDirectories = try await sourceFilesAndDirectories()
682687
if let targets = filesAndDirectories.files[document]?.targets {
683688
result.formUnion(targets)
684689
}
@@ -1037,46 +1042,40 @@ package actor BuildSystemManager: QueueBasedMessageHandler {
10371042
///
10381043
/// - SeeAlso: Comment in `sourceFilesAndDirectories` for a definition of what `buildable` means.
10391044
package func sourceFiles(includeNonBuildableFiles: Bool) async throws -> [DocumentURI: SourceFileInfo] {
1040-
return try await sourceFilesAndDirectories(includeNonBuildableFiles: includeNonBuildableFiles).files
1045+
let files = try await sourceFilesAndDirectories().files
1046+
if includeNonBuildableFiles {
1047+
return files
1048+
} else {
1049+
return files.filter(\.value.isBuildable)
1050+
}
10411051
}
10421052

10431053
/// Get all files and directories that are known to the build system, ie. that are returned by a `buildTarget/sources`
10441054
/// request for any target in the project.
10451055
///
1046-
/// Source files returned here fall into two categories:
1047-
/// - Buildable source files are files that can be built by the build system and that make sense to background index
1048-
/// - Non-buildable source files include eg. the SwiftPM package manifest or header files. We have sufficient
1049-
/// compiler arguments for these files to provide semantic editor functionality but we can't build them.
1050-
///
1051-
/// `includeNonBuildableFiles` determines whether non-buildable files should be included.
1052-
private func sourceFilesAndDirectories(includeNonBuildableFiles: Bool) async throws -> SourceFilesAndDirectories {
1053-
let targets = try await self.buildTargets()
1054-
let sourcesItems = try await self.sourceFiles(in: Set(targets.keys))
1055-
1056-
let key = SourceFilesAndDirectoriesKey(
1057-
includeNonBuildableFiles: includeNonBuildableFiles,
1058-
sourcesItems: sourcesItems
1059-
)
1056+
/// - Important: This method returns both buildable and non-buildable source files. Callers need to check
1057+
/// `SourceFileInfo.isBuildable` if they are only interested in buildable source files.
1058+
private func sourceFilesAndDirectories() async throws -> SourceFilesAndDirectories {
1059+
return try await cachedSourceFilesAndDirectories.get(
1060+
SourceFilesAndDirectoriesKey(),
1061+
isolation: self
1062+
) { key in
1063+
let targets = try await self.buildTargets()
1064+
let sourcesItems = try await self.sourceFiles(in: Set(targets.keys))
10601065

1061-
return try await cachedSourceFilesAndDirectories.get(key, isolation: self) { key in
10621066
var files: [DocumentURI: SourceFileInfo] = [:]
10631067
var directories: [DocumentURI: (pathComponents: [String]?, info: SourceFileInfo)] = [:]
1064-
for sourcesItem in key.sourcesItems {
1068+
for sourcesItem in sourcesItems {
10651069
let target = targets[sourcesItem.target]?.target
10661070
let isPartOfRootProject = !(target?.tags.contains(.dependency) ?? false)
10671071
let mayContainTests = target?.tags.contains(.test) ?? true
1068-
if !key.includeNonBuildableFiles && (target?.tags.contains(.notBuildable) ?? false) {
1069-
continue
1070-
}
1071-
10721072
for sourceItem in sourcesItem.sources {
1073-
if !key.includeNonBuildableFiles && sourceItem.sourceKitData?.isHeader ?? false {
1074-
continue
1075-
}
10761073
let info = SourceFileInfo(
10771074
targets: [sourcesItem.target],
10781075
isPartOfRootProject: isPartOfRootProject,
1079-
mayContainTests: mayContainTests
1076+
mayContainTests: mayContainTests,
1077+
isBuildable: !(target?.tags.contains(.notBuildable) ?? false)
1078+
&& !(sourceItem.sourceKitData?.isHeader ?? false)
10801079
)
10811080
switch sourceItem.kind {
10821081
case .file:

0 commit comments

Comments
 (0)