diff --git a/Sources/SKCore/TaskScheduler.swift b/Sources/SKCore/TaskScheduler.swift index 77e0f7ac2..23be76912 100644 --- a/Sources/SKCore/TaskScheduler.swift +++ b/Sources/SKCore/TaskScheduler.swift @@ -87,7 +87,7 @@ public enum TaskExecutionState { case finished } -fileprivate actor QueuedTask { +public actor QueuedTask { /// Result of `executionTask` / the tasks in `executionTaskCreatedContinuation`. /// See doc comment on `executionTask`. enum ExecutionTaskFinishStatus { @@ -147,14 +147,38 @@ fileprivate actor QueuedTask { /// Gets reset every time `executionTask` finishes. nonisolated(unsafe) private var cancelledToBeRescheduled: AtomicBool = .init(initialValue: false) + private nonisolated(unsafe) var _isExecuting: AtomicBool = .init(initialValue: false) + + /// Whether the task is currently executing or still queued to be executed later. + public nonisolated var isExecuting: Bool { + return _isExecuting.value + } + + /// Wait for the task to finish. + /// + /// If the tasks that waits for this queued task to finished is cancelled, the QueuedTask will still continue + /// executing. + public func waitToFinish() async { + return await resultTask.value + } + + /// Wait for the task to finish. + /// + /// If the tasks that waits for this queued task to finished is cancelled, the QueuedTask will also be cancelled. + /// This assumes that the caller of this method has unique control over the task and is the only one interested in its + /// value. + public func waitToFinishPropagatingCancellation() async { + return await resultTask.valuePropagatingCancellation + } + /// 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)? + private let executionStateChangedCallback: (@Sendable (QueuedTask, TaskExecutionState) async -> Void)? init( priority: TaskPriority? = nil, description: TaskDescription, - executionStateChangedCallback: (@Sendable (TaskExecutionState) async -> Void)? + executionStateChangedCallback: (@Sendable (QueuedTask, TaskExecutionState) async -> Void)? ) async { self._priority = .init(initialValue: priority?.rawValue ?? Task.currentPriority.rawValue) self.description = description @@ -214,19 +238,21 @@ fileprivate actor QueuedTask { } executionTask = task executionTaskCreatedContinuation.yield(task) - await executionStateChangedCallback?(.executing) + _isExecuting.value = true + await executionStateChangedCallback?(self, .executing) return await task.value } /// Implementation detail of `execute` that is called after `self.description.execute()` finishes. private func finalizeExecution() async -> ExecutionTaskFinishStatus { self.executionTask = nil + _isExecuting.value = false if Task.isCancelled && self.cancelledToBeRescheduled.value { - await executionStateChangedCallback?(.cancelledToBeRescheduled) + await executionStateChangedCallback?(self, .cancelledToBeRescheduled) self.cancelledToBeRescheduled.value = false return ExecutionTaskFinishStatus.cancelledToBeRescheduled } else { - await executionStateChangedCallback?(.finished) + await executionStateChangedCallback?(self, .finished) return ExecutionTaskFinishStatus.terminated } } @@ -327,8 +353,10 @@ public actor TaskScheduler { public func schedule( priority: TaskPriority? = nil, _ taskDescription: TaskDescription, - @_inheritActorContext executionStateChangedCallback: (@Sendable (TaskExecutionState) async -> Void)? = nil - ) async -> Task { + @_inheritActorContext executionStateChangedCallback: ( + @Sendable (QueuedTask, TaskExecutionState) async -> Void + )? = nil + ) async -> QueuedTask { let queuedTask = await QueuedTask( priority: priority, description: taskDescription, @@ -341,7 +369,7 @@ public actor TaskScheduler { // queued task. await self.poke() } - return queuedTask.resultTask + return queuedTask } /// Trigger all queued tasks to update their priority. diff --git a/Sources/SKSupport/Sequence+AsyncMap.swift b/Sources/SKSupport/Sequence+AsyncMap.swift index e6cb2f360..97d6817db 100644 --- a/Sources/SKSupport/Sequence+AsyncMap.swift +++ b/Sources/SKSupport/Sequence+AsyncMap.swift @@ -39,4 +39,19 @@ extension Sequence { return result } + + /// Just like `Sequence.map` but allows an `async` transform function. + public func asyncFilter( + @_inheritActorContext _ predicate: @Sendable (Element) async throws -> Bool + ) async rethrows -> [Element] { + var result: [Element] = [] + + for element in self { + if try await predicate(element) { + result.append(element) + } + } + + return result + } } diff --git a/Sources/SemanticIndex/CMakeLists.txt b/Sources/SemanticIndex/CMakeLists.txt index 85372d49e..b087edaeb 100644 --- a/Sources/SemanticIndex/CMakeLists.txt +++ b/Sources/SemanticIndex/CMakeLists.txt @@ -2,6 +2,7 @@ add_library(SemanticIndex STATIC CheckedIndex.swift CompilerCommandLineOption.swift + IndexStatusManager.swift IndexTaskDescription.swift PreparationTaskDescription.swift SemanticIndexManager.swift diff --git a/Sources/SemanticIndex/IndexStatusManager.swift b/Sources/SemanticIndex/IndexStatusManager.swift new file mode 100644 index 000000000..016243bf8 --- /dev/null +++ b/Sources/SemanticIndex/IndexStatusManager.swift @@ -0,0 +1,70 @@ +//===----------------------------------------------------------------------===// +// +// 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 Foundation +import SKCore + +/// Keeps track of whether an item (a target or file to index) is up-to-date. +actor IndexUpToDateStatusManager { + private enum Status { + /// The item is up-to-date. + case upToDate + + /// The target or file has been marked out-of-date at the given date. + /// + /// Keeping track of the date is necessary so that we don't mark a target as up-to-date if we have the following + /// ordering of events: + /// - Preparation started + /// - Target marked out of date + /// - Preparation finished + case outOfDate(Date) + } + + private var status: [Item: Status] = [:] + + /// Mark the target or file as up-to-date from a preparation/update-indexstore operation started at + /// `updateOperationStartDate`. + /// + /// See comment on `Status.outOfDate` why `updateOperationStartDate` needs to be passed. + func markUpToDate(_ items: [Item], updateOperationStartDate: Date) { + for item in items { + switch status[item] { + case .upToDate: + break + case .outOfDate(let markedOutOfDate): + if markedOutOfDate < updateOperationStartDate { + status[item] = .upToDate + } + case nil: + status[item] = .upToDate + } + } + } + + func markOutOfDate(_ items: some Collection) { + let date = Date() + for item in items { + status[item] = .outOfDate(date) + } + } + + func markAllOutOfDate() { + markOutOfDate(status.keys) + } + + func isUpToDate(_ item: Item) -> Bool { + if case .upToDate = status[item] { + return true + } + return false + } +} diff --git a/Sources/SemanticIndex/PreparationTaskDescription.swift b/Sources/SemanticIndex/PreparationTaskDescription.swift index 08263d9ea..049ce896c 100644 --- a/Sources/SemanticIndex/PreparationTaskDescription.swift +++ b/Sources/SemanticIndex/PreparationTaskDescription.swift @@ -35,6 +35,8 @@ 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 + private let preparationUpToDateStatus: IndexUpToDateStatusManager + /// Test hooks that should be called when the preparation task finishes. private let testHooks: IndexTestHooks @@ -54,10 +56,12 @@ public struct PreparationTaskDescription: IndexTaskDescription { init( targetsToPrepare: [ConfiguredTarget], buildSystemManager: BuildSystemManager, + preparationUpToDateStatus: IndexUpToDateStatusManager, testHooks: IndexTestHooks ) { self.targetsToPrepare = targetsToPrepare self.buildSystemManager = buildSystemManager + self.preparationUpToDateStatus = preparationUpToDateStatus self.testHooks = testHooks } @@ -66,10 +70,15 @@ public struct PreparationTaskDescription: IndexTaskDescription { // See comment in `withLoggingScope`. // The last 2 digits should be sufficient to differentiate between multiple concurrently running preparation operations await withLoggingScope("preparation-\(id % 100)") { - let startDate = Date() - let targetsToPrepare = targetsToPrepare.sorted(by: { + let targetsToPrepare = await targetsToPrepare.asyncFilter { + await !preparationUpToDateStatus.isUpToDate($0) + }.sorted(by: { ($0.targetID, $0.runDestinationID) < ($1.targetID, $1.runDestinationID) }) + if targetsToPrepare.isEmpty { + return + } + let targetsToPrepareDescription = targetsToPrepare .map { "\($0.targetID)-\($0.runDestinationID)" } @@ -77,6 +86,7 @@ public struct PreparationTaskDescription: IndexTaskDescription { logger.log( "Starting preparation with priority \(Task.currentPriority.rawValue, privacy: .public): \(targetsToPrepareDescription)" ) + let startDate = Date() do { try await buildSystemManager.prepare(targets: targetsToPrepare) } catch { @@ -85,6 +95,9 @@ public struct PreparationTaskDescription: IndexTaskDescription { ) } await testHooks.preparationTaskDidFinish?(self) + if !Task.isCancelled { + await preparationUpToDateStatus.markUpToDate(targetsToPrepare, updateOperationStartDate: startDate) + } logger.log( "Finished preparation in \(Date().timeIntervalSince(startDate) * 1000, privacy: .public)ms: \(targetsToPrepareDescription)" ) diff --git a/Sources/SemanticIndex/SemanticIndexManager.swift b/Sources/SemanticIndex/SemanticIndexManager.swift index 2f304ba70..6ce61adfc 100644 --- a/Sources/SemanticIndex/SemanticIndexManager.swift +++ b/Sources/SemanticIndex/SemanticIndexManager.swift @@ -15,40 +15,44 @@ import LSPLogging import LanguageServerProtocol import SKCore -/// Describes the state of indexing for a single source file -private enum IndexStatus { - /// The index is up-to-date. - case upToDate - /// The file or target is not up to date. We have scheduled a task to update the index store for the file / prepare - /// the target, but that index operation hasn't been started yet. - case scheduled(T) - /// We are currently actively updating the index store for the file / preparing the target, ie. we are running a - /// subprocess that updates the index store / prepares a target. - case executing(T) - - var description: String { - switch self { - case .upToDate: - return "upToDate" - case .scheduled: - return "scheduled" - case .executing: - return "executing" - } +/// A wrapper around `QueuedTask` that only allows equality comparison and inspection whether the `QueuedTask` is +/// currently executing. +/// +/// This way we can store `QueuedTask` in the `inProgress` dictionaries while guaranteeing that whoever created the +/// queued task still has exclusive ownership of the task and can thus control the task's cancellation. +private struct OpaqueQueuedIndexTask: Equatable { + private let task: QueuedTask + + var isExecuting: Bool { + task.isExecuting } -} -/// See `SemanticIndexManager.preparationStatus` -private struct PreparationTaskStatusData { - /// A UUID to track the task. This is used to ensure that status updates from this task don't update - /// `preparationStatus` for targets that are tracked by a different task. - let taskID: UUID + init(_ task: QueuedTask) { + self.task = task + } - /// The list of targets that are being prepared in a joint preparation operation. - let targets: [ConfiguredTarget] + static func == (lhs: OpaqueQueuedIndexTask, rhs: OpaqueQueuedIndexTask) -> Bool { + return lhs.task === rhs.task + } +} - /// The task that prepares the target - let task: Task +private enum InProgressIndexStore { + /// We are waiting for preparation of the file's target to finish before we can index it. + /// + /// `preparationTaskID` identifies the preparation task so that we can transition a file's index state to + /// `updatingIndexStore` when its preparation task has finished. + /// + /// `indexTask` is a task that finishes after both preparation and index store update are done. Whoever owns the index + /// task is still the sole owner of it and responsible for its cancellation. + case waitingForPreparation(preparationTaskID: UUID, indexTask: Task) + + /// The file's target has been prepared and we are updating the file's index store. + /// + /// `updateIndexStoreTask` is the task that updates the index store itself. + /// + /// `indexTask` is a task that finishes after both preparation and index store update are done. Whoever owns the index + /// task is still the sole owner of it and responsible for its cancellation. + case updatingIndexStore(updateIndexStoreTask: OpaqueQueuedIndexTask, indexTask: Task) } /// Schedules index tasks and keeps track of the index status of files. @@ -66,20 +70,21 @@ public final actor SemanticIndexManager { /// ...). `nil` if no build graph is currently being generated. private var generateBuildGraphTask: Task? - /// The preparation status of the targets that the `SemanticIndexManager` has started preparation for. - /// - /// Targets will be removed from this dictionary when they are no longer known to be up-to-date. - private var preparationStatus: [ConfiguredTarget: IndexStatus] = [:] + private let preparationUpToDateStatus = IndexUpToDateStatusManager() + + private let indexStoreUpToDateStatus = IndexUpToDateStatusManager() - /// The index status of the source files that the `SemanticIndexManager` knows about. + /// The preparation tasks that have been started and are either scheduled in the task scheduler or currently + /// executing. /// - /// Files will be removed from this dictionary if their index is no longer up-to-date. + /// After a preparation task finishes, it is removed from this dictionary. + private var inProgressPreparationTasks: [ConfiguredTarget: OpaqueQueuedIndexTask] = [:] + + /// The files that are currently being index, either waiting for their target to be prepared, waiting for the index + /// store update task to be scheduled in the task scheduler or which currently have an index store update running. /// - /// The associated values of the `IndexStatus` are: - /// - A UUID to track the task. This is used to ensure that status updates from this task don't update - /// `preparationStatus` for targets that are tracked by a different task. - /// - The task that prepares the target - private var indexStatus: [DocumentURI: IndexStatus<(UUID, Task)>] = [:] + /// After the file is indexed, it is removed from this dictionary. + private var inProgressIndexTasks: [DocumentURI: InProgressIndexStore] = [:] /// 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 @@ -103,21 +108,30 @@ public final actor SemanticIndexManager { /// 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: IndexStatus) in - if case .scheduled = status { - return uri + /// Scheduled tasks are files that are waiting for their target to be prepared or whose index store update task is + /// waiting to be scheduled by the task scheduler. + /// + /// `executing` are the files that currently have an active index store update task running. + public var inProgressIndexFiles: (scheduled: [DocumentURI], executing: [DocumentURI]) { + var scheduled: [DocumentURI] = [] + var executing: [DocumentURI] = [] + for (uri, status) in inProgressIndexTasks { + let isExecuting: Bool + switch status { + case .waitingForPreparation: + isExecuting = false + case .updatingIndexStore(updateIndexStoreTask: let updateIndexStoreTask, indexTask: _): + isExecuting = updateIndexStoreTask.isExecuting } - return nil - } - let inProgress = indexStatus.compactMap { (uri: DocumentURI, status: IndexStatus) in - if case .executing = status { - return uri + + if isExecuting { + executing.append(uri) + } else { + scheduled.append(uri) } - return nil } - return (scheduled, inProgress) + + return (scheduled, executing) } public init( @@ -176,14 +190,14 @@ public final actor SemanticIndexManager { await generateBuildGraphTask?.value await withTaskGroup(of: Void.self) { taskGroup in - for (_, status) in indexStatus { + for (_, status) in inProgressIndexTasks { switch status { - case .scheduled((_, let task)), .executing((_, let task)): + case .waitingForPreparation(preparationTaskID: _, indexTask: let indexTask), + .updatingIndexStore(updateIndexStoreTask: _, indexTask: let indexTask): taskGroup.addTask { - await task.value + await indexTask.value } - case .upToDate: - break + } } await taskGroup.waitForAll() @@ -216,21 +230,26 @@ public final actor SemanticIndexManager { // We only re-index the files that were changed and don't re-index any of their dependencies. See the // `Documentation/Files_To_Reindex.md` file. let changedFiles = events.map(\.uri) - // Reset the index status for these files so they get re-indexed by `index(files:priority:)` - for uri in changedFiles { - indexStatus[uri] = nil - } + await indexStoreUpToDateStatus.markOutOfDate(changedFiles) + // Note that configured targets are the right abstraction layer here (instead of a non-configured target) because a // build system might have targets that include different source files. Hence a source file might be in target T // configured for macOS but not in target T configured for iOS. let targets = await changedFiles.asyncMap { await buildSystemManager.configuredTargets(for: $0) }.flatMap { $0 } if let dependentTargets = await buildSystemManager.targets(dependingOn: targets) { - for dependentTarget in dependentTargets { - preparationStatus[dependentTarget] = nil - } + await preparationUpToDateStatus.markOutOfDate(dependentTargets) } else { // We couldn't determine which targets depend on the modified targets. Be conservative and assume all of them do. - preparationStatus = [:] + await indexStoreUpToDateStatus.markOutOfDate(changedFiles) + + await preparationUpToDateStatus.markAllOutOfDate() + // `markAllOutOfDate` only marks targets out-of-date that have been indexed before. Also mark all targets with + // in-progress preparation out of date. So we don't get into the following situation, which would result in an + // incorrect up-to-date status of a target + // - Target preparation starts for the first time + // - Files changed + // - Target preparation finishes. + await preparationUpToDateStatus.markOutOfDate(inProgressPreparationTasks.keys) } await scheduleBackgroundIndex(files: changedFiles) @@ -283,124 +302,83 @@ public final actor SemanticIndexManager { /// Prepare the given targets for indexing private func prepare(targets: [ConfiguredTarget], priority: TaskPriority?) async { - var targetsToPrepare: [ConfiguredTarget] = [] - var preparationTasksToAwait: [Task] = [] - for target in targets { - switch preparationStatus[target] { - case .upToDate: - break - case .scheduled(let existingTaskData), .executing(let existingTaskData): - // If we already have a task scheduled that prepares fewer targets, await that instead of overriding the - // target's preparation status with a longer-running task. The key benefit here is that when we get many - // preparation requests for the same target (eg. one for every text document request sent to a file), we don't - // re-create new `PreparationTaskDescription`s for every preparation request. Instead, all the preparation - // requests await the same task. At the same time, if we have a multi-file preparation request and then get a - // single-file preparation request, we will override the preparation of that target with the single-file - // preparation task, ensuring that the task gets prepared as quickly as possible. - if existingTaskData.targets.count <= targets.count { - preparationTasksToAwait.append(existingTaskData.task) - } else { - targetsToPrepare.append(target) - } - case nil: - targetsToPrepare.append(target) - } + // Perform a quick initial check whether the target is up-to-date, in which case we don't need to schedule a + // preparation operation at all. + // We will check the up-to-date status again in `PreparationTaskDescription.execute`. This ensures that if we + // schedule two preparations of the same target in quick succession, only the first one actually performs a prepare + // and the second one will be a no-op once it runs. + let targetsToPrepare = await targets.asyncFilter { + await !preparationUpToDateStatus.isUpToDate($0) + } + + guard !targetsToPrepare.isEmpty else { + return } let taskDescription = AnyIndexTaskDescription( PreparationTaskDescription( targetsToPrepare: targetsToPrepare, buildSystemManager: self.buildSystemManager, + preparationUpToDateStatus: preparationUpToDateStatus, testHooks: testHooks ) ) - if !targetsToPrepare.isEmpty { - // A UUID that is used to identify the task. This ensures that status updates from this task don't update - // `preparationStatus` for targets that are tracked by a different task, eg. because this task is a multi-target - // preparation task and the target's status is now tracked by a single-file preparation task. - let taskID = UUID() - let preparationTask = await self.indexTaskScheduler.schedule(priority: priority, taskDescription) { newState in - switch newState { - case .executing: - for target in targetsToPrepare { - if case .scheduled(let existingTaskData) = self.preparationStatus[target], existingTaskData.taskID == taskID - { - self.preparationStatus[target] = .executing(existingTaskData) - } - } - case .cancelledToBeRescheduled: - for target in targetsToPrepare { - if case .executing(let existingTaskData) = self.preparationStatus[target], existingTaskData.taskID == taskID - { - self.preparationStatus[target] = .scheduled(existingTaskData) - } - } - case .finished: - for target in targetsToPrepare { - switch self.preparationStatus[target] { - case .executing(let existingTaskData) where existingTaskData.taskID == taskID: - self.preparationStatus[target] = .upToDate - default: - break - } - } - self.indexTaskDidFinish() - } + let preparationTask = await indexTaskScheduler.schedule(priority: priority, taskDescription) { task, newState in + guard case .finished = newState else { + return } for target in targetsToPrepare { - preparationStatus[target] = .scheduled( - PreparationTaskStatusData(taskID: taskID, targets: targetsToPrepare, task: preparationTask) - ) - } - preparationTasksToAwait.append(preparationTask) - } - await withTaskGroup(of: Void.self) { taskGroup in - for task in preparationTasksToAwait { - taskGroup.addTask { - await task.valuePropagatingCancellation + if self.inProgressPreparationTasks[target] == OpaqueQueuedIndexTask(task) { + self.inProgressPreparationTasks[target] = nil } } - await taskGroup.waitForAll() + self.indexTaskDidFinish() + } + for target in targetsToPrepare { + inProgressPreparationTasks[target] = OpaqueQueuedIndexTask(preparationTask) } + return await preparationTask.waitToFinishPropagatingCancellation() } /// Update the index store for the given files, assuming that their targets have already been prepared. - private func updateIndexStore(for filesAndTargets: [FileAndTarget], taskID: UUID, priority: TaskPriority?) async { + private func updateIndexStore( + for filesAndTargets: [FileAndTarget], + preparationTaskID: UUID, + priority: TaskPriority? + ) async { let taskDescription = AnyIndexTaskDescription( UpdateIndexStoreTaskDescription( filesToIndex: filesAndTargets, buildSystemManager: self.buildSystemManager, index: index, + indexStoreUpToDateStatus: indexStoreUpToDateStatus, testHooks: testHooks ) ) - let updateIndexStoreTask = await self.indexTaskScheduler.schedule(priority: priority, taskDescription) { newState in - switch newState { - case .executing: - for fileAndTarget in filesAndTargets { - if case .scheduled((taskID, let task)) = self.indexStatus[fileAndTarget.file.sourceFile] { - self.indexStatus[fileAndTarget.file.sourceFile] = .executing((taskID, task)) - } - } - case .cancelledToBeRescheduled: - for fileAndTarget in filesAndTargets { - if case .executing((taskID, let task)) = self.indexStatus[fileAndTarget.file.sourceFile] { - self.indexStatus[fileAndTarget.file.sourceFile] = .scheduled((taskID, task)) - } - } - case .finished: - for fileAndTarget in filesAndTargets { - switch self.indexStatus[fileAndTarget.file.sourceFile] { - case .executing((taskID, _)): - self.indexStatus[fileAndTarget.file.sourceFile] = .upToDate - default: - break - } + let updateIndexTask = await indexTaskScheduler.schedule(priority: priority, taskDescription) { task, newState in + guard case .finished = newState else { + return + } + for fileAndTarget in filesAndTargets { + if case .updatingIndexStore(OpaqueQueuedIndexTask(task), _) = self.inProgressIndexTasks[ + fileAndTarget.file.sourceFile + ] { + self.inProgressIndexTasks[fileAndTarget.file.sourceFile] = nil } - self.indexTaskDidFinish() + } + self.indexTaskDidFinish() + } + for fileAndTarget in filesAndTargets { + if case .waitingForPreparation(preparationTaskID, let indexTask) = inProgressIndexTasks[ + fileAndTarget.file.sourceFile + ] { + inProgressIndexTasks[fileAndTarget.file.sourceFile] = .updatingIndexStore( + updateIndexStoreTask: OpaqueQueuedIndexTask(updateIndexTask), + indexTask: indexTask + ) } } - await updateIndexStoreTask.value + return await updateIndexTask.waitToFinishPropagatingCancellation() } /// Index the given set of files at the given priority, preparing their targets beforehand, if needed. @@ -410,11 +388,13 @@ public final actor SemanticIndexManager { of files: some Collection, priority: TaskPriority? ) async -> Task { - let outOfDateFiles = await filesToIndex(toCover: files).filter { - if case .upToDate = indexStatus[$0.sourceFile] { - return false - } - return true + // Perform a quick initial check to whether the files is up-to-date, in which case we don't need to schedule a + // prepare and index operation at all. + // We will check the up-to-date status again in `IndexTaskDescription.execute`. This ensures that if we schedule + // schedule two indexing jobs for the same file in quick succession, only the first one actually updates the index + // store and the second one will be a no-op once it runs. + let outOfDateFiles = await filesToIndex(toCover: files).asyncFilter { + return await !indexStoreUpToDateStatus.isUpToDate($0.sourceFile) } // sort files to get deterministic indexing order .sorted(by: { $0.sourceFile.stringValue < $1.sourceFile.stringValue }) @@ -456,7 +436,7 @@ public final actor SemanticIndexManager { // processor count, so we can get parallelism during preparation. // https://github.com/apple/sourcekit-lsp/issues/1262 for targetsBatch in sortedTargets.partition(intoBatchesOfSize: 1) { - let taskID = UUID() + let preparationTaskID = UUID() let indexTask = Task(priority: priority) { // First prepare the targets. await prepare(targets: targetsBatch, priority: priority) @@ -471,7 +451,7 @@ public final actor SemanticIndexManager { taskGroup.addTask { await self.updateIndexStore( for: fileBatch.map { FileAndTarget(file: $0, target: target) }, - taskID: taskID, + preparationTaskID: preparationTaskID, priority: priority ) } @@ -488,7 +468,10 @@ public final actor SemanticIndexManager { // 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.sourceFile] = .scheduled((taskID, indexTask)) + inProgressIndexTasks[file.sourceFile] = .waitingForPreparation( + preparationTaskID: preparationTaskID, + indexTask: indexTask + ) } indexTasksWereScheduled(filesToIndex.count) } diff --git a/Sources/SemanticIndex/UpdateIndexStoreTaskDescription.swift b/Sources/SemanticIndex/UpdateIndexStoreTaskDescription.swift index 0c0e8ad4c..3863a27ec 100644 --- a/Sources/SemanticIndex/UpdateIndexStoreTaskDescription.swift +++ b/Sources/SemanticIndex/UpdateIndexStoreTaskDescription.swift @@ -89,6 +89,8 @@ public struct UpdateIndexStoreTaskDescription: IndexTaskDescription { /// The build system manager that is used to get the toolchain and build settings for the files to index. private let buildSystemManager: BuildSystemManager + private let indexStoreUpToDateStatus: IndexUpToDateStatusManager + /// A reference to the underlying index store. Used to check if the index is already up-to-date for a file, in which /// case we don't need to index it again. private let index: UncheckedIndex @@ -113,11 +115,13 @@ public struct UpdateIndexStoreTaskDescription: IndexTaskDescription { filesToIndex: [FileAndTarget], buildSystemManager: BuildSystemManager, index: UncheckedIndex, + indexStoreUpToDateStatus: IndexUpToDateStatusManager, testHooks: IndexTestHooks ) { self.filesToIndex = filesToIndex self.buildSystemManager = buildSystemManager self.index = index + self.indexStoreUpToDateStatus = indexStoreUpToDateStatus self.testHooks = testHooks } @@ -176,6 +180,10 @@ public struct UpdateIndexStoreTaskDescription: IndexTaskDescription { } private func updateIndexStore(forSingleFile file: FileToIndex, in target: ConfiguredTarget) async { + guard await !indexStoreUpToDateStatus.isUpToDate(file.sourceFile) else { + // If we know that the file is up-to-date without having ot hit the index, do that because it's fastest. + return + } guard let sourceFileUrl = file.sourceFile.fileURL else { // The URI is not a file, so there's nothing we can index. return @@ -205,6 +213,7 @@ public struct UpdateIndexStoreTaskDescription: IndexTaskDescription { ) return } + let startDate = Date() switch language { case .swift: do { @@ -233,6 +242,7 @@ public struct UpdateIndexStoreTaskDescription: IndexTaskDescription { "Not updating index store for \(file) because it is a language that is not supported by background indexing" ) } + await indexStoreUpToDateStatus.markUpToDate([file.sourceFile], updateOperationStartDate: startDate) } private func updateIndexStore( diff --git a/Sources/SourceKitLSP/IndexProgressManager.swift b/Sources/SourceKitLSP/IndexProgressManager.swift index fdc745fa3..fb758af73 100644 --- a/Sources/SourceKitLSP/IndexProgressManager.swift +++ b/Sources/SourceKitLSP/IndexProgressManager.swift @@ -73,7 +73,7 @@ actor IndexProgressManager { var scheduled: [DocumentURI] = [] var executing: [DocumentURI] = [] for indexManager in await sourceKitLSPServer.workspaces.compactMap({ $0.semanticIndexManager }) { - let inProgress = await indexManager.inProgressIndexTasks + let inProgress = await indexManager.inProgressIndexFiles scheduled += inProgress.scheduled executing += inProgress.executing } diff --git a/Tests/SKCoreTests/TaskSchedulerTests.swift b/Tests/SKCoreTests/TaskSchedulerTests.swift index 9ca7a1bd0..90f07fe6c 100644 --- a/Tests/SKCoreTests/TaskSchedulerTests.swift +++ b/Tests/SKCoreTests/TaskSchedulerTests.swift @@ -348,7 +348,9 @@ fileprivate extension TaskScheduler { body, dependencies: dependencies ) - return await self.schedule(priority: priority, taskDescription) + return Task(priority: priority) { + await self.schedule(priority: priority, taskDescription).waitToFinishPropagatingCancellation() + } } } diff --git a/Tests/SourceKitLSPTests/BackgroundIndexingTests.swift b/Tests/SourceKitLSPTests/BackgroundIndexingTests.swift index 7f0a20812..8efb8b041 100644 --- a/Tests/SourceKitLSPTests/BackgroundIndexingTests.swift +++ b/Tests/SourceKitLSPTests/BackgroundIndexingTests.swift @@ -558,8 +558,7 @@ final class BackgroundIndexingTests: XCTestCase { ConfiguredTarget(targetID: "LibB", runDestinationID: "dummy"), ], [ - ConfiguredTarget(targetID: "LibA", runDestinationID: "dummy"), - ConfiguredTarget(targetID: "LibB", runDestinationID: "dummy"), + ConfiguredTarget(targetID: "LibB", runDestinationID: "dummy") ], ]) serverOptions.indexTestHooks = expectedPreparationTracker.testHooks