Skip to content

Commit e295a4e

Browse files
committed
Split up-to-date status tracking and index progress tracking
We were mixing the up-to-date status and in-progress status of an index task in `SemanticIndexManager`. This meant that a single `QueuedTask` in the task scheduler could be needed for eg. both preparation for editor functionality in a file of that target and to re-index a file in that target. This dual ownership made it unclear, which caller would be entitled to cancel the task. Furthermore, we needed to duplicate some logic from the preparation task dependencies in `SemanticIndexManager.prepare`. To simplify things: - Split the up-to-date status and the in-progress status into two different data structures - Make the caller of `prepare` and `scheduleIndex` responsible for cancellation of the task it has scheduled. `TaskScheduler` might receive more scheduled tasks this way but the additional tasks should all be no-ops because the status is known to be up-to-date when they execute.
1 parent d12c946 commit e295a4e

File tree

10 files changed

+300
-179
lines changed

10 files changed

+300
-179
lines changed

Sources/SKCore/TaskScheduler.swift

Lines changed: 37 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,7 @@ public enum TaskExecutionState {
8787
case finished
8888
}
8989

90-
fileprivate actor QueuedTask<TaskDescription: TaskDescriptionProtocol> {
90+
public actor QueuedTask<TaskDescription: TaskDescriptionProtocol> {
9191
/// Result of `executionTask` / the tasks in `executionTaskCreatedContinuation`.
9292
/// See doc comment on `executionTask`.
9393
enum ExecutionTaskFinishStatus {
@@ -147,14 +147,38 @@ fileprivate actor QueuedTask<TaskDescription: TaskDescriptionProtocol> {
147147
/// Gets reset every time `executionTask` finishes.
148148
nonisolated(unsafe) private var cancelledToBeRescheduled: AtomicBool = .init(initialValue: false)
149149

150+
private nonisolated(unsafe) var _isExecuting: AtomicBool = .init(initialValue: false)
151+
152+
/// Whether the task is currently executing or still queued to be executed later.
153+
public nonisolated var isExecuting: Bool {
154+
return _isExecuting.value
155+
}
156+
157+
/// Wait for the task to finish.
158+
///
159+
/// If the tasks that waits for this queued task to finished is cancelled, the QueuedTask will still continue
160+
/// executing.
161+
public func waitToFinish() async {
162+
return await resultTask.value
163+
}
164+
165+
/// Wait for the task to finish.
166+
///
167+
/// If the tasks that waits for this queued task to finished is cancelled, the QueuedTask will also be cancelled.
168+
/// This assumes that the caller of this method has unique control over the task and is the only one interested in its
169+
/// value.
170+
public func waitToFinishPropagatingCancellation() async {
171+
return await resultTask.valuePropagatingCancellation
172+
}
173+
150174
/// A callback that will be called when the task starts executing, is cancelled to be rescheduled, or when it finishes
151175
/// execution.
152-
private let executionStateChangedCallback: (@Sendable (TaskExecutionState) async -> Void)?
176+
private let executionStateChangedCallback: (@Sendable (QueuedTask, TaskExecutionState) async -> Void)?
153177

154178
init(
155179
priority: TaskPriority? = nil,
156180
description: TaskDescription,
157-
executionStateChangedCallback: (@Sendable (TaskExecutionState) async -> Void)?
181+
executionStateChangedCallback: (@Sendable (QueuedTask, TaskExecutionState) async -> Void)?
158182
) async {
159183
self._priority = .init(initialValue: priority?.rawValue ?? Task.currentPriority.rawValue)
160184
self.description = description
@@ -214,19 +238,21 @@ fileprivate actor QueuedTask<TaskDescription: TaskDescriptionProtocol> {
214238
}
215239
executionTask = task
216240
executionTaskCreatedContinuation.yield(task)
217-
await executionStateChangedCallback?(.executing)
241+
_isExecuting.value = true
242+
await executionStateChangedCallback?(self, .executing)
218243
return await task.value
219244
}
220245

221246
/// Implementation detail of `execute` that is called after `self.description.execute()` finishes.
222247
private func finalizeExecution() async -> ExecutionTaskFinishStatus {
223248
self.executionTask = nil
249+
_isExecuting.value = false
224250
if Task.isCancelled && self.cancelledToBeRescheduled.value {
225-
await executionStateChangedCallback?(.cancelledToBeRescheduled)
251+
await executionStateChangedCallback?(self, .cancelledToBeRescheduled)
226252
self.cancelledToBeRescheduled.value = false
227253
return ExecutionTaskFinishStatus.cancelledToBeRescheduled
228254
} else {
229-
await executionStateChangedCallback?(.finished)
255+
await executionStateChangedCallback?(self, .finished)
230256
return ExecutionTaskFinishStatus.terminated
231257
}
232258
}
@@ -327,8 +353,10 @@ public actor TaskScheduler<TaskDescription: TaskDescriptionProtocol> {
327353
public func schedule(
328354
priority: TaskPriority? = nil,
329355
_ taskDescription: TaskDescription,
330-
@_inheritActorContext executionStateChangedCallback: (@Sendable (TaskExecutionState) async -> Void)? = nil
331-
) async -> Task<Void, Never> {
356+
@_inheritActorContext executionStateChangedCallback: (
357+
@Sendable (QueuedTask<TaskDescription>, TaskExecutionState) async -> Void
358+
)? = nil
359+
) async -> QueuedTask<TaskDescription> {
332360
let queuedTask = await QueuedTask(
333361
priority: priority,
334362
description: taskDescription,
@@ -341,7 +369,7 @@ public actor TaskScheduler<TaskDescription: TaskDescriptionProtocol> {
341369
// queued task.
342370
await self.poke()
343371
}
344-
return queuedTask.resultTask
372+
return queuedTask
345373
}
346374

347375
/// Trigger all queued tasks to update their priority.

Sources/SKSupport/Sequence+AsyncMap.swift

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,4 +39,19 @@ extension Sequence {
3939

4040
return result
4141
}
42+
43+
/// Just like `Sequence.map` but allows an `async` transform function.
44+
public func asyncFilter(
45+
@_inheritActorContext _ predicate: @Sendable (Element) async throws -> Bool
46+
) async rethrows -> [Element] {
47+
var result: [Element] = []
48+
49+
for element in self {
50+
if try await predicate(element) {
51+
result.append(element)
52+
}
53+
}
54+
55+
return result
56+
}
4257
}

Sources/SemanticIndex/CMakeLists.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
add_library(SemanticIndex STATIC
33
CheckedIndex.swift
44
CompilerCommandLineOption.swift
5+
IndexStatusManager.swift
56
IndexTaskDescription.swift
67
PreparationTaskDescription.swift
78
SemanticIndexManager.swift
Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,70 @@
1+
//===----------------------------------------------------------------------===//
2+
//
3+
// This source file is part of the Swift.org open source project
4+
//
5+
// Copyright (c) 2014 - 2024 Apple Inc. and the Swift project authors
6+
// Licensed under Apache License v2.0 with Runtime Library Exception
7+
//
8+
// See https://swift.org/LICENSE.txt for license information
9+
// See https://swift.org/CONTRIBUTORS.txt for the list of Swift project authors
10+
//
11+
//===----------------------------------------------------------------------===//
12+
13+
import Foundation
14+
import SKCore
15+
16+
/// Keeps track of whether an item (a target or file to index) is up-to-date.
17+
actor IndexUpToDateStatusManager<Item: Hashable> {
18+
private enum Status {
19+
/// The item is up-to-date.
20+
case upToDate
21+
22+
/// The target or file has been marked out-of-date at the given date.
23+
///
24+
/// Keeping track of the date is necessary so that we don't mark a target as up-to-date if we have the following
25+
/// ordering of events:
26+
/// - Preparation started
27+
/// - Target marked out of date
28+
/// - Preparation finished
29+
case outOfDate(Date)
30+
}
31+
32+
private var status: [Item: Status] = [:]
33+
34+
/// Mark the target or file as up-to-date from a preparation/update-indexstore operation started at
35+
/// `updateOperationStartDate`.
36+
///
37+
/// See comment on `Status.outOfDate` why `updateOperationStartDate` needs to be passed.
38+
func markUpToDate(_ items: [Item], updateOperationStartDate: Date) {
39+
for item in items {
40+
switch status[item] {
41+
case .upToDate:
42+
break
43+
case .outOfDate(let markedOutOfDate):
44+
if markedOutOfDate < updateOperationStartDate {
45+
status[item] = .upToDate
46+
}
47+
case nil:
48+
status[item] = .upToDate
49+
}
50+
}
51+
}
52+
53+
func markOutOfDate(_ items: some Collection<Item>) {
54+
let date = Date()
55+
for item in items {
56+
status[item] = .outOfDate(date)
57+
}
58+
}
59+
60+
func markAllOutOfDate() {
61+
markOutOfDate(status.keys)
62+
}
63+
64+
func isUpToDate(_ item: Item) -> Bool {
65+
if case .upToDate = status[item] {
66+
return true
67+
}
68+
return false
69+
}
70+
}

Sources/SemanticIndex/PreparationTaskDescription.swift

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,8 @@ public struct PreparationTaskDescription: IndexTaskDescription {
3535
/// The build system manager that is used to get the toolchain and build settings for the files to index.
3636
private let buildSystemManager: BuildSystemManager
3737

38+
private let preparationUpToDateStatus: IndexUpToDateStatusManager<ConfiguredTarget>
39+
3840
/// Test hooks that should be called when the preparation task finishes.
3941
private let testHooks: IndexTestHooks
4042

@@ -54,10 +56,12 @@ public struct PreparationTaskDescription: IndexTaskDescription {
5456
init(
5557
targetsToPrepare: [ConfiguredTarget],
5658
buildSystemManager: BuildSystemManager,
59+
preparationUpToDateStatus: IndexUpToDateStatusManager<ConfiguredTarget>,
5760
testHooks: IndexTestHooks
5861
) {
5962
self.targetsToPrepare = targetsToPrepare
6063
self.buildSystemManager = buildSystemManager
64+
self.preparationUpToDateStatus = preparationUpToDateStatus
6165
self.testHooks = testHooks
6266
}
6367

@@ -66,17 +70,23 @@ public struct PreparationTaskDescription: IndexTaskDescription {
6670
// See comment in `withLoggingScope`.
6771
// The last 2 digits should be sufficient to differentiate between multiple concurrently running preparation operations
6872
await withLoggingScope("preparation-\(id % 100)") {
69-
let startDate = Date()
70-
let targetsToPrepare = targetsToPrepare.sorted(by: {
73+
let targetsToPrepare = await targetsToPrepare.asyncFilter {
74+
await !preparationUpToDateStatus.isUpToDate($0)
75+
}.sorted(by: {
7176
($0.targetID, $0.runDestinationID) < ($1.targetID, $1.runDestinationID)
7277
})
78+
if targetsToPrepare.isEmpty {
79+
return
80+
}
81+
7382
let targetsToPrepareDescription =
7483
targetsToPrepare
7584
.map { "\($0.targetID)-\($0.runDestinationID)" }
7685
.joined(separator: ", ")
7786
logger.log(
7887
"Starting preparation with priority \(Task.currentPriority.rawValue, privacy: .public): \(targetsToPrepareDescription)"
7988
)
89+
let startDate = Date()
8090
do {
8191
try await buildSystemManager.prepare(targets: targetsToPrepare)
8292
} catch {
@@ -85,6 +95,9 @@ public struct PreparationTaskDescription: IndexTaskDescription {
8595
)
8696
}
8797
await testHooks.preparationTaskDidFinish?(self)
98+
if !Task.isCancelled {
99+
await preparationUpToDateStatus.markUpToDate(targetsToPrepare, updateOperationStartDate: startDate)
100+
}
88101
logger.log(
89102
"Finished preparation in \(Date().timeIntervalSince(startDate) * 1000, privacy: .public)ms: \(targetsToPrepareDescription)"
90103
)

0 commit comments

Comments
 (0)