From 559f23957eb60f25c519b1b8d08ee5a1ff9b4ad5 Mon Sep 17 00:00:00 2001 From: Alex Hoppen Date: Thu, 5 Dec 2024 19:58:31 -0800 Subject: [PATCH] Improve performance of `sourceFilesAndDirectories` MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `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. --- .../BuildSystemManager.swift | 63 +++++++++---------- 1 file changed, 31 insertions(+), 32 deletions(-) diff --git a/Sources/BuildSystemIntegration/BuildSystemManager.swift b/Sources/BuildSystemIntegration/BuildSystemManager.swift index 023353b3c..8dff5ea11 100644 --- a/Sources/BuildSystemIntegration/BuildSystemManager.swift +++ b/Sources/BuildSystemIntegration/BuildSystemManager.swift @@ -54,6 +54,12 @@ package struct SourceFileInfo: Sendable { /// from non-test targets or files that don't actually contain any tests. package var mayContainTests: Bool + /// Source files returned here fall into two categories: + /// - Buildable source files are files that can be built by the build system and that make sense to background index + /// - Non-buildable source files include eg. the SwiftPM package manifest or header files. We have sufficient + /// compiler arguments for these files to provide semantic editor functionality but we can't build them. + package var isBuildable: Bool + fileprivate func merging(_ other: SourceFileInfo?) -> SourceFileInfo { guard let other else { return self @@ -61,7 +67,8 @@ package struct SourceFileInfo: Sendable { return SourceFileInfo( targets: targets.union(other.targets), isPartOfRootProject: other.isPartOfRootProject || isPartOfRootProject, - mayContainTests: other.mayContainTests || mayContainTests + mayContainTests: other.mayContainTests || mayContainTests, + isBuildable: other.isBuildable || isBuildable ) } } @@ -327,11 +334,9 @@ package actor BuildSystemManager: QueueBasedMessageHandler { private var cachedTargetSources = RequestCache() - /// The parameters with which `SourceFilesAndDirectories` can be cached in `cachedSourceFilesAndDirectories`. - private struct SourceFilesAndDirectoriesKey: Hashable { - let includeNonBuildableFiles: Bool - let sourcesItems: [SourcesItem] - } + /// `SourceFilesAndDirectories` is a global property that only gets reset when the build targets change and thus + /// has no real key. + private struct SourceFilesAndDirectoriesKey: Hashable {} private struct SourceFilesAndDirectories { /// The source files in the workspace, ie. all `SourceItem`s that have `kind == .file`. @@ -678,7 +683,7 @@ package actor BuildSystemManager: QueueBasedMessageHandler { package func targets(for document: DocumentURI) async -> Set { return await orLog("Getting targets for source file") { var result: Set = [] - let filesAndDirectories = try await sourceFilesAndDirectories(includeNonBuildableFiles: true) + let filesAndDirectories = try await sourceFilesAndDirectories() if let targets = filesAndDirectories.files[document]?.targets { result.formUnion(targets) } @@ -1037,46 +1042,40 @@ package actor BuildSystemManager: QueueBasedMessageHandler { /// /// - SeeAlso: Comment in `sourceFilesAndDirectories` for a definition of what `buildable` means. package func sourceFiles(includeNonBuildableFiles: Bool) async throws -> [DocumentURI: SourceFileInfo] { - return try await sourceFilesAndDirectories(includeNonBuildableFiles: includeNonBuildableFiles).files + let files = try await sourceFilesAndDirectories().files + if includeNonBuildableFiles { + return files + } else { + return files.filter(\.value.isBuildable) + } } /// Get all files and directories that are known to the build system, ie. that are returned by a `buildTarget/sources` /// request for any target in the project. /// - /// Source files returned here fall into two categories: - /// - Buildable source files are files that can be built by the build system and that make sense to background index - /// - Non-buildable source files include eg. the SwiftPM package manifest or header files. We have sufficient - /// compiler arguments for these files to provide semantic editor functionality but we can't build them. - /// - /// `includeNonBuildableFiles` determines whether non-buildable files should be included. - private func sourceFilesAndDirectories(includeNonBuildableFiles: Bool) async throws -> SourceFilesAndDirectories { - let targets = try await self.buildTargets() - let sourcesItems = try await self.sourceFiles(in: Set(targets.keys)) - - let key = SourceFilesAndDirectoriesKey( - includeNonBuildableFiles: includeNonBuildableFiles, - sourcesItems: sourcesItems - ) + /// - Important: This method returns both buildable and non-buildable source files. Callers need to check + /// `SourceFileInfo.isBuildable` if they are only interested in buildable source files. + private func sourceFilesAndDirectories() async throws -> SourceFilesAndDirectories { + return try await cachedSourceFilesAndDirectories.get( + SourceFilesAndDirectoriesKey(), + isolation: self + ) { key in + let targets = try await self.buildTargets() + let sourcesItems = try await self.sourceFiles(in: Set(targets.keys)) - return try await cachedSourceFilesAndDirectories.get(key, isolation: self) { key in var files: [DocumentURI: SourceFileInfo] = [:] var directories: [DocumentURI: (pathComponents: [String]?, info: SourceFileInfo)] = [:] - for sourcesItem in key.sourcesItems { + for sourcesItem in sourcesItems { let target = targets[sourcesItem.target]?.target let isPartOfRootProject = !(target?.tags.contains(.dependency) ?? false) let mayContainTests = target?.tags.contains(.test) ?? true - if !key.includeNonBuildableFiles && (target?.tags.contains(.notBuildable) ?? false) { - continue - } - for sourceItem in sourcesItem.sources { - if !key.includeNonBuildableFiles && sourceItem.sourceKitData?.isHeader ?? false { - continue - } let info = SourceFileInfo( targets: [sourcesItem.target], isPartOfRootProject: isPartOfRootProject, - mayContainTests: mayContainTests + mayContainTests: mayContainTests, + isBuildable: !(target?.tags.contains(.notBuildable) ?? false) + && !(sourceItem.sourceKitData?.isHeader ?? false) ) switch sourceItem.kind { case .file: