Skip to content

Commit f5fee4e

Browse files
committed
Rewrite how “Preparing current file” is differentiated from the generic “Preparing targets”
The old implementation of inspecting the `InProgressPrepareForEditorTask` has a few race conditions including the fact that we would start the preparation task before the `inProgressPrepareForEditorTask` was set. Instead, keep track of the purpose for each preparation task. This ends up simplifying the code as well.
1 parent 518aac4 commit f5fee4e

File tree

1 file changed

+42
-40
lines changed

1 file changed

+42
-40
lines changed

Sources/SemanticIndex/SemanticIndexManager.swift

Lines changed: 42 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -98,10 +98,6 @@ public enum IndexProgressStatus {
9898

9999
/// See `SemanticIndexManager.inProgressPrepareForEditorTask`.
100100
fileprivate struct InProgressPrepareForEditorTask {
101-
fileprivate enum State {
102-
case determiningCanonicalConfiguredTarget
103-
case preparingTarget
104-
}
105101
/// A unique ID that identifies the preparation task and is used to set
106102
/// `SemanticIndexManager.inProgressPrepareForEditorTask` to `nil` when the current in progress task finishes.
107103
let id: UUID
@@ -111,9 +107,21 @@ fileprivate struct InProgressPrepareForEditorTask {
111107

112108
/// The task that prepares the document. Should never be awaited and only be used to cancel the task.
113109
let task: Task<Void, Never>
110+
}
111+
112+
/// The reason why a target is being prepared. This is used to determine the `IndexProgressStatus`.
113+
fileprivate enum TargetPreparationPurpose: Comparable {
114+
/// We are preparing the target so we can index files in it.
115+
case forIndexing
116+
117+
/// We are preparing the target to provide semantic functionality in one of its files.
118+
case forEditorFunctionality
119+
}
114120

115-
/// Whether the task is currently determining the file's target or actually preparing the document.
116-
var state: State
121+
/// An entry in `SemanticIndexManager.inProgressPreparationTasks`.
122+
fileprivate struct InProgressPreparationTask {
123+
let task: OpaqueQueuedIndexTask
124+
let purpose: TargetPreparationPurpose
117125
}
118126

119127
/// Schedules index tasks and keeps track of the index status of files.
@@ -139,7 +147,7 @@ public final actor SemanticIndexManager {
139147
/// executing.
140148
///
141149
/// After a preparation task finishes, it is removed from this dictionary.
142-
private var inProgressPreparationTasks: [ConfiguredTarget: OpaqueQueuedIndexTask] = [:]
150+
private var inProgressPreparationTasks: [ConfiguredTarget: InProgressPreparationTask] = [:]
143151

144152
/// The files that are currently being index, either waiting for their target to be prepared, waiting for the index
145153
/// store update task to be scheduled in the task scheduler or which currently have an index store update running.
@@ -175,14 +183,14 @@ public final actor SemanticIndexManager {
175183

176184
/// A summary of the tasks that this `SemanticIndexManager` has currently scheduled or is currently indexing.
177185
public var progressStatus: IndexProgressStatus {
178-
if let inProgressPrepareForEditorTask, inProgressPrepareForEditorTask.state == .preparingTarget {
186+
if inProgressPreparationTasks.values.contains(where: { $0.purpose == .forEditorFunctionality }) {
179187
return .preparingFileForEditorFunctionality
180188
}
181189
if generateBuildGraphTask != nil {
182190
return .generatingBuildGraph
183191
}
184-
let preparationTasks = inProgressPreparationTasks.mapValues { queuedTask in
185-
return queuedTask.isExecuting ? IndexTaskStatus.executing : IndexTaskStatus.scheduled
192+
let preparationTasks = inProgressPreparationTasks.mapValues { inProgressTask in
193+
return inProgressTask.task.isExecuting ? IndexTaskStatus.executing : IndexTaskStatus.scheduled
186194
}
187195
let indexTasks = inProgressIndexTasks.mapValues { status in
188196
switch status {
@@ -385,38 +393,18 @@ public final actor SemanticIndexManager {
385393
let id = UUID()
386394
let task = Task(priority: priority) {
387395
await withLoggingScope("preparation") {
388-
defer {
389-
if inProgressPrepareForEditorTask?.id == id {
390-
inProgressPrepareForEditorTask = nil
391-
self.indexProgressStatusDidChange()
392-
}
393-
}
394-
// Should be kept in sync with `prepareFileForEditorFunctionality`
395-
guard let target = await buildSystemManager.canonicalConfiguredTarget(for: uri) else {
396-
return
397-
}
398-
if Task.isCancelled {
399-
return
400-
}
401-
if await preparationUpToDateTracker.isUpToDate(target) {
402-
// If the target is up-to-date, there is nothing to prepare.
403-
return
404-
}
396+
await prepareFileForEditorFunctionality(uri)
405397
if inProgressPrepareForEditorTask?.id == id {
406-
if inProgressPrepareForEditorTask?.state != .determiningCanonicalConfiguredTarget {
407-
logger.fault("inProgressPrepareForEditorTask is in unexpected state")
408-
}
409-
inProgressPrepareForEditorTask?.state = .preparingTarget
398+
inProgressPrepareForEditorTask = nil
399+
self.indexProgressStatusDidChange()
410400
}
411-
await self.prepare(targets: [target], priority: nil)
412401
}
413402
}
414403
inProgressPrepareForEditorTask?.task.cancel()
415404
inProgressPrepareForEditorTask = InProgressPrepareForEditorTask(
416405
id: id,
417406
document: uri,
418-
task: task,
419-
state: .determiningCanonicalConfiguredTarget
407+
task: task
420408
)
421409
self.indexProgressStatusDidChange()
422410
}
@@ -433,13 +421,16 @@ public final actor SemanticIndexManager {
433421
if Task.isCancelled {
434422
return
435423
}
436-
await self.prepare(targets: [target], priority: nil)
424+
if await preparationUpToDateTracker.isUpToDate(target) {
425+
// If the target is up-to-date, there is nothing to prepare.
426+
return
427+
}
428+
await self.prepare(targets: [target], purpose: .forEditorFunctionality, priority: nil)
437429
}
438430

439431
// MARK: - Helper functions
440432

441-
/// Prepare the given targets for indexing.
442-
private func prepare(targets: [ConfiguredTarget], priority: TaskPriority?) async {
433+
private func prepare(targets: [ConfiguredTarget], purpose: TargetPreparationPurpose, priority: TaskPriority?) async {
443434
// Perform a quick initial check whether the target is up-to-date, in which case we don't need to schedule a
444435
// preparation operation at all.
445436
// We will check the up-to-date status again in `PreparationTaskDescription.execute`. This ensures that if we
@@ -471,14 +462,25 @@ public final actor SemanticIndexManager {
471462
return
472463
}
473464
for target in targetsToPrepare {
474-
if self.inProgressPreparationTasks[target] == OpaqueQueuedIndexTask(task) {
465+
if self.inProgressPreparationTasks[target]?.task == OpaqueQueuedIndexTask(task) {
475466
self.inProgressPreparationTasks[target] = nil
476467
}
477468
}
478469
self.indexProgressStatusDidChange()
479470
}
480471
for target in targetsToPrepare {
481-
inProgressPreparationTasks[target] = OpaqueQueuedIndexTask(preparationTask)
472+
// If we are preparing the same target for indexing and editor functionality, pick editor functionality as the
473+
// purpose because it is more significant.
474+
let mergedPurpose =
475+
if let existingPurpose = inProgressPreparationTasks[target]?.purpose {
476+
max(existingPurpose, purpose)
477+
} else {
478+
purpose
479+
}
480+
inProgressPreparationTasks[target] = InProgressPreparationTask(
481+
task: OpaqueQueuedIndexTask(preparationTask),
482+
purpose: mergedPurpose
483+
)
482484
}
483485
await withTaskCancellationHandler {
484486
return await preparationTask.waitToFinish()
@@ -603,7 +605,7 @@ public final actor SemanticIndexManager {
603605
let preparationTaskID = UUID()
604606
let indexTask = Task(priority: priority) {
605607
// First prepare the targets.
606-
await prepare(targets: targetsBatch, priority: priority)
608+
await prepare(targets: targetsBatch, purpose: .forIndexing, priority: priority)
607609

608610
// And after preparation is done, index the files in the targets.
609611
await withTaskGroup(of: Void.self) { taskGroup in

0 commit comments

Comments
 (0)