diff --git a/Contributor Documentation/BSP Extensions.md b/Contributor Documentation/BSP Extensions.md index c8497f039..88c234fbd 100644 --- a/Contributor Documentation/BSP Extensions.md +++ b/Contributor Documentation/BSP Extensions.md @@ -130,8 +130,12 @@ export interface SourceKitSourceItemData { kind?: "source" | "header" | "doccCatalog"; /** - * The output path that is during indexing for this file, ie. the `-index-unit-output-path`, if it is specified - * in the compiler arguments or the file that is passed as `-o`, if `-index-unit-output-path` is not specified. + * The output path that is a string that uniquely identifies the index output of this file in this target. If an index + * store should be re-used between build and background indexing, it must match the `-o` path or + * `-index-unit-output-path` used during the build. + * + * The index unit output path historically matched the path used for `-o` during compilation but has since evolved to + * be an opaque string. In particular, it does not have to match to any file on disk. * * This allows SourceKit-LSP to remove index entries for source files that are removed from a target but remain * present on disk and to index a file that is part of multiple targets in the context of each target. diff --git a/Sources/BuildServerIntegration/BuildSettingsLogger.swift b/Sources/BuildServerIntegration/BuildSettingsLogger.swift index 856a5d4ed..56f33d50b 100644 --- a/Sources/BuildServerIntegration/BuildSettingsLogger.swift +++ b/Sources/BuildServerIntegration/BuildSettingsLogger.swift @@ -12,6 +12,7 @@ package import LanguageServerProtocol package import SKLogging +import SwiftExtensions // MARK: - Build settings logger @@ -27,11 +28,31 @@ package actor BuildSettingsLogger { Self.log(level: level, settings: settings, for: uri) } - /// Log the given build settings. + /// Log the given build settings for a single file /// /// In contrast to the instance method `log`, this will always log the build settings. The instance method only logs /// the build settings if they have changed. package static func log(level: LogLevel = .default, settings: FileBuildSettings, for uri: DocumentURI) { + log(level: level, settings: settings, for: [uri]) + } + + /// Log the given build settings for a list of source files that all share the same build settings. + /// + /// In contrast to the instance method `log`, this will always log the build settings. The instance method only logs + /// the build settings if they have changed. + package static func log(level: LogLevel = .default, settings: FileBuildSettings, for uris: [DocumentURI]) { + let header: String + if let uri = uris.only { + header = "Build settings for \(uri.forLogging)" + } else if let firstUri = uris.first { + header = "Build settings for \(firstUri.forLogging) and \(firstUri) and \(uris.count - 1) others" + } else { + header = "Build settings for empty list" + } + log(level: level, settings: settings, header: header) + } + + private static func log(level: LogLevel = .default, settings: FileBuildSettings, header: String) { let log = """ Compiler Arguments: \(settings.compilerArguments.joined(separator: "\n")) @@ -47,7 +68,7 @@ package actor BuildSettingsLogger { logger.log( level: level, """ - Build settings for \(uri.forLogging) (\(index + 1)/\(chunks.count)) + \(header) (\(index + 1)/\(chunks.count)) \(chunk) """ ) diff --git a/Sources/BuildServerIntegration/FileBuildSettings.swift b/Sources/BuildServerIntegration/FileBuildSettings.swift index 4af0dfbca..fab6ef5db 100644 --- a/Sources/BuildServerIntegration/FileBuildSettings.swift +++ b/Sources/BuildServerIntegration/FileBuildSettings.swift @@ -18,8 +18,7 @@ import LanguageServerProtocolExtensions /// /// Encapsulates all the settings needed to compile a single file, including the compiler arguments /// and working directory. `FileBuildSettings`` are typically the result of a build server query. -package struct FileBuildSettings: Equatable, Sendable { - +package struct FileBuildSettings: Hashable, Sendable { /// The compiler arguments to use for this file. package var compilerArguments: [String] diff --git a/Sources/BuildServerIntegration/SwiftPMBuildServer.swift b/Sources/BuildServerIntegration/SwiftPMBuildServer.swift index ed1291820..e357d9b25 100644 --- a/Sources/BuildServerIntegration/SwiftPMBuildServer.swift +++ b/Sources/BuildServerIntegration/SwiftPMBuildServer.swift @@ -514,6 +514,11 @@ package actor SwiftPMBuildServer: BuiltInBuildServer { throw NonFileURIError(uri: file) } + #if compiler(>=6.4) + #warning( + "Once we can guarantee that the toolchain can index multiple Swift files in a single invocation, we no longer need to set -index-unit-output-path since it's always set using an -output-file-map" + ) + #endif var compilerArguments = try buildTarget.compileArguments(for: fileURL) if buildTarget.compiler == .swift { compilerArguments += [ diff --git a/Sources/SKTestSupport/SkipUnless.swift b/Sources/SKTestSupport/SkipUnless.swift index c101f8816..5b9b4dd44 100644 --- a/Sources/SKTestSupport/SkipUnless.swift +++ b/Sources/SKTestSupport/SkipUnless.swift @@ -472,6 +472,15 @@ package actor SkipUnless { } } + package static func canIndexMultipleSwiftFilesInSingleInvocation( + file: StaticString = #filePath, + line: UInt = #line + ) async throws { + return try await shared.skipUnlessSupportedByToolchain(swiftVersion: SwiftVersion(6, 3), file: file, line: line) { + return await ToolchainRegistry.forTesting.default?.canIndexMultipleSwiftFilesInSingleInvocation ?? false + } + } + private static func getSourceKitD() async throws -> SourceKitD { guard let sourcekitdPath = await ToolchainRegistry.forTesting.default?.sourcekitd else { throw GenericError("Could not find SourceKitD") diff --git a/Sources/SemanticIndex/SemanticIndexManager.swift b/Sources/SemanticIndex/SemanticIndexManager.swift index be73209ee..b54b9d1c4 100644 --- a/Sources/SemanticIndex/SemanticIndexManager.swift +++ b/Sources/SemanticIndex/SemanticIndexManager.swift @@ -928,7 +928,13 @@ package final actor SemanticIndexManager { // (https://github.com/swiftlang/sourcekit-lsp/issues/1262) for targetsBatch in sortedTargets.partition(intoBatchesOfSize: 1) { let preparationTaskID = UUID() - let filesToIndex = targetsBatch.flatMap({ filesByTarget[$0]! }) + let filesToIndex = targetsBatch.flatMap { (target) -> [FileIndexInfo] in + guard let files = filesByTarget[target] else { + logger.fault("Unexpectedly found no files for target in target batch") + return [] + } + return files + } // First schedule preparation of the targets. We schedule the preparation outside of `indexTask` so that we // deterministically prepare targets in the topological order for indexing. If we triggered preparation inside the @@ -957,36 +963,36 @@ package final actor SemanticIndexManager { // And after preparation is done, index the files in the targets. await withTaskGroup(of: Void.self) { taskGroup in - for target in targetsBatch { - var filesByLanguage: [Language: [FileIndexInfo]] = [:] - for fileInfo in filesByTarget[target]! { - filesByLanguage[fileInfo.language, default: []].append(fileInfo) + let fileInfos = targetsBatch.flatMap { (target) -> [FileIndexInfo] in + guard let files = filesByTarget[target] else { + logger.fault("Unexpectedly found no files for target in target batch") + return [] } - for (language, fileInfos) in filesByLanguage { - // TODO: Once swiftc supports indexing of multiple files in a single invocation, increase the batch size to - // allow it to share AST builds between multiple files within a target. - // (https://github.com/swiftlang/sourcekit-lsp/issues/1268) - for fileBatch in fileInfos.partition(intoBatchesOfSize: 1) { - taskGroup.addTask { - let fileAndOutputPaths: [FileAndOutputPath] = fileBatch.compactMap { - guard $0.target == target else { - logger.fault( - "FileIndexInfo refers to different target than should be indexed \($0.target.forLogging) vs \(target.forLogging)" - ) - return nil - } - return FileAndOutputPath(file: $0.file, outputPath: $0.outputPath) - } - await self.updateIndexStore( - for: fileAndOutputPaths, - target: target, - language: language, - indexFilesWithUpToDateUnit: indexFilesWithUpToDateUnit, - preparationTaskID: preparationTaskID, - priority: priority + return files + } + let batches = await UpdateIndexStoreTaskDescription.batches( + toIndex: fileInfos, + buildServerManager: buildServerManager + ) + for (target, language, fileBatch) in batches { + taskGroup.addTask { + let fileAndOutputPaths: [FileAndOutputPath] = fileBatch.compactMap { + guard $0.target == target else { + logger.fault( + "FileIndexInfo refers to different target than should be indexed: \($0.target.forLogging) vs \(target.forLogging)" ) + return nil } + return FileAndOutputPath(file: $0.file, outputPath: $0.outputPath) } + await self.updateIndexStore( + for: fileAndOutputPaths, + target: target, + language: language, + indexFilesWithUpToDateUnit: indexFilesWithUpToDateUnit, + preparationTaskID: preparationTaskID, + priority: priority + ) } } await taskGroup.waitForAll() diff --git a/Sources/SemanticIndex/UpdateIndexStoreTaskDescription.swift b/Sources/SemanticIndex/UpdateIndexStoreTaskDescription.swift index ba289aa13..61dce87dd 100644 --- a/Sources/SemanticIndex/UpdateIndexStoreTaskDescription.swift +++ b/Sources/SemanticIndex/UpdateIndexStoreTaskDescription.swift @@ -88,6 +88,26 @@ package struct FileAndOutputPath: Sendable, Hashable { fileprivate var sourceFile: DocumentURI { file.sourceFile } } +/// A single file or a list of files that should be indexed in a single compiler invocation. +private enum UpdateIndexStorePartition { + case multipleFiles(filesAndOutputPaths: [(file: FileToIndex, outputPath: String)], buildSettings: FileBuildSettings) + case singleFile(file: FileAndOutputPath, buildSettings: FileBuildSettings) + + var buildSettings: FileBuildSettings { + switch self { + case .multipleFiles(_, let buildSettings): return buildSettings + case .singleFile(_, let buildSettings): return buildSettings + } + } + + var files: [FileToIndex] { + switch self { + case .multipleFiles(let filesAndOutputPaths, _): return filesAndOutputPaths.map(\.file) + case .singleFile(let file, _): return [file.file] + } + } +} + /// Describes a task to index a set of source files. /// /// This task description can be scheduled in a `TaskScheduler`. @@ -222,15 +242,7 @@ package struct UpdateIndexStoreTaskDescription: IndexTaskDescription { // Disjoint sets of files can be indexed concurrently. return nil } - if self.filesToIndex.count < other.filesToIndex.count { - // If there is an index operation with more files already running, suspend it. - // The most common use case for this is if we schedule an entire target to be indexed in the background and then - // need a single file indexed for use interaction. We should suspend the target-wide indexing and just index - // the current file to get index data for it ASAP. - return .cancelAndRescheduleDependency(other) - } else { - return .waitAndElevatePriorityOfDependency(other) - } + return .waitAndElevatePriorityOfDependency(other) } } @@ -264,6 +276,16 @@ package struct UpdateIndexStoreTaskDescription: IndexTaskDescription { ) } + // Compute the partitions within which we can perform multi-file indexing. For this we gather all the files that may + // only be indexed by themselves in `partitions` an collect all other files in `fileInfosByBuildSettings` so that we + // can create a partition for all files that share the same build settings. + // In most cases, we will only end up with a single partition for each `UpdateIndexStoreTaskDescription` since + // `UpdateIndexStoreTaskDescription.batches(toIndex:)` tries to batch the files in a way such that all files within + // the batch can be indexed by a single compiler invocation. However, we might discover that two Swift files within + // the same target have different build settings in the build server. In that case, the best thing we can do is + // trigger two compiler invocations. + var swiftFileInfosByBuildSettings: [FileBuildSettings: [(file: FileToIndex, outputPath: String)]] = [:] + var partitions: [UpdateIndexStorePartition] = [] for fileInfo in fileInfos { let buildSettings = await buildServerManager.buildSettings( for: fileInfo.mainFile, @@ -271,15 +293,16 @@ package struct UpdateIndexStoreTaskDescription: IndexTaskDescription { language: language, fallbackAfterTimeout: false ) - guard let buildSettings else { + guard var buildSettings else { logger.error("Not indexing \(fileInfo.file.forLogging) because it has no compiler arguments") continue } if buildSettings.isFallback { - // Fallback build settings don’t even have an indexstore path set, so they can't generate index data that we would - // pick up. Also, indexing with fallback args has some other problems:∂ - // - If it did generate a unit file, we would consider the file’s index up-to-date even if the compiler arguments - // change, which means that we wouldn't get any up-to-date-index even when we have build settings for the file. + // Fallback build settings don’t even have an indexstore path set, so they can't generate index data that we + // would pick up. Also, indexing with fallback args has some other problems: + // - If it did generate a unit file, we would consider the file’s index up-to-date even if the compiler + // arguments change, which means that we wouldn't get any up-to-date-index even when we have build settings + // for the file. // - It's unlikely that the index from a single file with fallback arguments will be very useful as it can't tie // into the rest of the project. // So, don't index the file. @@ -287,49 +310,109 @@ package struct UpdateIndexStoreTaskDescription: IndexTaskDescription { continue } - guard let toolchain = await buildServerManager.toolchain(for: target, language: buildSettings.language) else { + guard buildSettings.language == .swift else { + // We only support multi-file indexing for Swift files. Do not try to batch or normalize + // `-index-unit-output-path` for clang files. + partitions.append(.singleFile(file: fileInfo, buildSettings: buildSettings)) + continue + } + + guard + await buildServerManager.toolchain(for: target, language: language)? + .canIndexMultipleSwiftFilesInSingleInvocation ?? false + else { + partitions.append(.singleFile(file: fileInfo, buildSettings: buildSettings)) + continue + } + + // If the build settings contain `-index-unit-output-path`, remove it. We add the index unit output path back in + // using an `-output-file-map`. Removing it from the build settings allows us to index multiple Swift files in a + // single compiler invocation if they share all build settings and only differ in their `-index-unit-output-path`. + let indexUnitOutputPathFromSettings = removeIndexUnitOutputPath( + from: &buildSettings, + for: fileInfo.mainFile + ) + + switch (fileInfo.outputPath, indexUnitOutputPathFromSettings) { + case (.notSupported, nil): + partitions.append(.singleFile(file: fileInfo, buildSettings: buildSettings)) + case (.notSupported, let indexUnitOutputPathFromSettings?): + swiftFileInfosByBuildSettings[buildSettings, default: []].append( + (fileInfo.file, indexUnitOutputPathFromSettings) + ) + case (.path(let indexUnitOutputPath), nil): + swiftFileInfosByBuildSettings[buildSettings, default: []].append((fileInfo.file, indexUnitOutputPath)) + case (.path(let indexUnitOutputPath), let indexUnitOutputPathFromSettings?): + if indexUnitOutputPathFromSettings != indexUnitOutputPath { + logger.error( + "Output path reported by BSP server does not match -index-unit-output path in compiler arguments: \(indexUnitOutputPathFromSettings) vs \(indexUnitOutputPath)" + ) + } + swiftFileInfosByBuildSettings[buildSettings, default: []].append((fileInfo.file, indexUnitOutputPath)) + } + } + for (buildSettings, fileInfos) in swiftFileInfosByBuildSettings { + partitions.append(.multipleFiles(filesAndOutputPaths: fileInfos, buildSettings: buildSettings)) + } + + for partition in partitions { + guard let toolchain = await buildServerManager.toolchain(for: target, language: partition.buildSettings.language) + else { logger.fault( - "Unable to determine toolchain to index \(buildSettings.language.description, privacy: .public) files in \(target.forLogging)" + "Unable to determine toolchain to index \(partition.buildSettings.language.description, privacy: .public) files in \(target.forLogging)" ) continue } let startDate = Date() - switch buildSettings.language.semanticKind { + switch partition.buildSettings.language.semanticKind { case .swift: do { try await updateIndexStore( - forSwiftFile: fileInfo.mainFile, - buildSettings: buildSettings, + forSwiftFilesInPartition: partition, toolchain: toolchain ) } catch { - logger.error("Updating index store for \(fileInfo.mainFile) failed: \(error.forLogging)") - BuildSettingsLogger.log(settings: buildSettings, for: fileInfo.mainFile) + logger.error( + """ + Updating index store failed: \(error.forLogging). + Files: \(partition.files) + """ + ) + BuildSettingsLogger.log(settings: partition.buildSettings, for: partition.files.map(\.mainFile)) } case .clang: - do { - try await updateIndexStore( - forClangFile: fileInfo.mainFile, - buildSettings: buildSettings, - toolchain: toolchain - ) - } catch { - logger.error("Updating index store for \(fileInfo.mainFile.forLogging) failed: \(error.forLogging)") - BuildSettingsLogger.log(settings: buildSettings, for: fileInfo.mainFile) + for fileInfo in partition.files { + do { + try await updateIndexStore( + forClangFile: fileInfo.mainFile, + buildSettings: partition.buildSettings, + toolchain: toolchain + ) + } catch { + logger.error("Updating index store for \(fileInfo.mainFile.forLogging) failed: \(error.forLogging)") + BuildSettingsLogger.log(settings: partition.buildSettings, for: fileInfo.mainFile) + } } case nil: logger.error( - "Not updating index store for \(fileInfo.mainFile.forLogging) because it is a language that is not supported by background indexing" + """ + Not updating index store because \(partition.buildSettings.language.rawValue, privacy: .public) is not \ + supported by background indexing. + Files: \(partition.files) + """ ) } - await indexStoreUpToDateTracker.markUpToDate([(fileInfo.sourceFile, target)], updateOperationStartDate: startDate) + await indexStoreUpToDateTracker.markUpToDate( + partition.files.map { ($0.sourceFile, target) }, + updateOperationStartDate: startDate + ) } } /// If `args` does not contain an `-index-store-path` argument, add it, pointing to the build server's index store /// path. If an `-index-store-path` already exists, validate that it matches the build server's index store path and /// replace it by the build server's index store path if they don't match. - private func addOrReplaceIndexStorePath(in args: [String], for uri: DocumentURI) async throws -> [String] { + private func addOrReplaceIndexStorePath(in args: [String], for uris: [DocumentURI]) async throws -> [String] { var args = args guard let buildServerIndexStorePath = await self.buildServerManager.initializationData?.indexStorePath else { struct NoIndexStorePathError: Error {} @@ -340,9 +423,9 @@ package struct UpdateIndexStoreTaskDescription: IndexTaskDescription { if indexStorePath != buildServerIndexStorePath { logger.error( """ - Compiler arguments for \(uri) specify index store path \(indexStorePath) but build server specified an \ + Compiler arguments specify index store path \(indexStorePath) but build server specified an \ incompatible index store path \(buildServerIndexStorePath). Overriding with the path specified by the build \ - system. + system. For \(uris) """ ) args[indexStorePathIndex + 1] = buildServerIndexStorePath @@ -353,33 +436,104 @@ package struct UpdateIndexStoreTaskDescription: IndexTaskDescription { return args } + /// If the build settings contain an `-index-unit-output-path` argument, remove it and return the index unit output + /// path. Otherwise don't modify `buildSettings` and return `nil`. + private func removeIndexUnitOutputPath(from buildSettings: inout FileBuildSettings, for uri: DocumentURI) -> String? { + guard let indexUnitOutputPathIndex = buildSettings.compilerArguments.lastIndex(of: "-index-unit-output-path"), + indexUnitOutputPathIndex + 1 < buildSettings.compilerArguments.count + else { + return nil + } + let indexUnitOutputPath = buildSettings.compilerArguments[indexUnitOutputPathIndex + 1] + buildSettings.compilerArguments.removeSubrange(indexUnitOutputPathIndex...(indexUnitOutputPathIndex + 1)) + if buildSettings.compilerArguments.contains("-index-unit-output-path") { + logger.error("Build settings contained two -index-unit-output-path arguments") + BuildSettingsLogger.log(settings: buildSettings, for: uri) + } + return indexUnitOutputPath + } + private func updateIndexStore( - forSwiftFile uri: DocumentURI, - buildSettings: FileBuildSettings, + forSwiftFilesInPartition partition: UpdateIndexStorePartition, toolchain: Toolchain ) async throws { guard let swiftc = toolchain.swiftc else { logger.error( - "Not updating index store for \(uri.forLogging) because toolchain \(toolchain.identifier) does not contain a Swift compiler" + "Not updating index store for \(partition.files) because toolchain \(toolchain.identifier) does not contain a Swift compiler" ) return } var args = - try [swiftc.filePath] + buildSettings.compilerArguments + [ + try [swiftc.filePath] + partition.buildSettings.compilerArguments + [ "-index-file", - "-index-file-path", uri.pseudoPath, // batch mode is not compatible with -index-file "-disable-batch-mode", ] - args = try await addOrReplaceIndexStorePath(in: args, for: uri) + args = try await addOrReplaceIndexStorePath(in: args, for: partition.files.map(\.mainFile)) + + switch partition { + case .multipleFiles(let filesAndOutputPaths, let buildSettings): + if await !toolchain.canIndexMultipleSwiftFilesInSingleInvocation { + // We should never get here because we shouldn't create `multipleFiles` batches if the toolchain doesn't support + // indexing multiple files in a single compiler invocation. + logger.fault("Cannot index multiple files in a single compiler invocation.") + } - try await runIndexingProcess( - indexFile: uri, - buildSettings: buildSettings, - processArguments: args, - workingDirectory: buildSettings.workingDirectory.map(AbsolutePath.init(validating:)) - ) + struct OutputFileMapEntry: Encodable { + let indexUnitOutputPath: String + + private enum CodingKeys: String, CodingKey { + case indexUnitOutputPath = "index-unit-output-path" + } + } + var outputFileMap: [String: OutputFileMapEntry] = [:] + for (fileInfo, outputPath) in filesAndOutputPaths { + guard let filePath = try? fileInfo.mainFile.fileURL?.filePath else { + logger.error("Failed to determine file path of file to index \(fileInfo.mainFile.forLogging)") + continue + } + outputFileMap[filePath] = .init(indexUnitOutputPath: outputPath) + } + let tempFileUri = FileManager.default.temporaryDirectory + .appending(component: "sourcekit-lsp-output-file-map-\(UUID().uuidString).json") + try JSONEncoder().encode(outputFileMap).write(to: tempFileUri) + defer { + orLog("Delete output file map") { + try FileManager.default.removeItem(at: tempFileUri) + } + } + + let indexFiles = filesAndOutputPaths.map(\.file.mainFile) + // If the compiler arguments already contain an `-output-file-map` argument, we override it by adding a second one + // This is fine because we shouldn't be generating any outputs except for the index. + args += ["-output-file-map", try tempFileUri.filePath] + args += indexFiles.flatMap { (indexFile) -> [String] in + guard let filePath = try? indexFile.fileURL?.filePath else { + logger.error("Failed to determine file path of file to index \(indexFile.forLogging)") + return [] + } + return ["-index-file-path", filePath] + } + + try await runIndexingProcess( + indexFiles: indexFiles, + buildSettings: buildSettings, + processArguments: args, + workingDirectory: buildSettings.workingDirectory.map(AbsolutePath.init(validating:)) + ) + case .singleFile(let file, let buildSettings): + // We only end up in this case if the file's build settings didn't contain `-index-unit-output-path` and the build + // server is not a `outputPathsProvider`. + args += ["-index-file-path", file.mainFile.pseudoPath] + + try await runIndexingProcess( + indexFiles: [file.mainFile], + buildSettings: buildSettings, + processArguments: args, + workingDirectory: buildSettings.workingDirectory.map(AbsolutePath.init(validating:)) + ) + } } private func updateIndexStore( @@ -395,10 +549,10 @@ package struct UpdateIndexStoreTaskDescription: IndexTaskDescription { } var args = [try clang.filePath] + buildSettings.compilerArguments - args = try await addOrReplaceIndexStorePath(in: args, for: uri) + args = try await addOrReplaceIndexStorePath(in: args, for: [uri]) try await runIndexingProcess( - indexFile: uri, + indexFiles: [uri], buildSettings: buildSettings, processArguments: args, workingDirectory: buildSettings.workingDirectory.map(AbsolutePath.init(validating:)) @@ -406,7 +560,7 @@ package struct UpdateIndexStoreTaskDescription: IndexTaskDescription { } private func runIndexingProcess( - indexFile: DocumentURI, + indexFiles: [DocumentURI], buildSettings: FileBuildSettings, processArguments: [String], workingDirectory: AbsolutePath? @@ -420,7 +574,7 @@ package struct UpdateIndexStoreTaskDescription: IndexTaskDescription { let state = signposter.beginInterval( "Indexing", id: signpostID, - "Indexing \(indexFile.fileURL?.lastPathComponent ?? indexFile.pseudoPath)" + "Indexing \(indexFiles.map { $0.fileURL?.lastPathComponent ?? $0.pseudoPath })" ) defer { signposter.endInterval("Indexing", state) @@ -429,7 +583,9 @@ package struct UpdateIndexStoreTaskDescription: IndexTaskDescription { logMessageToIndexLog( processArguments.joined(separator: " "), .info, - .begin(StructuredLogBegin(title: "Indexing \(indexFile.pseudoPath)", taskID: taskId)) + .begin( + StructuredLogBegin(title: "Indexing \(indexFiles.map(\.pseudoPath).joined(separator: ", "))", taskID: taskId) + ) ) let stdoutHandler = PipeAsStringHandler { @@ -475,28 +631,88 @@ package struct UpdateIndexStoreTaskDescription: IndexTaskDescription { // Indexing will frequently fail if the source code is in an invalid state. Thus, log the failure at a low level. logger.debug( """ - Updating index store for \(indexFile.forLogging) terminated with non-zero exit code \(code) + Updating index store for terminated with non-zero exit code \(code) for \(indexFiles) Stderr: \(stderr) Stdout: \(stdout) """ ) - BuildSettingsLogger.log(level: .debug, settings: buildSettings, for: indexFile) + BuildSettingsLogger.log(level: .debug, settings: buildSettings, for: indexFiles) case .signalled(let signal): if !Task.isCancelled { // The indexing job finished with a signal. Could be because the compiler crashed. // Ignore signal exit codes if this task has been cancelled because the compiler exits with SIGINT if it gets // interrupted. - logger.error("Updating index store for \(indexFile.forLogging) signaled \(signal)") - BuildSettingsLogger.log(level: .error, settings: buildSettings, for: indexFile) + logger.error("Updating index store for signaled \(signal) for \(indexFiles)") + BuildSettingsLogger.log(level: .error, settings: buildSettings, for: indexFiles) } case .abnormal(let exception): if !Task.isCancelled { - logger.error("Updating index store for \(indexFile.forLogging) exited abnormally \(exception)") - BuildSettingsLogger.log(level: .error, settings: buildSettings, for: indexFile) + logger.error("Updating index store exited abnormally \(exception) for \(indexFiles)") + BuildSettingsLogger.log(level: .error, settings: buildSettings, for: indexFiles) + } + } + } + + /// Partition the given `FileIndexInfos` into batches so that a single `UpdateIndexStoreTaskDescription` should be + /// created for every one of these batches, taking advantage of multi-file indexing when it is supported. + static func batches( + toIndex fileIndexInfos: [FileIndexInfo], + buildServerManager: BuildServerManager + ) async -> [(target: BuildTargetIdentifier, language: Language, files: [FileIndexInfo])] { + struct TargetAndLanguage: Hashable, Comparable { + let target: BuildTargetIdentifier + let language: Language + + static func < (lhs: TargetAndLanguage, rhs: TargetAndLanguage) -> Bool { + if lhs.target.uri.stringValue < rhs.target.uri.stringValue { + return true + } else if lhs.target.uri.stringValue > rhs.target.uri.stringValue { + return false + } + if lhs.language.rawValue < rhs.language.rawValue { + return true + } else if lhs.language.rawValue > rhs.language.rawValue { + return false + } + return false } } + + var partitions: [(target: BuildTargetIdentifier, language: Language, files: [FileIndexInfo])] = [] + var fileIndexInfosToBatch: [TargetAndLanguage: [FileIndexInfo]] = [:] + for fileIndexInfo in fileIndexInfos { + guard fileIndexInfo.language == .swift, + await buildServerManager.toolchain(for: fileIndexInfo.target, language: fileIndexInfo.language)? + .canIndexMultipleSwiftFilesInSingleInvocation ?? false + else { + // Only Swift supports indexing multiple files in a single compiler invocation, so don't batch files of other + // languages. + partitions.append((fileIndexInfo.target, fileIndexInfo.language, [fileIndexInfo])) + continue + } + // Even for Swift files, we can only index files in a single compiler invocation if they have the same build + // settings (modulo some normalization in `updateIndexStore(forFiles:)`). We can't know that all the files do + // indeed have the same compiler arguments but loading build settings during scheduling from the build server + // is not desirable because it slows down a bottleneck. + // Since Swift files within the same target should build a single module, it is reasonable to assume that they all + // share the same build settings. + let languageAndTarget = TargetAndLanguage(target: fileIndexInfo.target, language: fileIndexInfo.language) + fileIndexInfosToBatch[languageAndTarget, default: []].append(fileIndexInfo) + } + let batchedPartitions = + fileIndexInfosToBatch + .sorted { $0.key < $1.key } // Ensure we get a deterministic partition order + .flatMap { targetAndLanguage, files in + // The batch size of 5 was chosen without too many significant performance measurements because most projects + // currently indexed by SourceKit-LSP are limited by preparation time instead of indexing time and it's thus + // hard to quanify the performance characteristics of different batch sizes. 5 seems like a good trade-off to + // share work between files within the same target without overloading a single job with too many files and + // thus losing parallelism. + files.partition(intoBatchesOfSize: 5).map { (targetAndLanguage.target, targetAndLanguage.language, $0) } + } + return partitions + batchedPartitions } } diff --git a/Sources/ToolchainRegistry/Toolchain.swift b/Sources/ToolchainRegistry/Toolchain.swift index 0848e258f..a73cb64a3 100644 --- a/Sources/ToolchainRegistry/Toolchain.swift +++ b/Sources/ToolchainRegistry/Toolchain.swift @@ -14,6 +14,7 @@ package import Foundation import RegexBuilder import SKLogging import SwiftExtensions +import TSCExtensions import class TSCBasic.Process @@ -144,6 +145,56 @@ public final class Toolchain: Sendable { } } + private let canIndexMultipleSwiftFilesInSingleInvocationTask = ThreadSafeBox?>( + initialValue: nil + ) + + /// Checks if the Swift compiler in this toolchain can index multiple Swift files in a single compiler invocation, i.e + /// if the Swift compiler contains https://github.com/swiftlang/swift-driver/pull/1979. + package var canIndexMultipleSwiftFilesInSingleInvocation: Bool { + get async { + let task = canIndexMultipleSwiftFilesInSingleInvocationTask.withLock { task in + if let task { + return task + } + let newTask = Task { () -> Bool in + #if compiler(>=6.4) + #warning( + "Once we no longer Swift 6.2 toolchains, we can assume that the compiler has https://github.com/swiftlang/swift-driver/pull/1979" + ) + #endif + let result = await orLog("Getting frontend invocation to check if multi-file indexing is supported") { + guard let swiftc else { + throw SwiftVersionParsingError.failedToFindSwiftc + } + return try await Process.run( + arguments: [ + swiftc.filePath, + "-index-file", "a.swift", "b.swift", + "-index-file-path", "a.swift", + "-index-file-path", "b.swift", + "-###", + ], + workingDirectory: nil + ).utf8Output() + } + guard let result else { + return false + } + + // Before https://github.com/swiftlang/swift-driver/pull/1979, only the last `-index-file-path` was declared + // as `-primary-file`. With https://github.com/swiftlang/swift-driver/pull/1979, all `-index-file-path`s are + // passed as primary files to the frontend. + return result.contains("-primary-file a.swift") && result.contains("-primary-file b.swift") + } + task = newTask + return newTask + } + + return await task.value + } + } + package init( identifier: String, displayName: String, diff --git a/Tests/SourceKitLSPTests/BackgroundIndexingTests.swift b/Tests/SourceKitLSPTests/BackgroundIndexingTests.swift index 4814e6b78..7360ae9c6 100644 --- a/Tests/SourceKitLSPTests/BackgroundIndexingTests.swift +++ b/Tests/SourceKitLSPTests/BackgroundIndexingTests.swift @@ -2775,6 +2775,224 @@ final class BackgroundIndexingTests: XCTestCase { return true } } + + func testIndexMultipleSwiftFilesInSameCompilerInvocation() async throws { + try await SkipUnless.canIndexMultipleSwiftFilesInSingleInvocation() + let hooks = Hooks( + indexHooks: IndexHooks( + updateIndexStoreTaskDidStart: { taskDescription in + XCTAssertEqual( + taskDescription.filesToIndex.map(\.file.sourceFile.fileURL?.lastPathComponent), + ["First.swift", "Second.swift"] + ) + } + ) + ) + _ = try await SwiftPMTestProject( + files: [ + "First.swift": "", + "Second.swift": "", + ], + hooks: hooks, + enableBackgroundIndexing: true + ) + } + + func testIndexMultipleSwiftFilesWithExistingOutputFileMap() async throws { + actor BuildServer: CustomBuildServer { + let inProgressRequestsTracker = CustomBuildServerInProgressRequestTracker() + private let projectRoot: URL + + init(projectRoot: URL, connectionToSourceKitLSP: any Connection) { + self.projectRoot = projectRoot + } + + func initializeBuildRequest(_ request: InitializeBuildRequest) async throws -> InitializeBuildResponse { + return try initializationResponseSupportingBackgroundIndexing( + projectRoot: projectRoot, + outputPathsProvider: false + ) + } + + func buildTargetSourcesRequest(_ request: BuildTargetSourcesRequest) async throws -> BuildTargetSourcesResponse { + return self.dummyTargetSourcesResponse(files: [ + DocumentURI(projectRoot.appending(component: "MyFile.swift")), + DocumentURI(projectRoot.appending(component: "MyOtherFile.swift")), + ]) + } + + func textDocumentSourceKitOptionsRequest( + _ request: TextDocumentSourceKitOptionsRequest + ) async throws -> TextDocumentSourceKitOptionsResponse? { + var arguments = [ + try projectRoot.appending(component: "MyFile.swift").filePath, + try projectRoot.appending(component: "MyOtherFile.swift").filePath, + ] + if let defaultSDKPath { + arguments += ["-sdk", defaultSDKPath] + } + arguments += ["-index-unit-output-path", request.textDocument.uri.pseudoPath + ".o"] + arguments += ["-output-file-map", "dummy.json"] + return TextDocumentSourceKitOptionsResponse(compilerArguments: arguments) + } + } + + let project = try await CustomBuildServerTestProject( + files: [ + "MyFile.swift": """ + func 1️⃣foo() {} + """, + "MyOtherFile.swift": """ + func 2️⃣bar() { + 3️⃣foo() + } + """, + ], + buildServer: BuildServer.self, + enableBackgroundIndexing: true + ) + + let (uri, positions) = try project.openDocument("MyFile.swift") + let prepare = try await project.testClient.send( + CallHierarchyPrepareRequest(textDocument: TextDocumentIdentifier(uri), position: positions["1️⃣"]) + ) + let initialItem = try XCTUnwrap(prepare?.only) + let calls = try await project.testClient.send(CallHierarchyIncomingCallsRequest(item: initialItem)) + XCTAssertEqual( + calls, + [ + CallHierarchyIncomingCall( + from: CallHierarchyItem( + name: "bar()", + kind: .function, + tags: nil, + uri: try project.uri(for: "MyOtherFile.swift"), + range: Range(try project.position(of: "2️⃣", in: "MyOtherFile.swift")), + selectionRange: Range(try project.position(of: "2️⃣", in: "MyOtherFile.swift")), + data: .dictionary([ + "usr": .string("s:4main3baryyF"), + "uri": .string(try project.uri(for: "MyOtherFile.swift").stringValue), + ]) + ), + fromRanges: [Range(try project.position(of: "3️⃣", in: "MyOtherFile.swift"))] + ) + ] + ) + } + + func testSwiftFilesInSameTargetHaveDifferentBuildSettings() async throws { + // In the real world, this shouldn't happen. If the files within the same target and thus module have different + // build settings, we wouldn't be able to build them with whole-module-optimization. + // Check for this anyway to make sure that we provide reasonable behavior even for build servers that are somewhat + // misbehaving, eg. if for some reasons targets and modules don't line up within the build server. + actor BuildServer: CustomBuildServer { + let inProgressRequestsTracker = CustomBuildServerInProgressRequestTracker() + private let projectRoot: URL + + init(projectRoot: URL, connectionToSourceKitLSP: any Connection) { + self.projectRoot = projectRoot + } + + func initializeBuildRequest(_ request: InitializeBuildRequest) async throws -> InitializeBuildResponse { + return try initializationResponseSupportingBackgroundIndexing( + projectRoot: projectRoot, + outputPathsProvider: false + ) + } + + func buildTargetSourcesRequest(_ request: BuildTargetSourcesRequest) async throws -> BuildTargetSourcesResponse { + return self.dummyTargetSourcesResponse(files: [ + DocumentURI(projectRoot.appending(component: "MyFile.swift")), + DocumentURI(projectRoot.appending(component: "MyOtherFile.swift")), + ]) + } + + func textDocumentSourceKitOptionsRequest( + _ request: TextDocumentSourceKitOptionsRequest + ) async throws -> TextDocumentSourceKitOptionsResponse? { + var arguments = [ + try projectRoot.appending(component: "MyFile.swift").filePath, + try projectRoot.appending(component: "MyOtherFile.swift").filePath, + ] + if let defaultSDKPath { + arguments += ["-sdk", defaultSDKPath] + } + arguments += ["-index-unit-output-path", request.textDocument.uri.pseudoPath + ".o"] + if request.textDocument.uri.fileURL?.lastPathComponent == "MyFile.swift" { + arguments += ["-DMY_FILE"] + } + if request.textDocument.uri.fileURL?.lastPathComponent == "MyOtherFile.swift" { + arguments += ["-DMY_OTHER_FILE"] + } + return TextDocumentSourceKitOptionsResponse(compilerArguments: arguments) + } + } + + let project = try await CustomBuildServerTestProject( + files: [ + "MyFile.swift": """ + func 1️⃣foo() {} + + #if MY_FILE + func 2️⃣boo() { + 3️⃣foo() + } + #endif + """, + "MyOtherFile.swift": """ + #if MY_OTHER_FILE + func 4️⃣bar() { + 5️⃣foo() + } + #endif + """, + ], + buildServer: BuildServer.self, + enableBackgroundIndexing: true + ) + + let (uri, positions) = try project.openDocument("MyFile.swift") + let prepare = try await project.testClient.send( + CallHierarchyPrepareRequest(textDocument: TextDocumentIdentifier(uri), position: positions["1️⃣"]) + ) + let initialItem = try XCTUnwrap(prepare?.only) + let calls = try await project.testClient.send(CallHierarchyIncomingCallsRequest(item: initialItem)) + XCTAssertEqual( + calls, + [ + CallHierarchyIncomingCall( + from: CallHierarchyItem( + name: "bar()", + kind: .function, + tags: nil, + uri: try project.uri(for: "MyOtherFile.swift"), + range: Range(try project.position(of: "4️⃣", in: "MyOtherFile.swift")), + selectionRange: Range(try project.position(of: "4️⃣", in: "MyOtherFile.swift")), + data: .dictionary([ + "usr": .string("s:4main3baryyF"), + "uri": .string(try project.uri(for: "MyOtherFile.swift").stringValue), + ]) + ), + fromRanges: [Range(try project.position(of: "5️⃣", in: "MyOtherFile.swift"))] + ), + CallHierarchyIncomingCall( + from: CallHierarchyItem( + name: "boo()", + kind: .function, + tags: nil, + uri: try project.uri(for: "MyFile.swift"), + range: Range(try project.position(of: "2️⃣", in: "MyFile.swift")), + selectionRange: Range(try project.position(of: "2️⃣", in: "MyFile.swift")), + data: .dictionary([ + "usr": .string("s:4main3booyyF"), + "uri": .string(try project.uri(for: "MyFile.swift").stringValue), + ]) + ), + fromRanges: [Range(try project.position(of: "3️⃣", in: "MyFile.swift"))] + ), + ] + ) + } } extension HoverResponseContents {