Skip to content

Commit eb982d5

Browse files
committed
Fix quadratic performance issue in AsyncQueue<Serial>
Adding an item to `AsyncQueue` was linear in the number of pending queue items, thus adding n items to an `AsyncQueue` before any can execute is in O(n^2). This decision was made intentionally because the primary use case for `AsyncQueue` was to track pending LSP requests, of which we don’t expect to have too many pending requests at any given time. While we can't fix the quadratic performance issue in general, we can resolve the quadratic issue of `AsyncQueue<Serial>` by making a new task only depend on the last item in the queue, which then transitively depends on all the previous items. `AsyncQueue<Serial>` are the queues that are most likely to contain many items. Fixes #1725 rdar://137886469
1 parent 6c5f5c9 commit eb982d5

File tree

4 files changed

+27
-13
lines changed

4 files changed

+27
-13
lines changed

Sources/BuildSystemIntegration/BuildSystemMessageDependencyTracker.swift

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ import BuildServerProtocol
1515
package import LanguageServerProtocol
1616
import LanguageServerProtocolExtensions
1717
import SKLogging
18-
import SwiftExtensions
18+
package import SwiftExtensions
1919
#else
2020
import BuildServerProtocol
2121
import LanguageServerProtocol
@@ -82,6 +82,10 @@ package enum BuildSystemMessageDependencyTracker: QueueBasedMessageHandlerDepend
8282
}
8383
}
8484

85+
package func dependencies(in pendingTasks: [PendingTask<Self>]) -> [PendingTask<Self>] {
86+
return pendingTasks.filter { $0.metadata.isDependency(of: self) }
87+
}
88+
8589
package init(_ request: some RequestType) {
8690
switch request {
8791
case is BuildShutdownRequest:

Sources/SourceKitLSP/MessageHandlingDependencyTracker.swift

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@
1414
package import LanguageServerProtocol
1515
import LanguageServerProtocolExtensions
1616
import SKLogging
17-
import SwiftExtensions
17+
package import SwiftExtensions
1818
#else
1919
import LanguageServerProtocol
2020
import LanguageServerProtocolExtensions
@@ -90,6 +90,10 @@ package enum MessageHandlingDependencyTracker: QueueBasedMessageHandlerDependenc
9090
}
9191
}
9292

93+
package func dependencies(in pendingTasks: [PendingTask<Self>]) -> [PendingTask<Self>] {
94+
return pendingTasks.filter { $0.metadata.isDependency(of: self) }
95+
}
96+
9397
package init(_ notification: some NotificationType) {
9498
switch notification {
9599
case is CancelRequestNotification:

Sources/SourceKitLSP/Swift/SyntacticTestIndex.swift

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,6 @@ fileprivate enum TaskMetadata: DependencyTracker, Equatable {
3737
case (.index(_), .read):
3838
// We require all index tasks scheduled before the read to be finished.
3939
// This ensures that the index has been updated at least to the state of file at which the read was scheduled.
40-
// Adding the dependency also elevates the index task's priorities.
4140
return true
4241
case (.index(let lhsUris), .index(let rhsUris)):
4342
// Technically, we should be able to allow simultaneous indexing of the same file. But conceptually the code
@@ -47,6 +46,10 @@ fileprivate enum TaskMetadata: DependencyTracker, Equatable {
4746
return !lhsUris.intersection(rhsUris).isEmpty
4847
}
4948
}
49+
50+
package func dependencies(in pendingTasks: [PendingTask<Self>]) -> [PendingTask<Self>] {
51+
return pendingTasks.filter { $0.metadata.isDependency(of: self) }
52+
}
5053
}
5154

5255
/// Data from a syntactic scan of a source file for tests.

Sources/SwiftExtensions/AsyncQueue.swift

Lines changed: 13 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -28,28 +28,31 @@ extension Task: AnyTask {
2828

2929
/// A type that is able to track dependencies between tasks.
3030
package protocol DependencyTracker: Sendable {
31-
/// Whether the task described by `self` needs to finish executing before
32-
/// `other` can start executing.
33-
func isDependency(of other: Self) -> Bool
31+
/// Which tasks need to finish before a task described by `self` may start executing.
32+
/// `pendingTasks` is sorted in the order in which the tasks were enqueued to `AsyncQueue`.
33+
func dependencies(in pendingTasks: [PendingTask<Self>]) -> [PendingTask<Self>]
3434
}
3535

3636
/// A dependency tracker where each task depends on every other, i.e. a serial
3737
/// queue.
3838
package struct Serial: DependencyTracker {
39-
package func isDependency(of other: Serial) -> Bool {
40-
return true
39+
package func dependencies(in pendingTasks: [PendingTask<Self>]) -> [PendingTask<Self>] {
40+
if let lastTask = pendingTasks.last {
41+
return [lastTask]
42+
}
43+
return []
4144
}
4245
}
4346

44-
private struct PendingTask<TaskMetadata: Sendable>: Sendable {
47+
package struct PendingTask<TaskMetadata: Sendable>: Sendable {
4548
/// The task that is pending.
46-
let task: any AnyTask
49+
fileprivate let task: any AnyTask
4750

48-
let metadata: TaskMetadata
51+
package let metadata: TaskMetadata
4952

5053
/// A unique value used to identify the task. This allows tasks to get
5154
/// removed from `pendingTasks` again after they finished executing.
52-
let id: UUID
55+
fileprivate let id: UUID
5356
}
5457

5558
/// A list of pending tasks that can be sent across actor boundaries and is guarded by a lock.
@@ -132,7 +135,7 @@ package final class AsyncQueue<TaskMetadata: DependencyTracker>: Sendable {
132135
return pendingTasks.withLock { tasks in
133136
// Build the list of tasks that need to finished execution before this one
134137
// can be executed
135-
let dependencies: [PendingTask] = tasks.filter { $0.metadata.isDependency(of: metadata) }
138+
let dependencies = metadata.dependencies(in: tasks)
136139

137140
// Schedule the task.
138141
let task = Task(priority: priority) { [pendingTasks] in

0 commit comments

Comments
 (0)