From 5cce99b920fcf55ebd9eab02efa830076e7fd4f8 Mon Sep 17 00:00:00 2001 From: Alex Hoppen Date: Thu, 9 May 2024 16:52:58 -0700 Subject: [PATCH 1/7] Make `IndexTaskDescription` protocol-based instead of enum-based This simplifies the implementation. --- .../SemanticIndex/IndexTaskDescription.swift | 110 +++++++----------- .../PreparationTaskDescription.swift | 4 +- .../SemanticIndex/SemanticIndexManager.swift | 16 +-- .../UpdateIndexStoreTaskDescription.swift | 3 +- Sources/SourceKitLSP/SourceKitLSPServer.swift | 2 +- Sources/SourceKitLSP/Workspace.swift | 8 +- 6 files changed, 62 insertions(+), 81 deletions(-) diff --git a/Sources/SemanticIndex/IndexTaskDescription.swift b/Sources/SemanticIndex/IndexTaskDescription.swift index e2e2c21f2..33f032dcd 100644 --- a/Sources/SemanticIndex/IndexTaskDescription.swift +++ b/Sources/SemanticIndex/IndexTaskDescription.swift @@ -12,94 +12,72 @@ import SKCore -/// A task that either prepares targets or updates the index store for a set of files. -public enum IndexTaskDescription: TaskDescriptionProtocol { - case updateIndexStore(UpdateIndexStoreTaskDescription) - case preparation(PreparationTaskDescription) +/// Protocol of tasks that are executed on the index task scheduler. +/// +/// It is assumed that `IndexTaskDescription` of different types are allowed to execute in parallel. +protocol IndexTaskDescription: TaskDescriptionProtocol { + /// A string that is unique to this type of `IndexTaskDescription`. It is used to produce unique IDs for tasks of + /// different types in `AnyIndexTaskDescription` + static var idPrefix: String { get } + + var id: UInt32 { get } +} + +extension IndexTaskDescription { + func dependencies( + to currentlyExecutingTasks: [AnyIndexTaskDescription] + ) -> [TaskDependencyAction] { + return self.dependencies(to: currentlyExecutingTasks.compactMap { $0.wrapped as? Self }) + .map { + switch $0 { + case .cancelAndRescheduleDependency(let td): + return .cancelAndRescheduleDependency(AnyIndexTaskDescription(td)) + case .waitAndElevatePriorityOfDependency(let td): + return .waitAndElevatePriorityOfDependency(AnyIndexTaskDescription(td)) + } + } + + } +} + +/// Type-erased wrapper of an `IndexTaskDescription`. +public struct AnyIndexTaskDescription: TaskDescriptionProtocol { + let wrapped: any IndexTaskDescription + + init(_ wrapped: any IndexTaskDescription) { + self.wrapped = wrapped + } public var isIdempotent: Bool { - switch self { - case .updateIndexStore(let taskDescription): return taskDescription.isIdempotent - case .preparation(let taskDescription): return taskDescription.isIdempotent - } + return wrapped.isIdempotent } public var estimatedCPUCoreCount: Int { - switch self { - case .updateIndexStore(let taskDescription): return taskDescription.estimatedCPUCoreCount - case .preparation(let taskDescription): return taskDescription.estimatedCPUCoreCount - } + return wrapped.estimatedCPUCoreCount } public var id: String { - switch self { - case .updateIndexStore(let taskDescription): return "indexing-\(taskDescription.id)" - case .preparation(let taskDescription): return "preparation-\(taskDescription.id)" - } + return "\(type(of: wrapped).idPrefix)-\(wrapped.id)" } public var description: String { - switch self { - case .updateIndexStore(let taskDescription): return taskDescription.description - case .preparation(let taskDescription): return taskDescription.description - } + return wrapped.description } public var redactedDescription: String { - switch self { - case .updateIndexStore(let taskDescription): return taskDescription.redactedDescription - case .preparation(let taskDescription): return taskDescription.redactedDescription - } + return wrapped.redactedDescription } public func execute() async { - switch self { - case .updateIndexStore(let taskDescription): return await taskDescription.execute() - case .preparation(let taskDescription): return await taskDescription.execute() - } + return await wrapped.execute() } /// Forward to the underlying task to compute the dependencies. Preparation and index tasks don't have any /// dependencies that are managed by `TaskScheduler`. `SemanticIndexManager` awaits the preparation of a target before /// indexing files within it. public func dependencies( - to currentlyExecutingTasks: [IndexTaskDescription] - ) -> [TaskDependencyAction] { - switch self { - case .updateIndexStore(let taskDescription): - let currentlyExecutingTasks = - currentlyExecutingTasks - .compactMap { (currentlyExecutingTask) -> UpdateIndexStoreTaskDescription? in - if case .updateIndexStore(let currentlyExecutingTask) = currentlyExecutingTask { - return currentlyExecutingTask - } - return nil - } - return taskDescription.dependencies(to: currentlyExecutingTasks).map { - switch $0 { - case .waitAndElevatePriorityOfDependency(let td): - return .waitAndElevatePriorityOfDependency(.updateIndexStore(td)) - case .cancelAndRescheduleDependency(let td): - return .cancelAndRescheduleDependency(.updateIndexStore(td)) - } - } - case .preparation(let taskDescription): - let currentlyExecutingTasks = - currentlyExecutingTasks - .compactMap { (currentlyExecutingTask) -> PreparationTaskDescription? in - if case .preparation(let currentlyExecutingTask) = currentlyExecutingTask { - return currentlyExecutingTask - } - return nil - } - return taskDescription.dependencies(to: currentlyExecutingTasks).map { - switch $0 { - case .waitAndElevatePriorityOfDependency(let td): - return .waitAndElevatePriorityOfDependency(.preparation(td)) - case .cancelAndRescheduleDependency(let td): - return .cancelAndRescheduleDependency(.preparation(td)) - } - } - } + to currentlyExecutingTasks: [AnyIndexTaskDescription] + ) -> [TaskDependencyAction] { + return wrapped.dependencies(to: currentlyExecutingTasks) } } diff --git a/Sources/SemanticIndex/PreparationTaskDescription.swift b/Sources/SemanticIndex/PreparationTaskDescription.swift index d2348b272..b48e6b93e 100644 --- a/Sources/SemanticIndex/PreparationTaskDescription.swift +++ b/Sources/SemanticIndex/PreparationTaskDescription.swift @@ -24,7 +24,9 @@ private var preparationIDForLogging = AtomicUInt32(initialValue: 1) /// Describes a task to prepare a set of targets. /// /// This task description can be scheduled in a `TaskScheduler`. -public struct PreparationTaskDescription: TaskDescriptionProtocol { +public struct PreparationTaskDescription: IndexTaskDescription { + public static let idPrefix = "prepare" + public let id = preparationIDForLogging.fetchAndIncrement() /// The targets that should be prepared. diff --git a/Sources/SemanticIndex/SemanticIndexManager.swift b/Sources/SemanticIndex/SemanticIndexManager.swift index 8069818ae..e1aac5862 100644 --- a/Sources/SemanticIndex/SemanticIndexManager.swift +++ b/Sources/SemanticIndex/SemanticIndexManager.swift @@ -44,20 +44,20 @@ public final actor SemanticIndexManager { /// The `TaskScheduler` that manages the scheduling of index tasks. This is shared among all `SemanticIndexManager`s /// in the process, to ensure that we don't schedule more index operations than processor cores from multiple /// workspaces. - private let indexTaskScheduler: TaskScheduler + private let indexTaskScheduler: TaskScheduler /// Callback that is called when an index task has finished. /// /// Currently only used for testing. - private let indexTaskDidFinish: (@Sendable (IndexTaskDescription) -> Void)? + private let indexTaskDidFinish: (@Sendable (AnyIndexTaskDescription) -> Void)? // MARK: - Public API public init( index: UncheckedIndex, buildSystemManager: BuildSystemManager, - indexTaskScheduler: TaskScheduler, - indexTaskDidFinish: (@Sendable (IndexTaskDescription) -> Void)? + indexTaskScheduler: TaskScheduler, + indexTaskDidFinish: (@Sendable (AnyIndexTaskDescription) -> Void)? ) { self.index = index.checked(for: .modifiedFiles) self.buildSystemManager = buildSystemManager @@ -133,12 +133,12 @@ public final actor SemanticIndexManager { private func prepare(targets: [ConfiguredTarget], priority: TaskPriority?) async { await self.indexTaskScheduler.schedule( priority: priority, - .preparation( + AnyIndexTaskDescription( PreparationTaskDescription( targetsToPrepare: targets, buildSystemManager: self.buildSystemManager, didFinishCallback: { [weak self] taskDescription in - self?.indexTaskDidFinish?(.preparation(taskDescription)) + self?.indexTaskDidFinish?(AnyIndexTaskDescription(taskDescription)) } ) ) @@ -149,13 +149,13 @@ public final actor SemanticIndexManager { private func updateIndexStore(for files: [DocumentURI], priority: TaskPriority?) async { await self.indexTaskScheduler.schedule( priority: priority, - .updateIndexStore( + AnyIndexTaskDescription( UpdateIndexStoreTaskDescription( filesToIndex: Set(files), buildSystemManager: self.buildSystemManager, index: self.index.unchecked, didFinishCallback: { [weak self] taskDescription in - self?.indexTaskDidFinish?(.updateIndexStore(taskDescription)) + self?.indexTaskDidFinish?(AnyIndexTaskDescription(taskDescription)) } ) ) diff --git a/Sources/SemanticIndex/UpdateIndexStoreTaskDescription.swift b/Sources/SemanticIndex/UpdateIndexStoreTaskDescription.swift index 46a8b6732..6b9968a14 100644 --- a/Sources/SemanticIndex/UpdateIndexStoreTaskDescription.swift +++ b/Sources/SemanticIndex/UpdateIndexStoreTaskDescription.swift @@ -25,7 +25,8 @@ private nonisolated(unsafe) var updateIndexStoreIDForLogging = AtomicUInt32(init /// Describes a task to index a set of source files. /// /// This task description can be scheduled in a `TaskScheduler`. -public struct UpdateIndexStoreTaskDescription: TaskDescriptionProtocol { +public struct UpdateIndexStoreTaskDescription: IndexTaskDescription { + public static let idPrefix = "update-indexstore" public let id = updateIndexStoreIDForLogging.fetchAndIncrement() /// The files that should be indexed. diff --git a/Sources/SourceKitLSP/SourceKitLSPServer.swift b/Sources/SourceKitLSP/SourceKitLSPServer.swift index 410a44c71..484310375 100644 --- a/Sources/SourceKitLSP/SourceKitLSPServer.swift +++ b/Sources/SourceKitLSP/SourceKitLSPServer.swift @@ -454,7 +454,7 @@ public actor SourceKitLSPServer { /// /// Shared process-wide to ensure the scheduled index operations across multiple workspaces don't exceed the maximum /// number of processor cores that the user allocated to background indexing. - private let indexTaskScheduler: TaskScheduler + private let indexTaskScheduler: TaskScheduler private var packageLoadingWorkDoneProgress = WorkDoneProgressState( "SourceKitLSP.SourceKitLSPServer.reloadPackage", diff --git a/Sources/SourceKitLSP/Workspace.swift b/Sources/SourceKitLSP/Workspace.swift index 04c63b9b2..050671edc 100644 --- a/Sources/SourceKitLSP/Workspace.swift +++ b/Sources/SourceKitLSP/Workspace.swift @@ -89,7 +89,7 @@ public final class Workspace: Sendable { underlyingBuildSystem: BuildSystem?, index uncheckedIndex: UncheckedIndex?, indexDelegate: SourceKitIndexDelegate?, - indexTaskScheduler: TaskScheduler + indexTaskScheduler: TaskScheduler ) async { self.documentManager = documentManager self.buildSetup = options.buildSetup @@ -142,7 +142,7 @@ public final class Workspace: Sendable { options: SourceKitLSPServer.Options, compilationDatabaseSearchPaths: [RelativePath], indexOptions: IndexOptions = IndexOptions(), - indexTaskScheduler: TaskScheduler, + indexTaskScheduler: TaskScheduler, reloadPackageStatusCallback: @Sendable @escaping (ReloadPackageStatus) async -> Void ) async throws { var buildSystem: BuildSystem? = nil @@ -306,7 +306,7 @@ public struct IndexOptions: Sendable { /// A callback that is called when an index task finishes. /// /// Intended for testing purposes. - public var indexTaskDidFinish: (@Sendable (IndexTaskDescription) -> Void)? + public var indexTaskDidFinish: (@Sendable (AnyIndexTaskDescription) -> Void)? public init( indexStorePath: AbsolutePath? = nil, @@ -315,7 +315,7 @@ public struct IndexOptions: Sendable { listenToUnitEvents: Bool = true, enableBackgroundIndexing: Bool = false, maxCoresPercentageToUseForBackgroundIndexing: Double = 1, - indexTaskDidFinish: (@Sendable (IndexTaskDescription) -> Void)? = nil + indexTaskDidFinish: (@Sendable (AnyIndexTaskDescription) -> Void)? = nil ) { self.indexStorePath = indexStorePath self.indexDatabasePath = indexDatabasePath From 0dc5cdd7c79e44e71fc0373bc188bec1630c7c85 Mon Sep 17 00:00:00 2001 From: Alex Hoppen Date: Fri, 10 May 2024 06:47:51 -0700 Subject: [PATCH 2/7] Call `indexTaskDidFinish` from `SemanticIndexManager` This fixes a bug where `indexTaskDidFinish` would also get called when a task is cancelled to be rescheduled. --- .../PreparationTaskDescription.swift | 12 +----- .../SemanticIndex/SemanticIndexManager.swift | 38 ++++++++----------- .../UpdateIndexStoreTaskDescription.swift | 10 +---- 3 files changed, 17 insertions(+), 43 deletions(-) diff --git a/Sources/SemanticIndex/PreparationTaskDescription.swift b/Sources/SemanticIndex/PreparationTaskDescription.swift index b48e6b93e..7765c63e3 100644 --- a/Sources/SemanticIndex/PreparationTaskDescription.swift +++ b/Sources/SemanticIndex/PreparationTaskDescription.swift @@ -35,11 +35,6 @@ public struct PreparationTaskDescription: IndexTaskDescription { /// The build system manager that is used to get the toolchain and build settings for the files to index. private let buildSystemManager: BuildSystemManager - /// A callback that is called when the task finishes. - /// - /// Intended for testing purposes. - private let didFinishCallback: @Sendable (PreparationTaskDescription) -> Void - /// The task is idempotent because preparing the same target twice produces the same result as preparing it once. public var isIdempotent: Bool { true } @@ -55,18 +50,13 @@ public struct PreparationTaskDescription: IndexTaskDescription { init( targetsToPrepare: [ConfiguredTarget], - buildSystemManager: BuildSystemManager, - didFinishCallback: @escaping @Sendable (PreparationTaskDescription) -> Void + buildSystemManager: BuildSystemManager ) { self.targetsToPrepare = targetsToPrepare self.buildSystemManager = buildSystemManager - self.didFinishCallback = didFinishCallback } public func execute() async { - defer { - didFinishCallback(self) - } // Only use the last two digits of the preparation ID for the logging scope to avoid creating too many scopes. // See comment in `withLoggingScope`. // The last 2 digits should be sufficient to differentiate between multiple concurrently running preparation operations diff --git a/Sources/SemanticIndex/SemanticIndexManager.swift b/Sources/SemanticIndex/SemanticIndexManager.swift index e1aac5862..9f615cbf3 100644 --- a/Sources/SemanticIndex/SemanticIndexManager.swift +++ b/Sources/SemanticIndex/SemanticIndexManager.swift @@ -131,38 +131,30 @@ public final actor SemanticIndexManager { /// Prepare the given targets for indexing private func prepare(targets: [ConfiguredTarget], priority: TaskPriority?) async { - await self.indexTaskScheduler.schedule( - priority: priority, - AnyIndexTaskDescription( - PreparationTaskDescription( - targetsToPrepare: targets, - buildSystemManager: self.buildSystemManager, - didFinishCallback: { [weak self] taskDescription in - self?.indexTaskDidFinish?(AnyIndexTaskDescription(taskDescription)) - } - ) + let taskDescription = AnyIndexTaskDescription( + PreparationTaskDescription( + targetsToPrepare: targets, + buildSystemManager: self.buildSystemManager ) - ).value + ) + await self.indexTaskScheduler.schedule(priority: priority, taskDescription).value + self.indexTaskDidFinish?(taskDescription) } /// Update the index store for the given files, assuming that their targets have already been prepared. private func updateIndexStore(for files: [DocumentURI], priority: TaskPriority?) async { - await self.indexTaskScheduler.schedule( - priority: priority, - AnyIndexTaskDescription( - UpdateIndexStoreTaskDescription( - filesToIndex: Set(files), - buildSystemManager: self.buildSystemManager, - index: self.index.unchecked, - didFinishCallback: { [weak self] taskDescription in - self?.indexTaskDidFinish?(AnyIndexTaskDescription(taskDescription)) - } - ) + let taskDescription = AnyIndexTaskDescription( + UpdateIndexStoreTaskDescription( + filesToIndex: Set(files), + buildSystemManager: self.buildSystemManager, + index: self.index.unchecked ) - ).value + ) + await self.indexTaskScheduler.schedule(priority: priority, taskDescription).value for file in files { self.indexStatus[file] = .upToDate } + self.indexTaskDidFinish?(taskDescription) } /// Index the given set of files at the given priority. diff --git a/Sources/SemanticIndex/UpdateIndexStoreTaskDescription.swift b/Sources/SemanticIndex/UpdateIndexStoreTaskDescription.swift index 6b9968a14..2de7ea4c0 100644 --- a/Sources/SemanticIndex/UpdateIndexStoreTaskDescription.swift +++ b/Sources/SemanticIndex/UpdateIndexStoreTaskDescription.swift @@ -39,9 +39,6 @@ public struct UpdateIndexStoreTaskDescription: IndexTaskDescription { /// case we don't need to index it again. private let index: UncheckedIndex - /// A callback that is called when the index task finishes - private let didFinishCallback: @Sendable (UpdateIndexStoreTaskDescription) -> Void - /// The task is idempotent because indexing the same file twice produces the same result as indexing it once. public var isIdempotent: Bool { true } @@ -58,19 +55,14 @@ public struct UpdateIndexStoreTaskDescription: IndexTaskDescription { init( filesToIndex: Set, buildSystemManager: BuildSystemManager, - index: UncheckedIndex, - didFinishCallback: @escaping @Sendable (UpdateIndexStoreTaskDescription) -> Void + index: UncheckedIndex ) { self.filesToIndex = filesToIndex self.buildSystemManager = buildSystemManager self.index = index - self.didFinishCallback = didFinishCallback } public func execute() async { - defer { - didFinishCallback(self) - } // Only use the last two digits of the indexing ID for the logging scope to avoid creating too many scopes. // See comment in `withLoggingScope`. // The last 2 digits should be sufficient to differentiate between multiple concurrently running indexing operation. From c399b438584f520455dda4d90e946eebc240998c Mon Sep 17 00:00:00 2001 From: Alex Hoppen Date: Fri, 10 May 2024 06:50:40 -0700 Subject: [PATCH 3/7] =?UTF-8?q?Don=E2=80=99t=20pass=20the=20finished=20`ta?= =?UTF-8?q?skDescription`=20to=20`indexTaskDidFinish`=20callback?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This follows the general paradigm that callbacks shouldn’t carry much state and instead only notify an observer that state has changed, which the observer can then poll. --- Sources/SemanticIndex/SemanticIndexManager.swift | 8 ++++---- Sources/SourceKitLSP/Workspace.swift | 4 ++-- Tests/SourceKitLSPTests/BackgroundIndexingTests.swift | 4 ++-- 3 files changed, 8 insertions(+), 8 deletions(-) diff --git a/Sources/SemanticIndex/SemanticIndexManager.swift b/Sources/SemanticIndex/SemanticIndexManager.swift index 9f615cbf3..250a0ec0f 100644 --- a/Sources/SemanticIndex/SemanticIndexManager.swift +++ b/Sources/SemanticIndex/SemanticIndexManager.swift @@ -49,7 +49,7 @@ public final actor SemanticIndexManager { /// Callback that is called when an index task has finished. /// /// Currently only used for testing. - private let indexTaskDidFinish: (@Sendable (AnyIndexTaskDescription) -> Void)? + private let indexTaskDidFinish: (@Sendable () -> Void)? // MARK: - Public API @@ -57,7 +57,7 @@ public final actor SemanticIndexManager { index: UncheckedIndex, buildSystemManager: BuildSystemManager, indexTaskScheduler: TaskScheduler, - indexTaskDidFinish: (@Sendable (AnyIndexTaskDescription) -> Void)? + indexTaskDidFinish: (@Sendable () -> Void)? ) { self.index = index.checked(for: .modifiedFiles) self.buildSystemManager = buildSystemManager @@ -138,7 +138,7 @@ public final actor SemanticIndexManager { ) ) await self.indexTaskScheduler.schedule(priority: priority, taskDescription).value - self.indexTaskDidFinish?(taskDescription) + self.indexTaskDidFinish?() } /// Update the index store for the given files, assuming that their targets have already been prepared. @@ -154,7 +154,7 @@ public final actor SemanticIndexManager { for file in files { self.indexStatus[file] = .upToDate } - self.indexTaskDidFinish?(taskDescription) + self.indexTaskDidFinish?() } /// Index the given set of files at the given priority. diff --git a/Sources/SourceKitLSP/Workspace.swift b/Sources/SourceKitLSP/Workspace.swift index 050671edc..c89cdd53b 100644 --- a/Sources/SourceKitLSP/Workspace.swift +++ b/Sources/SourceKitLSP/Workspace.swift @@ -306,7 +306,7 @@ public struct IndexOptions: Sendable { /// A callback that is called when an index task finishes. /// /// Intended for testing purposes. - public var indexTaskDidFinish: (@Sendable (AnyIndexTaskDescription) -> Void)? + public var indexTaskDidFinish: (@Sendable () -> Void)? public init( indexStorePath: AbsolutePath? = nil, @@ -315,7 +315,7 @@ public struct IndexOptions: Sendable { listenToUnitEvents: Bool = true, enableBackgroundIndexing: Bool = false, maxCoresPercentageToUseForBackgroundIndexing: Double = 1, - indexTaskDidFinish: (@Sendable (AnyIndexTaskDescription) -> Void)? = nil + indexTaskDidFinish: (@Sendable () -> Void)? = nil ) { self.indexStorePath = indexStorePath self.indexDatabasePath = indexDatabasePath diff --git a/Tests/SourceKitLSPTests/BackgroundIndexingTests.swift b/Tests/SourceKitLSPTests/BackgroundIndexingTests.swift index a2c4c3539..7ec0dd82a 100644 --- a/Tests/SourceKitLSPTests/BackgroundIndexingTests.swift +++ b/Tests/SourceKitLSPTests/BackgroundIndexingTests.swift @@ -165,10 +165,10 @@ final class BackgroundIndexingTests: XCTestCase { func testBackgroundIndexingHappensWithLowPriority() async throws { var serverOptions = backgroundIndexingOptions - serverOptions.indexOptions.indexTaskDidFinish = { taskDescription in + serverOptions.indexOptions.indexTaskDidFinish = { XCTAssert( Task.currentPriority == .low, - "\(taskDescription.description) ran with priority \(Task.currentPriority)" + "An index task ran with priority \(Task.currentPriority)" ) } let project = try await SwiftPMTestProject( From d70a68f37cc08c9d4f74c5d78e4e68fa4463eff8 Mon Sep 17 00:00:00 2001 From: Alex Hoppen Date: Fri, 10 May 2024 11:44:17 -0700 Subject: [PATCH 4/7] Wait until initialization has finished before starting a work done progress --- Sources/SourceKitLSP/SourceKitLSPServer.swift | 54 +++++++++++++++---- .../WorkDoneProgressManager.swift | 21 +++++++- 2 files changed, 62 insertions(+), 13 deletions(-) diff --git a/Sources/SourceKitLSP/SourceKitLSPServer.swift b/Sources/SourceKitLSP/SourceKitLSPServer.swift index 484310375..14553c26a 100644 --- a/Sources/SourceKitLSP/SourceKitLSPServer.swift +++ b/Sources/SourceKitLSP/SourceKitLSPServer.swift @@ -115,6 +115,10 @@ final actor WorkDoneProgressState { case progressCreationFailed } + /// A queue so we can have synchronous `startProgress` and `endProgress` functions that don't need to wait for the + /// work done progress to be started or ended. + private let queue = AsyncQueue() + /// How many active tasks are running. /// /// A work done progress should be displayed if activeTasks > 0 @@ -135,13 +139,16 @@ final actor WorkDoneProgressState { /// Start a new task, creating a new `WorkDoneProgress` if none is running right now. /// /// - Parameter server: The server that is used to create the `WorkDoneProgress` on the client - func startProgress(server: SourceKitLSPServer) async { + nonisolated func startProgress(server: SourceKitLSPServer) { + queue.async { + await self.startProgressImpl(server: server) + } + } + + func startProgressImpl(server: SourceKitLSPServer) async { + await server.waitUntilInitialized() activeTasks += 1 guard await server.capabilityRegistry?.clientCapabilities.window?.workDoneProgress ?? false else { - // If the client doesn't support workDoneProgress, keep track of the active task count but don't update the state. - // That way, if we call `startProgress` before initialization finishes, we won't send the - // `CreateWorkDoneProgressRequest` but if we call `startProgress` again after initialization finished (and thus - // the capability is set), we will create the work done progress. return } if state == .noProgress { @@ -180,7 +187,13 @@ final actor WorkDoneProgressState { /// If this drops the active task count to 0, the work done progress is ended on the client. /// /// - Parameter server: The server that is used to send and update of the `WorkDoneProgress` to the client - func endProgress(server: SourceKitLSPServer) async { + nonisolated func endProgress(server: SourceKitLSPServer) { + queue.async { + await self.endProgressImpl(server: server) + } + } + + func endProgressImpl(server: SourceKitLSPServer) async { assert(activeTasks > 0, "Unbalanced startProgress/endProgress calls") activeTasks -= 1 guard await server.capabilityRegistry?.clientCapabilities.window?.workDoneProgress ?? false else { @@ -440,11 +453,16 @@ public actor SourceKitLSPServer { /// The connection to the editor. public let client: Connection + /// Set to `true` after the `SourceKitLSPServer` has send the reply to the `InitializeRequest`. + /// + /// Initialization can be awaited using `waitUntilInitialized`. + private var initialized: Bool = false + var options: Options let toolchainRegistry: ToolchainRegistry - var capabilityRegistry: CapabilityRegistry? + public var capabilityRegistry: CapabilityRegistry? var languageServices: [LanguageServerType: [LanguageService]] = [:] @@ -508,19 +526,15 @@ public actor SourceKitLSPServer { self.inProgressRequests[id] = task } - let fs: FileSystem - var onExit: () -> Void /// Creates a language server for the given client. public init( client: Connection, - fileSystem: FileSystem = localFileSystem, toolchainRegistry: ToolchainRegistry, options: Options, onExit: @escaping () -> Void = {} ) { - self.fs = fileSystem self.toolchainRegistry = toolchainRegistry self.options = options self.onExit = onExit @@ -534,6 +548,22 @@ public actor SourceKitLSPServer { ]) } + /// Await until the server has send the reply to the initialize request. + func waitUntilInitialized() async { + // The polling of `initialized` is not perfect but it should be OK, because + // - In almost all cases the server should already be initialized. + // - If it's not initialized, we expect initialization to finish fairly quickly. Even if initialization takes 5s + // this only results in 50 polls, which is acceptable. + // Alternative solutions that signal via an async sequence seem overkill here. + while !initialized { + do { + try await Task.sleep(for: .seconds(0.1)) + } catch { + break + } + } + } + /// Search through all the parent directories of `uri` and check if any of these directories contain a workspace /// capable of handling `uri`. /// @@ -958,6 +988,8 @@ extension SourceKitLSPServer: MessageHandler { switch request { case let request as RequestAndReply: await request.reply { try await initialize(request.params) } + // Only set `initialized` to `true` after we have sent the response to the initialize request to the client. + initialized = true case let request as RequestAndReply: await request.reply { try await shutdown(request.params) } case let request as RequestAndReply: diff --git a/Sources/SourceKitLSP/WorkDoneProgressManager.swift b/Sources/SourceKitLSP/WorkDoneProgressManager.swift index 089e76f90..f4f59d496 100644 --- a/Sources/SourceKitLSP/WorkDoneProgressManager.swift +++ b/Sources/SourceKitLSP/WorkDoneProgressManager.swift @@ -23,20 +23,37 @@ final class WorkDoneProgressManager { private let queue = AsyncQueue() private let server: SourceKitLSPServer - init?(server: SourceKitLSPServer, capabilityRegistry: CapabilityRegistry, title: String, message: String? = nil) { + convenience init?(server: SourceKitLSPServer, title: String, message: String? = nil, percentage: Int? = nil) async { + guard let capabilityRegistry = await server.capabilityRegistry else { + return nil + } + self.init(server: server, capabilityRegistry: capabilityRegistry, title: title, message: message) + } + + init?( + server: SourceKitLSPServer, + capabilityRegistry: CapabilityRegistry, + title: String, + message: String? = nil, + percentage: Int? = nil + ) { guard capabilityRegistry.clientCapabilities.window?.workDoneProgress ?? false else { return nil } self.token = .string("WorkDoneProgress-\(UUID())") self.server = server queue.async { [server, token] in + await server.waitUntilInitialized() await withCheckedContinuation { (continuation: CheckedContinuation) in _ = server.client.send(CreateWorkDoneProgressRequest(token: token)) { result in continuation.resume() } } await server.sendNotificationToClient( - WorkDoneProgress(token: token, value: .begin(WorkDoneProgressBegin(title: title, message: message))) + WorkDoneProgress( + token: token, + value: .begin(WorkDoneProgressBegin(title: title, message: message, percentage: percentage)) + ) ) } } From eaa378f3903d3d88447b57b82a5adce7396164b9 Mon Sep 17 00:00:00 2001 From: Alex Hoppen Date: Fri, 10 May 2024 11:45:16 -0700 Subject: [PATCH 5/7] Plumb `capabilities` to `SwiftPMTestProject` and add a closure that can be executed before SourceKit-LSP is initialized --- Sources/SKTestSupport/MultiFileTestProject.swift | 4 ++++ Sources/SKTestSupport/SwiftPMTestProject.swift | 4 ++++ Sources/SKTestSupport/TestSourceKitLSPClient.swift | 6 +++++- 3 files changed, 13 insertions(+), 1 deletion(-) diff --git a/Sources/SKTestSupport/MultiFileTestProject.swift b/Sources/SKTestSupport/MultiFileTestProject.swift index 68baba274..ea300f497 100644 --- a/Sources/SKTestSupport/MultiFileTestProject.swift +++ b/Sources/SKTestSupport/MultiFileTestProject.swift @@ -80,8 +80,10 @@ public class MultiFileTestProject { public init( files: [RelativeFileLocation: String], workspaces: (URL) async throws -> [WorkspaceFolder] = { [WorkspaceFolder(uri: DocumentURI($0))] }, + capabilities: ClientCapabilities = ClientCapabilities(), serverOptions: SourceKitLSPServer.Options = .testDefault, usePullDiagnostics: Bool = true, + preInitialization: ((TestSourceKitLSPClient) -> Void)? = nil, testName: String = #function ) async throws { scratchDirectory = try testScratchDir(testName: testName) @@ -112,8 +114,10 @@ public class MultiFileTestProject { self.testClient = try await TestSourceKitLSPClient( serverOptions: serverOptions, + capabilities: capabilities, usePullDiagnostics: usePullDiagnostics, workspaceFolders: workspaces(scratchDirectory), + preInitialization: preInitialization, cleanUp: { [scratchDirectory] in if cleanScratchDirectories { try? FileManager.default.removeItem(at: scratchDirectory) diff --git a/Sources/SKTestSupport/SwiftPMTestProject.swift b/Sources/SKTestSupport/SwiftPMTestProject.swift index 72334c0d6..abd4a20d2 100644 --- a/Sources/SKTestSupport/SwiftPMTestProject.swift +++ b/Sources/SKTestSupport/SwiftPMTestProject.swift @@ -42,8 +42,10 @@ public class SwiftPMTestProject: MultiFileTestProject { workspaces: (URL) async throws -> [WorkspaceFolder] = { [WorkspaceFolder(uri: DocumentURI($0))] }, build: Bool = false, allowBuildFailure: Bool = false, + capabilities: ClientCapabilities = ClientCapabilities(), serverOptions: SourceKitLSPServer.Options = .testDefault, pollIndex: Bool = true, + preInitialization: ((TestSourceKitLSPClient) -> Void)? = nil, usePullDiagnostics: Bool = true, testName: String = #function ) async throws { @@ -66,8 +68,10 @@ public class SwiftPMTestProject: MultiFileTestProject { try await super.init( files: filesByPath, workspaces: workspaces, + capabilities: capabilities, serverOptions: serverOptions, usePullDiagnostics: usePullDiagnostics, + preInitialization: preInitialization, testName: testName ) diff --git a/Sources/SKTestSupport/TestSourceKitLSPClient.swift b/Sources/SKTestSupport/TestSourceKitLSPClient.swift index 1ab9a7541..833b0f02a 100644 --- a/Sources/SKTestSupport/TestSourceKitLSPClient.swift +++ b/Sources/SKTestSupport/TestSourceKitLSPClient.swift @@ -83,6 +83,8 @@ public final class TestSourceKitLSPClient: MessageHandler { /// - capabilities: The test client's capabilities. /// - usePullDiagnostics: Whether to use push diagnostics or use push-based diagnostics /// - workspaceFolders: Workspace folders to open. + /// - preInitialization: A closure that is called after the test client is created but before SourceKit-LSP is + /// initialized. This can be used to eg. register request handlers. /// - cleanUp: A closure that is called when the `TestSourceKitLSPClient` is destructed. /// This allows e.g. a `IndexedSingleSwiftFileTestProject` to delete its temporary files when they are no longer /// needed. @@ -94,6 +96,7 @@ public final class TestSourceKitLSPClient: MessageHandler { capabilities: ClientCapabilities = ClientCapabilities(), usePullDiagnostics: Bool = true, workspaceFolders: [WorkspaceFolder]? = nil, + preInitialization: ((TestSourceKitLSPClient) -> Void)? = nil, cleanUp: @escaping () -> Void = {} ) async throws { if !useGlobalModuleCache { @@ -135,7 +138,7 @@ public final class TestSourceKitLSPClient: MessageHandler { guard capabilities.textDocument!.diagnostic == nil else { struct ConflictingDiagnosticsError: Error, CustomStringConvertible { var description: String { - "usePushDiagnostics = false is not supported if capabilities already contain diagnostic options" + "usePullDiagnostics = false is not supported if capabilities already contain diagnostic options" } } throw ConflictingDiagnosticsError() @@ -145,6 +148,7 @@ public final class TestSourceKitLSPClient: MessageHandler { XCTAssertEqual(request.registrations.only?.method, DocumentDiagnosticsRequest.method) return VoidResponse() } + preInitialization?(self) } if initialize { _ = try await self.send( From c9b51f9b345fe8d7fcaa844df7f214628c10e9cc Mon Sep 17 00:00:00 2001 From: Alex Hoppen Date: Fri, 10 May 2024 11:45:44 -0700 Subject: [PATCH 6/7] Allow client request handlers specified on `TestSourceKitLSPClient` to be executed in any order --- .../SKTestSupport/TestSourceKitLSPClient.swift | 17 +++++++++-------- .../UpdateIndexStoreTaskDescription.swift | 2 +- 2 files changed, 10 insertions(+), 9 deletions(-) diff --git a/Sources/SKTestSupport/TestSourceKitLSPClient.swift b/Sources/SKTestSupport/TestSourceKitLSPClient.swift index 833b0f02a..f3854c039 100644 --- a/Sources/SKTestSupport/TestSourceKitLSPClient.swift +++ b/Sources/SKTestSupport/TestSourceKitLSPClient.swift @@ -290,18 +290,19 @@ public final class TestSourceKitLSPClient: MessageHandler { id: LanguageServerProtocol.RequestID, reply: @escaping (LSPResult) -> Void ) { - guard let requestHandler = requestHandlers.first else { - reply(.failure(.methodNotFound(Request.method))) - return - } - guard let requestHandler = requestHandler as? RequestHandler else { - print("\(RequestHandler.self)") - XCTFail("Received request of unexpected type \(Request.method)") + let requestHandlerAndIndex = requestHandlers.enumerated().compactMap { + (index, handler) -> (RequestHandler, Int)? in + guard let handler = handler as? RequestHandler else { + return nil + } + return (handler, index) + }.first + guard let (requestHandler, index) = requestHandlerAndIndex else { reply(.failure(.methodNotFound(Request.method))) return } reply(.success(requestHandler(params))) - requestHandlers.removeFirst() + requestHandlers.remove(at: index) } // MARK: - Convenience functions diff --git a/Sources/SemanticIndex/UpdateIndexStoreTaskDescription.swift b/Sources/SemanticIndex/UpdateIndexStoreTaskDescription.swift index 2de7ea4c0..cd8b48ea6 100644 --- a/Sources/SemanticIndex/UpdateIndexStoreTaskDescription.swift +++ b/Sources/SemanticIndex/UpdateIndexStoreTaskDescription.swift @@ -49,7 +49,7 @@ public struct UpdateIndexStoreTaskDescription: IndexTaskDescription { } public var redactedDescription: String { - return "indexing-\(id)" + return "update-indexstore-\(id)" } init( From a6389e58e2df161c2ef064d2e15f2ee3005d533c Mon Sep 17 00:00:00 2001 From: Alex Hoppen Date: Mon, 13 May 2024 21:59:49 -0700 Subject: [PATCH 7/7] Show a work done progress that shows the index progress This makes it a lot easier to work on background indexing because you can easily see how background indexing is making progress. Resolves #1257 rdar://127474057 --- Sources/SKCore/TaskScheduler.swift | 38 +++++- .../SemanticIndex/SemanticIndexManager.swift | 108 +++++++++++++++--- Sources/SourceKitLSP/CMakeLists.txt | 1 + .../SourceKitLSP/IndexProgressManager.swift | 103 +++++++++++++++++ .../SourceKitLSPServer+Options.swift | 5 + Sources/SourceKitLSP/SourceKitLSPServer.swift | 30 ++++- Sources/SourceKitLSP/Workspace.swift | 24 ++-- .../BackgroundIndexingTests.swift | 47 +++++++- .../SourceKitLSPTests/BuildSystemTests.swift | 4 +- 9 files changed, 324 insertions(+), 36 deletions(-) create mode 100644 Sources/SourceKitLSP/IndexProgressManager.swift diff --git a/Sources/SKCore/TaskScheduler.swift b/Sources/SKCore/TaskScheduler.swift index deb3a3399..58cf052fb 100644 --- a/Sources/SKCore/TaskScheduler.swift +++ b/Sources/SKCore/TaskScheduler.swift @@ -76,6 +76,19 @@ public protocol TaskDescriptionProtocol: Identifiable, Sendable, CustomLogString var estimatedCPUCoreCount: Int { get } } +/// Parameter that's passed to `executionStateChangedCallback` to indicate the new state of a scheduled task. +public enum TaskExecutionState { + /// The task started executing. + case executing + + /// The task was cancelled and will be re-scheduled for execution later. Will be followed by another call with + /// `executing`. + case cancelledToBeRescheduled + + /// The task has finished executing. Now more state updates will come after this one. + case finished +} + fileprivate actor QueuedTask { /// Result of `executionTask` / the tasks in `executionTaskCreatedContinuation`. /// See doc comment on `executionTask`. @@ -136,9 +149,18 @@ fileprivate actor QueuedTask { /// Gets reset every time `executionTask` finishes. nonisolated(unsafe) private var cancelledToBeRescheduled: AtomicBool = .init(initialValue: false) - init(priority: TaskPriority? = nil, description: TaskDescription) async { + /// A callback that will be called when the task starts executing, is cancelled to be rescheduled, or when it finishes + /// execution. + private let executionStateChangedCallback: (@Sendable (TaskExecutionState) async -> Void)? + + init( + priority: TaskPriority? = nil, + description: TaskDescription, + executionStateChangedCallback: (@Sendable (TaskExecutionState) async -> Void)? + ) async { self._priority = .init(initialValue: priority?.rawValue ?? Task.currentPriority.rawValue) self.description = description + self.executionStateChangedCallback = executionStateChangedCallback var updatePriorityContinuation: AsyncStream.Continuation! let updatePriorityStream = AsyncStream { @@ -194,16 +216,19 @@ fileprivate actor QueuedTask { } executionTask = task executionTaskCreatedContinuation.yield(task) + await executionStateChangedCallback?(.executing) return await task.value } /// Implementation detail of `execute` that is called after `self.description.execute()` finishes. - private func finalizeExecution() -> ExecutionTaskFinishStatus { + private func finalizeExecution() async -> ExecutionTaskFinishStatus { self.executionTask = nil if Task.isCancelled && self.cancelledToBeRescheduled.value { + await executionStateChangedCallback?(.cancelledToBeRescheduled) self.cancelledToBeRescheduled.value = false return ExecutionTaskFinishStatus.cancelledToBeRescheduled } else { + await executionStateChangedCallback?(.finished) return ExecutionTaskFinishStatus.terminated } } @@ -308,12 +333,17 @@ public actor TaskScheduler { @discardableResult public func schedule( priority: TaskPriority? = nil, - _ taskDescription: TaskDescription + _ taskDescription: TaskDescription, + @_inheritActorContext executionStateChangedCallback: (@Sendable (TaskExecutionState) async -> Void)? = nil ) async -> Task { withLoggingSubsystemAndScope(subsystem: taskSchedulerSubsystem, scope: nil) { logger.debug("Scheduling \(taskDescription.forLogging)") } - let queuedTask = await QueuedTask(priority: priority, description: taskDescription) + let queuedTask = await QueuedTask( + priority: priority, + description: taskDescription, + executionStateChangedCallback: executionStateChangedCallback + ) pendingTasks.append(queuedTask) Task.detached(priority: priority ?? Task.currentPriority) { // Poke the `TaskScheduler` to execute a new task. If the `TaskScheduler` is already working at its capacity diff --git a/Sources/SemanticIndex/SemanticIndexManager.swift b/Sources/SemanticIndex/SemanticIndexManager.swift index 250a0ec0f..c60a12a3e 100644 --- a/Sources/SemanticIndex/SemanticIndexManager.swift +++ b/Sources/SemanticIndex/SemanticIndexManager.swift @@ -19,8 +19,22 @@ import SKCore private enum FileIndexStatus { /// The index is up-to-date. case upToDate - /// The file is being indexed by the given task. - case inProgress(Task) + /// The file is not up to date and we have scheduled a task to index it but that index operation hasn't been started + /// yet. + case scheduled(Task) + /// We are currently actively indexing this file, ie. we are running a subprocess that indexes the file. + case executing(Task) + + var description: String { + switch self { + case .upToDate: + return "upToDate" + case .scheduled: + return "scheduled" + case .executing: + return "executing" + } + } } /// Schedules index tasks and keeps track of the index status of files. @@ -46,22 +60,51 @@ public final actor SemanticIndexManager { /// workspaces. private let indexTaskScheduler: TaskScheduler + /// Called when files are scheduled to be indexed. + /// + /// The parameter is the number of files that were scheduled to be indexed. + private let indexTasksWereScheduled: @Sendable (_ numberOfFileScheduled: Int) -> Void + /// Callback that is called when an index task has finished. /// - /// Currently only used for testing. - private let indexTaskDidFinish: (@Sendable () -> Void)? + /// An object observing this property probably wants to check `inProgressIndexTasks` when the callback is called to + /// get the current list of in-progress index tasks. + /// + /// The number of `indexTaskDidFinish` calls does not have to relate to the number of `indexTasksWereScheduled` calls. + private let indexTaskDidFinish: @Sendable () -> Void // MARK: - Public API + /// The files that still need to be indexed. + /// + /// See `FileIndexStatus` for the distinction between `scheduled` and `executing`. + public var inProgressIndexTasks: (scheduled: [DocumentURI], executing: [DocumentURI]) { + let scheduled = indexStatus.compactMap { (uri: DocumentURI, status: FileIndexStatus) in + if case .scheduled = status { + return uri + } + return nil + } + let inProgress = indexStatus.compactMap { (uri: DocumentURI, status: FileIndexStatus) in + if case .executing = status { + return uri + } + return nil + } + return (scheduled, inProgress) + } + public init( index: UncheckedIndex, buildSystemManager: BuildSystemManager, indexTaskScheduler: TaskScheduler, - indexTaskDidFinish: (@Sendable () -> Void)? + indexTasksWereScheduled: @escaping @Sendable (Int) -> Void, + indexTaskDidFinish: @escaping @Sendable () -> Void ) { self.index = index.checked(for: .modifiedFiles) self.buildSystemManager = buildSystemManager self.indexTaskScheduler = indexTaskScheduler + self.indexTasksWereScheduled = indexTasksWereScheduled self.indexTaskDidFinish = indexTaskDidFinish } @@ -93,7 +136,7 @@ public final actor SemanticIndexManager { await withTaskGroup(of: Void.self) { taskGroup in for (_, status) in indexStatus { switch status { - case .inProgress(let task): + case .scheduled(let task), .executing(let task): taskGroup.addTask { await task.value } @@ -138,7 +181,7 @@ public final actor SemanticIndexManager { ) ) await self.indexTaskScheduler.schedule(priority: priority, taskDescription).value - self.indexTaskDidFinish?() + self.indexTaskDidFinish() } /// Update the index store for the given files, assuming that their targets have already been prepared. @@ -150,11 +193,44 @@ public final actor SemanticIndexManager { index: self.index.unchecked ) ) - await self.indexTaskScheduler.schedule(priority: priority, taskDescription).value - for file in files { - self.indexStatus[file] = .upToDate + let updateIndexStoreTask = await self.indexTaskScheduler.schedule(priority: priority, taskDescription) { newState in + switch newState { + case .executing: + for file in files { + if case .scheduled(let task) = self.indexStatus[file] { + self.indexStatus[file] = .executing(task) + } else { + logger.fault( + """ + Index status of \(file) is in an unexpected state \ + '\(self.indexStatus[file]?.description ?? "", privacy: .public)' when update index store task \ + started executing + """ + ) + } + } + case .cancelledToBeRescheduled: + for file in files { + if case .executing(let task) = self.indexStatus[file] { + self.indexStatus[file] = .scheduled(task) + } else { + logger.fault( + """ + Index status of \(file) is in an unexpected state \ + '\(self.indexStatus[file]?.description ?? "", privacy: .public)' when update index store task \ + is cancelled to be rescheduled. + """ + ) + } + } + case .finished: + for file in files { + self.indexStatus[file] = .upToDate + } + self.indexTaskDidFinish() + } } - self.indexTaskDidFinish?() + await updateIndexStoreTask.value } /// Index the given set of files at the given priority. @@ -226,9 +302,15 @@ public final actor SemanticIndexManager { } indexTasks.append(indexTask) - for file in targetsBatch.flatMap({ filesByTarget[$0]! }) { - indexStatus[file] = .inProgress(indexTask) + let filesToIndex = targetsBatch.flatMap({ filesByTarget[$0]! }) + for file in filesToIndex { + // indexStatus will get set to `.upToDate` by `updateIndexStore`. Setting it to `.upToDate` cannot race with + // setting it to `.scheduled` because we don't have an `await` call between the creation of `indexTask` and + // this loop, so we still have exclusive access to the `SemanticIndexManager` actor and hence `updateIndexStore` + // can't execute until we have set all index statuses to `.scheduled`. + indexStatus[file] = .scheduled(indexTask) } + indexTasksWereScheduled(filesToIndex.count) } let indexTasksImmutable = indexTasks diff --git a/Sources/SourceKitLSP/CMakeLists.txt b/Sources/SourceKitLSP/CMakeLists.txt index a86c621f9..45684cec9 100644 --- a/Sources/SourceKitLSP/CMakeLists.txt +++ b/Sources/SourceKitLSP/CMakeLists.txt @@ -3,6 +3,7 @@ add_library(SourceKitLSP STATIC CapabilityRegistry.swift DocumentManager.swift DocumentSnapshot+FromFileContents.swift + IndexProgressManager.swift IndexStoreDB+MainFilesProvider.swift LanguageService.swift Rename.swift diff --git a/Sources/SourceKitLSP/IndexProgressManager.swift b/Sources/SourceKitLSP/IndexProgressManager.swift new file mode 100644 index 000000000..fdc745fa3 --- /dev/null +++ b/Sources/SourceKitLSP/IndexProgressManager.swift @@ -0,0 +1,103 @@ +//===----------------------------------------------------------------------===// +// +// This source file is part of the Swift.org open source project +// +// Copyright (c) 2014 - 2024 Apple Inc. and the Swift project authors +// Licensed under Apache License v2.0 with Runtime Library Exception +// +// See https://swift.org/LICENSE.txt for license information +// See https://swift.org/CONTRIBUTORS.txt for the list of Swift project authors +// +//===----------------------------------------------------------------------===// + +import LanguageServerProtocol +import SKSupport +import SemanticIndex + +/// Listens for index status updates from `SemanticIndexManagers`. From that information, it manages a +/// `WorkDoneProgress` that communicates the index progress to the editor. +actor IndexProgressManager { + /// A queue on which `indexTaskWasQueued` and `indexStatusDidChange` are handled. + /// + /// This allows the two functions two be `nonisolated` (and eg. the caller of `indexStatusDidChange` doesn't have to + /// wait for the work done progress to be updated) while still guaranteeing that we handle them in the order they + /// were called. + private let queue = AsyncQueue() + + /// The `SourceKitLSPServer` for which this manages the index progress. It gathers all `SemanticIndexManagers` from + /// the workspaces in the `SourceKitLSPServer`. + private weak var sourceKitLSPServer: SourceKitLSPServer? + + /// This is the target number of index tasks (eg. the `3` in `1/3 done`). + /// + /// Every time a new index task is scheduled, this number gets incremented, so that it only ever increases. + /// When indexing of one session is done (ie. when there are no more `scheduled` or `executing` tasks in any + /// `SemanticIndexManager`), `queuedIndexTasks` gets reset to 0 and the work done progress gets ended. + /// This way, when the next work done progress is started, it starts at zero again. + /// + /// The number of outstanding tasks is determined from the `scheduled` and `executing` tasks in all the + /// `SemanticIndexManager`s. + private var queuedIndexTasks = 0 + + /// While there are ongoing index tasks, a `WorkDoneProgressManager` that displays the work done progress. + private var workDoneProgress: WorkDoneProgressManager? + + init(sourceKitLSPServer: SourceKitLSPServer) { + self.sourceKitLSPServer = sourceKitLSPServer + } + + /// Called when a new file is scheduled to be indexed. Increments the target index count, eg. the 3 in `1/3`. + nonisolated func indexTaskWasQueued(count: Int) { + queue.async { + await self.indexTaskWasQueuedImpl(count: count) + } + } + + private func indexTaskWasQueuedImpl(count: Int) async { + queuedIndexTasks += count + await indexStatusDidChangeImpl() + } + + /// Called when a `SemanticIndexManager` finishes indexing a file. Adjusts the done index count, eg. the 1 in `1/3`. + nonisolated func indexStatusDidChange() { + queue.async { + await self.indexStatusDidChangeImpl() + } + } + + private func indexStatusDidChangeImpl() async { + guard let sourceKitLSPServer else { + workDoneProgress = nil + return + } + var scheduled: [DocumentURI] = [] + var executing: [DocumentURI] = [] + for indexManager in await sourceKitLSPServer.workspaces.compactMap({ $0.semanticIndexManager }) { + let inProgress = await indexManager.inProgressIndexTasks + scheduled += inProgress.scheduled + executing += inProgress.executing + } + + if scheduled.isEmpty && executing.isEmpty { + // Nothing left to index. Reset the target count and dismiss the work done progress. + queuedIndexTasks = 0 + workDoneProgress = nil + return + } + + let finishedTasks = queuedIndexTasks - scheduled.count - executing.count + let message = "\(finishedTasks) / \(queuedIndexTasks)" + + let percentage = Int(Double(finishedTasks) / Double(queuedIndexTasks) * 100) + if let workDoneProgress { + workDoneProgress.update(message: message, percentage: percentage) + } else { + workDoneProgress = await WorkDoneProgressManager( + server: sourceKitLSPServer, + title: "Indexing", + message: message, + percentage: percentage + ) + } + } +} diff --git a/Sources/SourceKitLSP/SourceKitLSPServer+Options.swift b/Sources/SourceKitLSP/SourceKitLSPServer+Options.swift index 747eaee62..b7005c989 100644 --- a/Sources/SourceKitLSP/SourceKitLSPServer+Options.swift +++ b/Sources/SourceKitLSP/SourceKitLSPServer+Options.swift @@ -49,6 +49,11 @@ extension SourceKitLSPServer { /// notification when running unit tests. public var swiftPublishDiagnosticsDebounceDuration: TimeInterval + /// A callback that is called when an index task finishes. + /// + /// Intended for testing purposes. + public var indexTaskDidFinish: (@Sendable () -> Void)? + public init( buildSetup: BuildSetup = .default, clangdOptions: [String] = [], diff --git a/Sources/SourceKitLSP/SourceKitLSPServer.swift b/Sources/SourceKitLSP/SourceKitLSPServer.swift index 14553c26a..6c82f148d 100644 --- a/Sources/SourceKitLSP/SourceKitLSPServer.swift +++ b/Sources/SourceKitLSP/SourceKitLSPServer.swift @@ -474,6 +474,12 @@ public actor SourceKitLSPServer { /// number of processor cores that the user allocated to background indexing. private let indexTaskScheduler: TaskScheduler + /// Implicitly unwrapped optional so we can create an `IndexProgressManager` that has a weak reference to + /// `SourceKitLSPServer`. + /// `nonisolated(unsafe)` because `indexProgressManager` will not be modified after it is assigned from the + /// initializer. + private nonisolated(unsafe) var indexProgressManager: IndexProgressManager! + private var packageLoadingWorkDoneProgress = WorkDoneProgressState( "SourceKitLSP.SourceKitLSPServer.reloadPackage", title: "SourceKit-LSP: Reloading Package" @@ -546,6 +552,8 @@ public actor SourceKitLSPServer { (TaskPriority.medium, processorCount), (TaskPriority.low, max(Int(lowPriorityCores), 1)), ]) + self.indexProgressManager = nil + self.indexProgressManager = IndexProgressManager(sourceKitLSPServer: self) } /// Await until the server has send the reply to the initialize request. @@ -1192,6 +1200,7 @@ extension SourceKitLSPServer { logger.log("Cannot open workspace before server is initialized") return nil } + let indexTaskDidFinishCallback = options.indexTaskDidFinish var options = self.options options.buildSetup = self.options.buildSetup.merging(buildSetup(for: workspaceFolder)) return try? await Workspace( @@ -1205,16 +1214,19 @@ extension SourceKitLSPServer { indexTaskScheduler: indexTaskScheduler, reloadPackageStatusCallback: { [weak self] status in guard let self else { return } - guard capabilityRegistry.clientCapabilities.window?.workDoneProgress ?? false else { - // Client doesn’t support work done progress - return - } switch status { case .start: await self.packageLoadingWorkDoneProgress.startProgress(server: self) case .end: await self.packageLoadingWorkDoneProgress.endProgress(server: self) } + }, + indexTasksWereScheduled: { [weak self] count in + self?.indexProgressManager.indexTaskWasQueued(count: count) + }, + indexTaskDidFinish: { [weak self] in + self?.indexProgressManager.indexStatusDidChange() + indexTaskDidFinishCallback?() } ) } @@ -1263,6 +1275,7 @@ extension SourceKitLSPServer { if self.workspaces.isEmpty { logger.error("no workspace found") + let indexTaskDidFinishCallback = self.options.indexTaskDidFinish let workspace = await Workspace( documentManager: self.documentManager, rootUri: req.rootURI, @@ -1272,7 +1285,14 @@ extension SourceKitLSPServer { underlyingBuildSystem: nil, index: nil, indexDelegate: nil, - indexTaskScheduler: self.indexTaskScheduler + indexTaskScheduler: self.indexTaskScheduler, + indexTasksWereScheduled: { [weak self] count in + self?.indexProgressManager.indexTaskWasQueued(count: count) + }, + indexTaskDidFinish: { [weak self] in + self?.indexProgressManager.indexStatusDidChange() + indexTaskDidFinishCallback?() + } ) self.workspacesAndIsImplicit.append((workspace: workspace, isImplicit: false)) diff --git a/Sources/SourceKitLSP/Workspace.swift b/Sources/SourceKitLSP/Workspace.swift index c89cdd53b..cd7a083f4 100644 --- a/Sources/SourceKitLSP/Workspace.swift +++ b/Sources/SourceKitLSP/Workspace.swift @@ -89,7 +89,9 @@ public final class Workspace: Sendable { underlyingBuildSystem: BuildSystem?, index uncheckedIndex: UncheckedIndex?, indexDelegate: SourceKitIndexDelegate?, - indexTaskScheduler: TaskScheduler + indexTaskScheduler: TaskScheduler, + indexTasksWereScheduled: @escaping @Sendable (Int) -> Void, + indexTaskDidFinish: @escaping @Sendable () -> Void ) async { self.documentManager = documentManager self.buildSetup = options.buildSetup @@ -107,7 +109,8 @@ public final class Workspace: Sendable { index: uncheckedIndex, buildSystemManager: buildSystemManager, indexTaskScheduler: indexTaskScheduler, - indexTaskDidFinish: options.indexOptions.indexTaskDidFinish + indexTasksWereScheduled: indexTasksWereScheduled, + indexTaskDidFinish: indexTaskDidFinish ) } else { self.semanticIndexManager = nil @@ -143,7 +146,9 @@ public final class Workspace: Sendable { compilationDatabaseSearchPaths: [RelativePath], indexOptions: IndexOptions = IndexOptions(), indexTaskScheduler: TaskScheduler, - reloadPackageStatusCallback: @Sendable @escaping (ReloadPackageStatus) async -> Void + reloadPackageStatusCallback: @Sendable @escaping (ReloadPackageStatus) async -> Void, + indexTasksWereScheduled: @Sendable @escaping (Int) -> Void, + indexTaskDidFinish: @Sendable @escaping () -> Void ) async throws { var buildSystem: BuildSystem? = nil @@ -247,7 +252,9 @@ public final class Workspace: Sendable { underlyingBuildSystem: buildSystem, index: UncheckedIndex(index), indexDelegate: indexDelegate, - indexTaskScheduler: indexTaskScheduler + indexTaskScheduler: indexTaskScheduler, + indexTasksWereScheduled: indexTasksWereScheduled, + indexTaskDidFinish: indexTaskDidFinish ) } @@ -303,19 +310,13 @@ public struct IndexOptions: Sendable { /// Setting this to a value < 1 ensures that background indexing doesn't use all CPU resources. public var maxCoresPercentageToUseForBackgroundIndexing: Double - /// A callback that is called when an index task finishes. - /// - /// Intended for testing purposes. - public var indexTaskDidFinish: (@Sendable () -> Void)? - public init( indexStorePath: AbsolutePath? = nil, indexDatabasePath: AbsolutePath? = nil, indexPrefixMappings: [PathPrefixMapping]? = nil, listenToUnitEvents: Bool = true, enableBackgroundIndexing: Bool = false, - maxCoresPercentageToUseForBackgroundIndexing: Double = 1, - indexTaskDidFinish: (@Sendable () -> Void)? = nil + maxCoresPercentageToUseForBackgroundIndexing: Double = 1 ) { self.indexStorePath = indexStorePath self.indexDatabasePath = indexDatabasePath @@ -323,6 +324,5 @@ public struct IndexOptions: Sendable { self.listenToUnitEvents = listenToUnitEvents self.enableBackgroundIndexing = enableBackgroundIndexing self.maxCoresPercentageToUseForBackgroundIndexing = maxCoresPercentageToUseForBackgroundIndexing - self.indexTaskDidFinish = indexTaskDidFinish } } diff --git a/Tests/SourceKitLSPTests/BackgroundIndexingTests.swift b/Tests/SourceKitLSPTests/BackgroundIndexingTests.swift index 7ec0dd82a..3e1bbf702 100644 --- a/Tests/SourceKitLSPTests/BackgroundIndexingTests.swift +++ b/Tests/SourceKitLSPTests/BackgroundIndexingTests.swift @@ -165,7 +165,7 @@ final class BackgroundIndexingTests: XCTestCase { func testBackgroundIndexingHappensWithLowPriority() async throws { var serverOptions = backgroundIndexingOptions - serverOptions.indexOptions.indexTaskDidFinish = { + serverOptions.indexTaskDidFinish = { XCTAssert( Task.currentPriority == .low, "An index task ran with priority \(Task.currentPriority)" @@ -328,4 +328,49 @@ final class BackgroundIndexingTests: XCTestCase { ] ) } + + func testBackgroundIndexingStatusWorkDoneProgress() async throws { + let workDoneProgressCreated = self.expectation(description: "Work done progress created") + let project = try await SwiftPMTestProject( + files: [ + "MyFile.swift": """ + func foo() {} + func bar() { + foo() + } + """ + ], + capabilities: ClientCapabilities(window: WindowClientCapabilities(workDoneProgress: true)), + serverOptions: backgroundIndexingOptions, + preInitialization: { testClient in + testClient.handleNextRequest { (request: CreateWorkDoneProgressRequest) in + workDoneProgressCreated.fulfill() + return VoidResponse() + } + } + ) + try await fulfillmentOfOrThrow([workDoneProgressCreated]) + let workBeginProgress = try await project.testClient.nextNotification(ofType: WorkDoneProgress.self) + guard case .begin = workBeginProgress.value else { + XCTFail("Expected begin work done progress") + return + } + var didGetEndWorkDoneProgress = false + for _ in 0..<3 { + let workEndProgress = try await project.testClient.nextNotification(ofType: WorkDoneProgress.self) + switch workEndProgress.value { + case .begin: + XCTFail("Unexpected begin work done progress") + case .report: + // Allow up to 2 work done progress reports. + continue + case .end: + didGetEndWorkDoneProgress = true + } + break + } + XCTAssert(didGetEndWorkDoneProgress, "Expected end work done progress") + + withExtendedLifetime(project) {} + } } diff --git a/Tests/SourceKitLSPTests/BuildSystemTests.swift b/Tests/SourceKitLSPTests/BuildSystemTests.swift index 501abd0b7..c3eb343c8 100644 --- a/Tests/SourceKitLSPTests/BuildSystemTests.swift +++ b/Tests/SourceKitLSPTests/BuildSystemTests.swift @@ -127,7 +127,9 @@ final class BuildSystemTests: XCTestCase { underlyingBuildSystem: buildSystem, index: nil, indexDelegate: nil, - indexTaskScheduler: .forTesting + indexTaskScheduler: .forTesting, + indexTasksWereScheduled: { _ in }, + indexTaskDidFinish: {} ) await server.setWorkspaces([(workspace: workspace, isImplicit: false)])