diff --git a/Contributor Documentation/BSP Extensions.md b/Contributor Documentation/BSP Extensions.md index cb28c6f3d..2701ef4ab 100644 --- a/Contributor Documentation/BSP Extensions.md +++ b/Contributor Documentation/BSP Extensions.md @@ -22,6 +22,9 @@ export interface SourceKitInitializeBuildResponseData { * for `swiftc` or `clang` invocations **/ indexStorePath?: string; + /** Options to control how many targets should be prepared simultaneously by SourceKit-LSP. */ + multiTargetPreparation?: MultiTargetPreparationSupport + /** Whether the server set the `outputPath` property in the `buildTarget/sources` request */ outputPathsProvider?: bool; @@ -34,7 +37,17 @@ export interface SourceKitInitializeBuildResponseData { /** The files to watch for changes. * Changes to these files are sent to the BSP server using `workspace/didChangeWatchedFiles`. * `FileSystemWatcher` is the same as in LSP. */ - watchers: [FileSystemWatcher]? + watchers?: [FileSystemWatcher] +} + +export interface MultiTargetPreparationSupport { + /** Whether the build server can prepare multiple targets in parallel. + * If this value is not provided, it will be assumed to be `true`. */ + supported?: bool; + + /** The number of targets to prepare in parallel. + * If not provided, SourceKit-LSP will calculate an appropriate value based on the environment. */ + batchSize?: int; } ``` diff --git a/Sources/BuildServerIntegration/BuildServerManager.swift b/Sources/BuildServerIntegration/BuildServerManager.swift index 5346d8337..679ddcbec 100644 --- a/Sources/BuildServerIntegration/BuildServerManager.swift +++ b/Sources/BuildServerIntegration/BuildServerManager.swift @@ -322,7 +322,7 @@ package actor BuildServerManager: QueueBasedMessageHandler { private var connectionToClient: BuildServerManagerConnectionToClient - /// The build serer adapter that is used to answer build server queries. + /// The build server adapter that is used to answer build server queries. private var buildServerAdapter: BuildServerAdapter? /// The build server adapter after initialization finishes. When sending messages to the BSP server, this should be diff --git a/Sources/BuildServerIntegration/BuiltInBuildServer.swift b/Sources/BuildServerIntegration/BuiltInBuildServer.swift index 814448e61..aabaecaca 100644 --- a/Sources/BuildServerIntegration/BuiltInBuildServer.swift +++ b/Sources/BuildServerIntegration/BuiltInBuildServer.swift @@ -35,6 +35,9 @@ package protocol BuiltInBuildServer: AnyObject, Sendable { /// The path to put the index database, if any. var indexDatabasePath: URL? { get async } + /// Whether the build server can prepare multiple targets in parallel. + var supportsMultiTargetPreparation: Bool { get } + /// Whether the build server is capable of preparing a target for indexing and determining the output paths for the /// target, ie. whether the `prepare` method has been implemented and this build server populates the `outputPath` /// property in the `buildTarget/sources` request. diff --git a/Sources/BuildServerIntegration/BuiltInBuildServerAdapter.swift b/Sources/BuildServerIntegration/BuiltInBuildServerAdapter.swift index a22396175..328a2d45b 100644 --- a/Sources/BuildServerIntegration/BuiltInBuildServerAdapter.swift +++ b/Sources/BuildServerIntegration/BuiltInBuildServerAdapter.swift @@ -94,6 +94,7 @@ actor BuiltInBuildServerAdapter: QueueBasedMessageHandler { indexStorePath: await orLog("getting index store file path") { try await underlyingBuildServer.indexStorePath?.filePath }, + multiTargetPreparation: MultiTargetPreparationSupport(supported: underlyingBuildServer.supportsMultiTargetPreparation), outputPathsProvider: underlyingBuildServer.supportsPreparationAndOutputPaths, prepareProvider: underlyingBuildServer.supportsPreparationAndOutputPaths, sourceKitOptionsProvider: true, diff --git a/Sources/BuildServerIntegration/FixedCompilationDatabaseBuildServer.swift b/Sources/BuildServerIntegration/FixedCompilationDatabaseBuildServer.swift index ef084482a..46370c77e 100644 --- a/Sources/BuildServerIntegration/FixedCompilationDatabaseBuildServer.swift +++ b/Sources/BuildServerIntegration/FixedCompilationDatabaseBuildServer.swift @@ -59,6 +59,8 @@ package actor FixedCompilationDatabaseBuildServer: BuiltInBuildServer { indexStorePath?.deletingLastPathComponent().appendingPathComponent("IndexDatabase") } + package nonisolated var supportsMultiTargetPreparation: Bool { true } + package nonisolated var supportsPreparationAndOutputPaths: Bool { false } private static func parseCompileFlags(at configPath: URL) throws -> [String] { diff --git a/Sources/BuildServerIntegration/JSONCompilationDatabaseBuildServer.swift b/Sources/BuildServerIntegration/JSONCompilationDatabaseBuildServer.swift index abd8e5196..acf5eb42e 100644 --- a/Sources/BuildServerIntegration/JSONCompilationDatabaseBuildServer.swift +++ b/Sources/BuildServerIntegration/JSONCompilationDatabaseBuildServer.swift @@ -96,6 +96,8 @@ package actor JSONCompilationDatabaseBuildServer: BuiltInBuildServer { indexStorePath?.deletingLastPathComponent().appendingPathComponent("IndexDatabase") } + package nonisolated var supportsMultiTargetPreparation: Bool { true } + package nonisolated var supportsPreparationAndOutputPaths: Bool { false } package init( diff --git a/Sources/BuildServerIntegration/LegacyBuildServer.swift b/Sources/BuildServerIntegration/LegacyBuildServer.swift index 44e49b262..026454e1a 100644 --- a/Sources/BuildServerIntegration/LegacyBuildServer.swift +++ b/Sources/BuildServerIntegration/LegacyBuildServer.swift @@ -138,6 +138,8 @@ actor LegacyBuildServer: MessageHandler, BuiltInBuildServer { connectionToSourceKitLSP.send(OnBuildTargetDidChangeNotification(changes: nil)) } + package nonisolated var supportsMultiTargetPreparation: Bool { true } + package nonisolated var supportsPreparationAndOutputPaths: Bool { false } package func buildTargets(request: WorkspaceBuildTargetsRequest) async throws -> WorkspaceBuildTargetsResponse { diff --git a/Sources/BuildServerIntegration/SwiftPMBuildServer.swift b/Sources/BuildServerIntegration/SwiftPMBuildServer.swift index 4f9737a23..db1223820 100644 --- a/Sources/BuildServerIntegration/SwiftPMBuildServer.swift +++ b/Sources/BuildServerIntegration/SwiftPMBuildServer.swift @@ -459,6 +459,8 @@ package actor SwiftPMBuildServer: BuiltInBuildServer { connectionToSourceKitLSP.send(OnBuildTargetDidChangeNotification(changes: nil)) } + package nonisolated var supportsMultiTargetPreparation: Bool { false } + package nonisolated var supportsPreparationAndOutputPaths: Bool { options.backgroundIndexingOrDefault } package var buildPath: URL { diff --git a/Sources/BuildServerProtocol/Messages/InitializeBuildRequest.swift b/Sources/BuildServerProtocol/Messages/InitializeBuildRequest.swift index 15c720d70..13579335c 100644 --- a/Sources/BuildServerProtocol/Messages/InitializeBuildRequest.swift +++ b/Sources/BuildServerProtocol/Messages/InitializeBuildRequest.swift @@ -269,6 +269,9 @@ public struct SourceKitInitializeBuildResponseData: LSPAnyCodable, Codable, Send /// The path at which SourceKit-LSP can store its index database, aggregating data from `indexStorePath` public var indexStorePath: String? + /// Options to control how many targets should be prepared simultaneously by SourceKit-LSP. + public var multiTargetPreparation: MultiTargetPreparationSupport? + /// Whether the server implements the `buildTarget/outputPaths` request. public var outputPathsProvider: Bool? @@ -286,12 +289,14 @@ public struct SourceKitInitializeBuildResponseData: LSPAnyCodable, Codable, Send public init( indexDatabasePath: String? = nil, indexStorePath: String? = nil, + multiTargetPreparation: MultiTargetPreparationSupport? = nil, watchers: [FileSystemWatcher]? = nil, prepareProvider: Bool? = nil, sourceKitOptionsProvider: Bool? = nil ) { self.indexDatabasePath = indexDatabasePath self.indexStorePath = indexStorePath + self.multiTargetPreparation = multiTargetPreparation self.watchers = watchers self.prepareProvider = prepareProvider self.sourceKitOptionsProvider = sourceKitOptionsProvider @@ -300,6 +305,7 @@ public struct SourceKitInitializeBuildResponseData: LSPAnyCodable, Codable, Send public init( indexDatabasePath: String? = nil, indexStorePath: String? = nil, + multiTargetPreparation: MultiTargetPreparationSupport? = nil, outputPathsProvider: Bool? = nil, prepareProvider: Bool? = nil, sourceKitOptionsProvider: Bool? = nil, @@ -307,6 +313,7 @@ public struct SourceKitInitializeBuildResponseData: LSPAnyCodable, Codable, Send ) { self.indexDatabasePath = indexDatabasePath self.indexStorePath = indexStorePath + self.multiTargetPreparation = multiTargetPreparation self.outputPathsProvider = outputPathsProvider self.prepareProvider = prepareProvider self.sourceKitOptionsProvider = sourceKitOptionsProvider @@ -320,6 +327,9 @@ public struct SourceKitInitializeBuildResponseData: LSPAnyCodable, Codable, Send if case .string(let indexStorePath) = dictionary[CodingKeys.indexStorePath.stringValue] { self.indexStorePath = indexStorePath } + if case .dictionary(let multiTargetPreparation) = dictionary[CodingKeys.multiTargetPreparation.stringValue] { + self.multiTargetPreparation = MultiTargetPreparationSupport(fromLSPDictionary: multiTargetPreparation) + } if case .bool(let outputPathsProvider) = dictionary[CodingKeys.outputPathsProvider.stringValue] { self.outputPathsProvider = outputPathsProvider } @@ -342,6 +352,9 @@ public struct SourceKitInitializeBuildResponseData: LSPAnyCodable, Codable, Send if let indexStorePath { result[CodingKeys.indexStorePath.stringValue] = .string(indexStorePath) } + if let multiTargetPreparation { + result[CodingKeys.multiTargetPreparation.stringValue] = multiTargetPreparation.encodeToLSPAny() + } if let outputPathsProvider { result[CodingKeys.outputPathsProvider.stringValue] = .bool(outputPathsProvider) } @@ -357,3 +370,38 @@ public struct SourceKitInitializeBuildResponseData: LSPAnyCodable, Codable, Send return .dictionary(result) } } + +public struct MultiTargetPreparationSupport: LSPAnyCodable, Codable, Sendable { + /// Whether the build server can prepare multiple targets in parallel. + /// If this value is not provided, it will be assumed to be `true`. + public var supported: Bool? + + /// The number of targets to prepare in parallel. + /// If not provided, SourceKit-LSP will calculate an appropriate value based on the environment. + public var batchSize: Int? + + public init(supported: Bool? = nil, batchSize: Int? = nil) { + self.supported = supported + self.batchSize = batchSize + } + + public init?(fromLSPDictionary dictionary: [String: LanguageServerProtocol.LSPAny]) { + if case .bool(let supported) = dictionary[CodingKeys.supported.stringValue] { + self.supported = supported + } + if case .int(let batchSize) = dictionary[CodingKeys.batchSize.stringValue] { + self.batchSize = batchSize + } + } + + public func encodeToLSPAny() -> LanguageServerProtocol.LSPAny { + var result: [String: LSPAny] = [:] + if let supported { + result[CodingKeys.supported.stringValue] = .bool(supported) + } + if let batchSize { + result[CodingKeys.batchSize.stringValue] = .int(batchSize) + } + return .dictionary(result) + } +} diff --git a/Sources/SKTestSupport/CustomBuildServerTestProject.swift b/Sources/SKTestSupport/CustomBuildServerTestProject.swift index 2f73fdff6..5c082fb2c 100644 --- a/Sources/SKTestSupport/CustomBuildServerTestProject.swift +++ b/Sources/SKTestSupport/CustomBuildServerTestProject.swift @@ -175,12 +175,14 @@ package extension CustomBuildServer { func initializationResponseSupportingBackgroundIndexing( projectRoot: URL, - outputPathsProvider: Bool + outputPathsProvider: Bool, + multiTargetPreparation: MultiTargetPreparationSupport? = nil ) throws -> InitializeBuildResponse { return initializationResponse( initializeData: SourceKitInitializeBuildResponseData( indexDatabasePath: try projectRoot.appendingPathComponent("index-db").filePath, indexStorePath: try projectRoot.appendingPathComponent("index-store").filePath, + multiTargetPreparation: multiTargetPreparation, outputPathsProvider: outputPathsProvider, prepareProvider: true, sourceKitOptionsProvider: true diff --git a/Sources/SemanticIndex/PreparationTaskDescription.swift b/Sources/SemanticIndex/PreparationTaskDescription.swift index 4d45be438..f19a83352 100644 --- a/Sources/SemanticIndex/PreparationTaskDescription.swift +++ b/Sources/SemanticIndex/PreparationTaskDescription.swift @@ -39,6 +39,9 @@ package struct PreparationTaskDescription: IndexTaskDescription { private let preparationUpToDateTracker: UpToDateTracker + /// The purpose of the preparation task. + private let purpose: TargetPreparationPurpose + /// See `SemanticIndexManager.logMessageToIndexLog`. private let logMessageToIndexLog: @Sendable (_ message: String, _ type: WindowMessageType, _ structure: StructuredLogKind) -> Void @@ -63,6 +66,7 @@ package struct PreparationTaskDescription: IndexTaskDescription { targetsToPrepare: [BuildTargetIdentifier], buildServerManager: BuildServerManager, preparationUpToDateTracker: UpToDateTracker, + purpose: TargetPreparationPurpose, logMessageToIndexLog: @escaping @Sendable ( _ message: String, _ type: WindowMessageType, _ structure: StructuredLogKind @@ -72,6 +76,7 @@ package struct PreparationTaskDescription: IndexTaskDescription { self.targetsToPrepare = targetsToPrepare self.buildServerManager = buildServerManager self.preparationUpToDateTracker = preparationUpToDateTracker + self.purpose = purpose self.logMessageToIndexLog = logMessageToIndexLog self.hooks = hooks } @@ -119,14 +124,22 @@ package struct PreparationTaskDescription: IndexTaskDescription { to currentlyExecutingTasks: [PreparationTaskDescription] ) -> [TaskDependencyAction] { return currentlyExecutingTasks.compactMap { (other) -> TaskDependencyAction? in - if other.targetsToPrepare.count > self.targetsToPrepare.count { - // If there is an prepare operation with more targets already running, suspend it. - // The most common use case for this is if we prepare all targets simultaneously during the initial preparation - // when a project is opened and need a single target indexed for user interaction. We should suspend the - // workspace-wide preparation and just prepare the currently needed target. + if other.purpose == .forIndexing && self.purpose == .forEditorFunctionality { + // If we're running a background indexing operation but need a target indexed for user interaction, + // we should prioritize the latter. return .cancelAndRescheduleDependency(other) } return .waitAndElevatePriorityOfDependency(other) } } } + +/// The reason why a target is being prepared. This is used to determine the `IndexProgressStatus` +/// and to prioritize preparation tasks when several of them are running. +package enum TargetPreparationPurpose: Comparable { + /// We are preparing the target so we can index files in it. + case forIndexing + + /// We are preparing the target to provide semantic functionality in one of its files. + case forEditorFunctionality +} diff --git a/Sources/SemanticIndex/SemanticIndexManager.swift b/Sources/SemanticIndex/SemanticIndexManager.swift index cad195edf..5335a0edb 100644 --- a/Sources/SemanticIndex/SemanticIndexManager.swift +++ b/Sources/SemanticIndex/SemanticIndexManager.swift @@ -145,15 +145,6 @@ private struct InProgressPrepareForEditorTask { let task: Task } -/// The reason why a target is being prepared. This is used to determine the `IndexProgressStatus`. -private enum TargetPreparationPurpose: Comparable { - /// We are preparing the target so we can index files in it. - case forIndexing - - /// We are preparing the target to provide semantic functionality in one of its files. - case forEditorFunctionality -} - /// An entry in `SemanticIndexManager.inProgressPreparationTasks`. private struct InProgressPreparationTask { let task: OpaqueQueuedIndexTask @@ -222,6 +213,9 @@ package final actor SemanticIndexManager { /// The parameter is the number of files that were scheduled to be indexed. private let indexTasksWereScheduled: @Sendable (_ numberOfFileScheduled: Int) -> Void + /// The size of the batches in which the `SemanticIndexManager` should dispatch preparation tasks. + private let indexTaskBatchSize: Int + /// Callback that is called when `progressStatus` might have changed. private let indexProgressStatusDidChange: @Sendable () -> Void @@ -261,6 +255,7 @@ package final actor SemanticIndexManager { updateIndexStoreTimeout: Duration, hooks: IndexHooks, indexTaskScheduler: TaskScheduler, + indexTaskBatchSize: Int, logMessageToIndexLog: @escaping @Sendable ( _ message: String, _ type: WindowMessageType, _ structure: StructuredLogKind @@ -273,6 +268,7 @@ package final actor SemanticIndexManager { self.updateIndexStoreTimeout = updateIndexStoreTimeout self.hooks = hooks self.indexTaskScheduler = indexTaskScheduler + self.indexTaskBatchSize = indexTaskBatchSize self.logMessageToIndexLog = logMessageToIndexLog self.indexTasksWereScheduled = indexTasksWereScheduled self.indexProgressStatusDidChange = indexProgressStatusDidChange @@ -655,6 +651,7 @@ package final actor SemanticIndexManager { targetsToPrepare: targetsToPrepare, buildServerManager: self.buildServerManager, preparationUpToDateTracker: preparationUpToDateTracker, + purpose: purpose, logMessageToIndexLog: logMessageToIndexLog, hooks: hooks ) @@ -877,10 +874,7 @@ package final actor SemanticIndexManager { var indexTasks: [Task] = [] - // TODO: When we can index multiple targets concurrently in SwiftPM, increase the batch size to half the - // processor count, so we can get parallelism during preparation. - // (https://github.com/swiftlang/sourcekit-lsp/issues/1262) - for targetsBatch in sortedTargets.partition(intoBatchesOfSize: 1) { + for targetsBatch in sortedTargets.partition(intoBatchesOfSize: indexTaskBatchSize) { let preparationTaskID = UUID() let filesToIndex = targetsBatch.flatMap({ filesByTarget[$0]! }) @@ -904,10 +898,7 @@ 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 { - // 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 filesByTarget[target]!.partition(intoBatchesOfSize: 1) { + for fileBatch in filesByTarget[target]!.partition(intoBatchesOfSize: indexTaskBatchSize) { taskGroup.addTask { await self.updateIndexStore( for: fileBatch, diff --git a/Sources/SourceKitLSP/Workspace.swift b/Sources/SourceKitLSP/Workspace.swift index 41fd7dc05..dba9df84c 100644 --- a/Sources/SourceKitLSP/Workspace.swift +++ b/Sources/SourceKitLSP/Workspace.swift @@ -162,12 +162,25 @@ package final class Workspace: Sendable, BuildServerManagerDelegate { if options.backgroundIndexingOrDefault, let uncheckedIndex, await buildServerManager.initializationData?.prepareProvider ?? false { + let shouldIndexInParallel = await buildServerManager.initializationData?.multiTargetPreparation?.supported ?? true + let batchSize: Int + if shouldIndexInParallel { + if let customBatchSize = await buildServerManager.initializationData?.multiTargetPreparation?.batchSize { + batchSize = customBatchSize + } else { + let processorCount = ProcessInfo.processInfo.activeProcessorCount + batchSize = max(1, processorCount / 2) + } + } else { + batchSize = 1 + } self.semanticIndexManager = SemanticIndexManager( index: uncheckedIndex, buildServerManager: buildServerManager, updateIndexStoreTimeout: options.indexOrDefault.updateIndexStoreTimeoutOrDefault, hooks: hooks.indexHooks, indexTaskScheduler: indexTaskScheduler, + indexTaskBatchSize: batchSize, logMessageToIndexLog: { [weak sourceKitLSPServer] in sourceKitLSPServer?.logMessageToIndexLog(message: $0, type: $1, structure: $2) }, diff --git a/Tests/SourceKitLSPTests/BackgroundIndexingTests.swift b/Tests/SourceKitLSPTests/BackgroundIndexingTests.swift index 9538222d1..e1f3b0cb4 100644 --- a/Tests/SourceKitLSPTests/BackgroundIndexingTests.swift +++ b/Tests/SourceKitLSPTests/BackgroundIndexingTests.swift @@ -2624,6 +2624,84 @@ final class BackgroundIndexingTests: XCTestCase { let symbols = try await project.testClient.send(WorkspaceSymbolsRequest(query: "myTestFu")) XCTAssertEqual(symbols?.count, 1) } + + func testBuildServerUsesCustomTaskBatchSize() async throws { + final class BuildServer: CustomBuildServer { + let inProgressRequestsTracker = CustomBuildServerInProgressRequestTracker() + private let projectRoot: URL + private var testFileURL: URL { projectRoot.appendingPathComponent("test.swift").standardized } + + nonisolated(unsafe) var preparedTargetBatches = [[BuildTargetIdentifier]]() + + required init(projectRoot: URL, connectionToSourceKitLSP: any LanguageServerProtocol.Connection) { + self.projectRoot = projectRoot + } + + func initializeBuildRequest(_ request: InitializeBuildRequest) async throws -> InitializeBuildResponse { + return try initializationResponseSupportingBackgroundIndexing( + projectRoot: projectRoot, + outputPathsProvider: false, + multiTargetPreparation: MultiTargetPreparationSupport(supported: true, batchSize: 3) + ) + } + + func buildTargetSourcesRequest(_ request: BuildTargetSourcesRequest) async throws -> BuildTargetSourcesResponse { + var dummyTargets = [BuildTargetIdentifier]() + for i in 0..<10 { + dummyTargets.append(BuildTargetIdentifier(uri: try! URI(string: "dummy://dummy-\(i)"))) + } + return BuildTargetSourcesResponse(items: dummyTargets.map { + SourcesItem(target: $0, sources: [SourceItem(uri: URI(testFileURL), kind: .file, generated: false)]) + }) + } + + func textDocumentSourceKitOptionsRequest( + _ request: TextDocumentSourceKitOptionsRequest + ) async throws -> TextDocumentSourceKitOptionsResponse? { + return TextDocumentSourceKitOptionsResponse(compilerArguments: [request.textDocument.uri.pseudoPath]) + } + + func prepareTarget(_ request: BuildTargetPrepareRequest) async throws -> VoidResponse { + preparedTargetBatches.append(request.targets.sorted { $0.uri.stringValue < $1.uri.stringValue }) + return VoidResponse() + } + } + + let project = try await CustomBuildServerTestProject( + files: [ + "test.swift": """ + func testFunction() {} + """ + ], + buildServer: BuildServer.self, + enableBackgroundIndexing: true, + ) + + try await project.testClient.send(SynchronizeRequest(index: true)) + + let buildServer = try project.buildServer() + let preparedBatches = buildServer.preparedTargetBatches.sorted { $0[0].uri.stringValue < $1[0].uri.stringValue } + XCTAssertEqual(preparedBatches, [ + [ + BuildTargetIdentifier(uri: try! URI(string: "dummy://dummy-0")), + BuildTargetIdentifier(uri: try! URI(string: "dummy://dummy-1")), + BuildTargetIdentifier(uri: try! URI(string: "dummy://dummy-2")), + ], + [ + BuildTargetIdentifier(uri: try! URI(string: "dummy://dummy-3")), + BuildTargetIdentifier(uri: try! URI(string: "dummy://dummy-4")), + BuildTargetIdentifier(uri: try! URI(string: "dummy://dummy-5")), + ], + [ + BuildTargetIdentifier(uri: try! URI(string: "dummy://dummy-6")), + BuildTargetIdentifier(uri: try! URI(string: "dummy://dummy-7")), + BuildTargetIdentifier(uri: try! URI(string: "dummy://dummy-8")), + ], + [ + BuildTargetIdentifier(uri: try! URI(string: "dummy://dummy-9")), + ] + ]) + } } extension HoverResponseContents {