Skip to content

Merge main into release/6.0 #1364

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 35 commits into from
May 29, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
35 commits
Select commit Hold shift + click to select a range
5fffb0a
Watch for changes to source files
ahoppen May 23, 2024
8a413d2
Add some logging to `TaskScheduler` again
ahoppen May 22, 2024
83b8be8
Add a `withTaskPriorityChangedHandler` function and use it in `TaskSc…
ahoppen May 23, 2024
cb24ce9
Skip `testTasksWithElevatedPrioritiesGetExecutedFirst` if the host OS…
ahoppen May 23, 2024
7cf9f15
Merge pull request #1340 from ahoppen/watch-for-source-file-changes
ahoppen May 24, 2024
4ad6eb3
Ensure that the indexstore-db is up-to-date before deciding which sou…
ahoppen May 24, 2024
88de421
Reset the state of `WorkDoneProgressState` when the progress has ended
ahoppen May 24, 2024
8cb6136
On macOS, reveal diagnostic bundle in Finder after it has been created
ahoppen May 24, 2024
cdc4f2b
Set `cancelBuilds` to 0 on close instead of open
ahoppen May 24, 2024
7fbb39e
Merge pull request #1344 from ahoppen/update-indexstore-db-before-dec…
ahoppen May 24, 2024
31a8cf3
Merge pull request #1345 from ahoppen/reset-workdoneprogress-state
ahoppen May 24, 2024
1ea20eb
Show a work done progress while the semantic index is rebuilding a bu…
ahoppen May 24, 2024
eb784eb
Log when the build graph starts being generated and when it finishes
ahoppen May 24, 2024
6103661
Merge pull request #1346 from ahoppen/reveal-diag-bundle
ahoppen May 24, 2024
ea6a585
Merge pull request #1335 from ahoppen/task-scheduler-logging
ahoppen May 24, 2024
193ab77
Don’t re-index files in languages that cannot be indexed
ahoppen May 25, 2024
2b04ad6
Merge pull request #1347 from ahoppen/cancel-builds-0-on-close
ahoppen May 25, 2024
069e8a2
Call `indexProgressManager.indexStatusDidChange` when workspaces in t…
ahoppen May 24, 2024
84007ca
Merge pull request #1343 from ahoppen/generating-build-graph-logging
ahoppen May 25, 2024
ecacd7b
Show work done progress while a source file is being prepared for edi…
ahoppen May 25, 2024
de4474f
A couple minor priority elevation bug fixes in `TaskScheduler`
ahoppen May 25, 2024
5d75e14
Merge pull request #1352 from ahoppen/dont-reindex
ahoppen May 25, 2024
4f91a98
Split SourceKitLSPServer.swift into multiple files
ahoppen May 25, 2024
3ae6501
Time out updating the index store after 2 minutes
ahoppen May 25, 2024
05dd102
Wait for target to be prepared before returning diagnostics
ahoppen May 25, 2024
85cdd90
Merge pull request #1357 from ahoppen/task-scheduler-improvements
ahoppen May 28, 2024
aa356cb
Merge pull request #1359 from ahoppen/split-sourcekitlspserver
ahoppen May 28, 2024
24dfbd5
Merge pull request #1356 from ahoppen/progress-indicator-current-file…
ahoppen May 28, 2024
a703b72
Merge pull request #1354 from ahoppen/timeout-indexstore-update
ahoppen May 28, 2024
f85d821
Don’t cause any file system file effects when trying to find an impli…
ahoppen May 24, 2024
a29f1ac
Merge pull request #1355 from ahoppen/wait-for-preparation-before-diags
ahoppen May 28, 2024
8765bb1
Merge pull request #1350 from ahoppen/no-side-effects-when-searching-…
ahoppen May 28, 2024
92ed134
Fix a test build issue due to PRs racing
ahoppen May 28, 2024
3356873
Merge pull request #1363 from ahoppen/fix-build-issue
hamishknight May 28, 2024
bac062f
Merge branch 'main' into 6.0/merge-main-2024-05-28
ahoppen May 28, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions Package.swift
Original file line number Diff line number Diff line change
Expand Up @@ -367,6 +367,7 @@ let package = Package(
name: "SourceKitLSPTests",
dependencies: [
"BuildServerProtocol",
"CAtomics",
"LSPLogging",
"LSPTestSupport",
"LanguageServerProtocol",
Expand Down
9 changes: 9 additions & 0 deletions Sources/Diagnose/DiagnoseCommand.swift
Original file line number Diff line number Diff line change
Expand Up @@ -352,6 +352,15 @@ public struct DiagnoseCommand: AsyncParsableCommand {
"""
)

#if os(macOS)
// Reveal the bundle in Finder on macOS
do {
let process = try Process.launch(arguments: ["open", "-R", bundlePath.path], workingDirectory: nil)
try await process.waitUntilExitSendingSigIntOnTaskCancellation()
} catch {
// If revealing the bundle in Finder should fail, we don't care. We still printed the bundle path to stdout.
}
#endif
}

@MainActor
Expand Down
2 changes: 1 addition & 1 deletion Sources/SKCore/BuildServerBuildSystem.swift
Original file line number Diff line number Diff line change
Expand Up @@ -279,7 +279,7 @@ extension BuildServerBuildSystem: BuildSystem {
return [ConfiguredTarget(targetID: "dummy", runDestinationID: "dummy")]
}

public func generateBuildGraph() {}
public func generateBuildGraph(allowFileSystemWrites: Bool) {}

public func topologicalSort(of targets: [ConfiguredTarget]) async -> [ConfiguredTarget]? {
return nil
Expand Down
11 changes: 8 additions & 3 deletions Sources/SKCore/BuildSystem.swift
Original file line number Diff line number Diff line change
Expand Up @@ -135,9 +135,14 @@ public protocol BuildSystem: AnyObject, Sendable {
/// Return the list of targets and run destinations that the given document can be built for.
func configuredTargets(for document: DocumentURI) async -> [ConfiguredTarget]

/// Re-generate the build graph including all the tasks that are necessary for building the entire build graph, like
/// resolving package versions.
func generateBuildGraph() async throws
/// Re-generate the build graph.
///
/// If `allowFileSystemWrites` is `true`, this should include all the tasks that are necessary for building the entire
/// build graph, like resolving package versions.
///
/// If `allowFileSystemWrites` is `false`, no files must be written to disk. This mode is used to determine whether
/// the build system can handle a source file, and decide whether a workspace should be opened with this build system
func generateBuildGraph(allowFileSystemWrites: Bool) async throws

/// Sort the targets so that low-level targets occur before high-level targets.
///
Expand Down
4 changes: 2 additions & 2 deletions Sources/SKCore/BuildSystemManager.swift
Original file line number Diff line number Diff line change
Expand Up @@ -219,8 +219,8 @@ extension BuildSystemManager {
return settings
}

public func generateBuildGraph() async throws {
try await self.buildSystem?.generateBuildGraph()
public func generateBuildGraph(allowFileSystemWrites: Bool) async throws {
try await self.buildSystem?.generateBuildGraph(allowFileSystemWrites: allowFileSystemWrites)
}

public func topologicalSort(of targets: [ConfiguredTarget]) async throws -> [ConfiguredTarget]? {
Expand Down
2 changes: 1 addition & 1 deletion Sources/SKCore/CompilationDatabaseBuildSystem.swift
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,7 @@ extension CompilationDatabaseBuildSystem: BuildSystem {
throw PrepareNotSupportedError()
}

public func generateBuildGraph() {}
public func generateBuildGraph(allowFileSystemWrites: Bool) {}

public func topologicalSort(of targets: [ConfiguredTarget]) -> [ConfiguredTarget]? {
return nil
Expand Down
122 changes: 53 additions & 69 deletions Sources/SKCore/TaskScheduler.swift
Original file line number Diff line number Diff line change
Expand Up @@ -13,13 +13,16 @@
import CAtomics
import Foundation
import LSPLogging
import SKSupport

/// See comment on ``TaskDescriptionProtocol/dependencies(to:taskPriority:)``
public enum TaskDependencyAction<TaskDescription: TaskDescriptionProtocol> {
case waitAndElevatePriorityOfDependency(TaskDescription)
case cancelAndRescheduleDependency(TaskDescription)
}

private let taskSchedulerSubsystem = "org.swift.sourcekit-lsp.task-scheduler"

public protocol TaskDescriptionProtocol: Identifiable, Sendable, CustomLogStringConvertible {
/// Execute the task.
///
Expand Down Expand Up @@ -123,10 +126,6 @@ public actor QueuedTask<TaskDescription: TaskDescriptionProtocol> {
/// Every time `execute` gets called, a new task is placed in this continuation. See comment on `executionTask`.
private let executionTaskCreatedContinuation: AsyncStream<Task<ExecutionTaskFinishStatus, Never>>.Continuation

/// Placing a new value in this continuation will cause `resultTask` to query its priority and set
/// `QueuedTask.priority`.
private let updatePriorityContinuation: AsyncStream<Void>.Continuation

nonisolated(unsafe) private var _priority: AtomicUInt8

/// The latest known priority of the task.
Expand Down Expand Up @@ -183,20 +182,14 @@ public actor QueuedTask<TaskDescription: TaskDescriptionProtocol> {
private let executionStateChangedCallback: (@Sendable (QueuedTask, TaskExecutionState) async -> Void)?

init(
priority: TaskPriority? = nil,
priority: TaskPriority,
description: TaskDescription,
executionStateChangedCallback: (@Sendable (QueuedTask, TaskExecutionState) async -> Void)?
) async {
self._priority = .init(initialValue: priority?.rawValue ?? Task.currentPriority.rawValue)
self._priority = AtomicUInt8(initialValue: priority.rawValue)
self.description = description
self.executionStateChangedCallback = executionStateChangedCallback

var updatePriorityContinuation: AsyncStream<Void>.Continuation!
let updatePriorityStream = AsyncStream {
updatePriorityContinuation = $0
}
self.updatePriorityContinuation = updatePriorityContinuation

var executionTaskCreatedContinuation: AsyncStream<Task<ExecutionTaskFinishStatus, Never>>.Continuation!
let executionTaskCreatedStream = AsyncStream {
executionTaskCreatedContinuation = $0
Expand All @@ -205,31 +198,24 @@ public actor QueuedTask<TaskDescription: TaskDescriptionProtocol> {

self.resultTask = Task.detached(priority: priority) {
await withTaskCancellationHandler {
await withTaskGroup(of: Void.self) { taskGroup in
taskGroup.addTask {
for await _ in updatePriorityStream {
self.priority = Task.currentPriority
}
}
taskGroup.addTask {
for await task in executionTaskCreatedStream {
switch await task.valuePropagatingCancellation {
case .cancelledToBeRescheduled:
// Break the switch and wait for a new `executionTask` to be placed into `executionTaskCreatedStream`.
break
case .terminated:
// The task finished. We are done with this `QueuedTask`
return
}
await withTaskPriorityChangedHandler(initialPriority: self.priority) {
for await task in executionTaskCreatedStream {
switch await task.valuePropagatingCancellation {
case .cancelledToBeRescheduled:
// Break the switch and wait for a new `executionTask` to be placed into `executionTaskCreatedStream`.
break
case .terminated:
// The task finished. We are done with this `QueuedTask`
return
}
}
// The first (update priority) task never finishes, so this waits for the second (wait for execution) task
// to terminate.
// Afterwards we also cancel the update priority task.
for await _ in taskGroup {
taskGroup.cancelAll()
return
} taskPriorityChanged: {
withLoggingSubsystemAndScope(subsystem: taskSchedulerSubsystem, scope: nil) {
logger.debug(
"Updating priority of \(self.description.forLogging) from \(self.priority.rawValue) to \(Task.currentPriority.rawValue)"
)
}
self.priority = Task.currentPriority
}
} onCancel: {
self.resultTaskCancelled.value = true
Expand Down Expand Up @@ -282,20 +268,19 @@ public actor QueuedTask<TaskDescription: TaskDescriptionProtocol> {
self.executionTask = nil
}

/// Trigger `QueuedTask.priority` to be updated with the current priority of the underlying task.
///
/// This is an asynchronous operation that makes no guarantees when the updated priority will be available.
///
/// This is needed because tasks can't subscribe to priority updates (ie. there is no `withPriorityHandler` similar to
/// `withCancellationHandler`, https://github.com/apple/swift/issues/73367).
func triggerPriorityUpdate() {
updatePriorityContinuation.yield()
}

/// If the priority of this task is less than `targetPriority`, elevate the priority to `targetPriority` by spawning
/// a new task that depends on it. Otherwise a no-op.
nonisolated func elevatePriority(to targetPriority: TaskPriority) {
if priority < targetPriority {
withLoggingSubsystemAndScope(subsystem: taskSchedulerSubsystem, scope: nil) {
logger.debug(
"Elevating priority of \(self.description.forLogging) from \(self.priority.rawValue) to \(targetPriority.rawValue)"
)
}
// Awaiting the result task from a higher-priority task will eventually update `priority` through
// `withTaskPriorityChangedHandler` but that might take a while because `withTaskPriorityChangedHandler` polls.
// Since we know that the priority will be elevated, set it now. That way we don't try to elevate it again.
self.priority = targetPriority
Task(priority: targetPriority) {
await self.resultTask.value
}
Expand Down Expand Up @@ -371,7 +356,7 @@ public actor TaskScheduler<TaskDescription: TaskDescriptionProtocol> {
)? = nil
) async -> QueuedTask<TaskDescription> {
let queuedTask = await QueuedTask(
priority: priority,
priority: priority ?? Task.currentPriority,
description: taskDescription,
executionStateChangedCallback: executionStateChangedCallback
)
Expand All @@ -385,16 +370,6 @@ public actor TaskScheduler<TaskDescription: TaskDescriptionProtocol> {
return queuedTask
}

/// Trigger all queued tasks to update their priority.
///
/// Should be called occasionally to elevate tasks in the queue whose underlying `Swift.Task` had their priority
/// elevated because a higher-priority task started depending on them.
private func triggerPriorityUpdateOfQueuedTasks() async {
for task in pendingTasks {
await task.triggerPriorityUpdate()
}
}

/// Returns the maximum number of concurrent tasks that are allowed to execute at the given priority.
private func maxConcurrentTasks(at priority: TaskPriority) -> Int {
for (atPriority, maxConcurrentTasks) in maxConcurrentTasksByPriority {
Expand All @@ -417,9 +392,8 @@ public actor TaskScheduler<TaskDescription: TaskDescriptionProtocol> {
{
// We don't have any execution slots left. Thus, this poker has nothing to do and is done.
// When the next task finishes, it calls `poke` again.
// If the low priority task's priority gets elevated, that will be picked up when the next task in the
// `TaskScheduler` finishes, which causes `triggerPriorityUpdateOfQueuedTasks` to be called, which transfers
// the new elevated priority to `QueuedTask.priority` and which can then be picked up by the next `poke` call.
// If the low priority task's priority gets elevated that task's priority will get elevated and it will be
// picked up on the next `poke` call.
return
}
let dependencies = task.description.dependencies(to: currentlyExecutingTasks.map(\.description))
Expand All @@ -428,13 +402,17 @@ public actor TaskScheduler<TaskDescription: TaskDescriptionProtocol> {
case .cancelAndRescheduleDependency(let taskDescription):
guard let dependency = self.currentlyExecutingTasks.first(where: { $0.description.id == taskDescription.id })
else {
logger.fault(
"Cannot find task to wait for \(taskDescription.forLogging) in list of currently executing tasks"
)
withLoggingSubsystemAndScope(subsystem: taskSchedulerSubsystem, scope: nil) {
logger.fault(
"Cannot find task to wait for \(taskDescription.forLogging) in list of currently executing tasks"
)
}
return nil
}
if !taskDescription.isIdempotent {
logger.fault("Cannot reschedule task '\(taskDescription.forLogging)' since it is not idempotent")
withLoggingSubsystemAndScope(subsystem: taskSchedulerSubsystem, scope: nil) {
logger.fault("Cannot reschedule task '\(taskDescription.forLogging)' since it is not idempotent")
}
return dependency
}
if dependency.priority > task.priority {
Expand All @@ -445,9 +423,11 @@ public actor TaskScheduler<TaskDescription: TaskDescriptionProtocol> {
case .waitAndElevatePriorityOfDependency(let taskDescription):
guard let dependency = self.currentlyExecutingTasks.first(where: { $0.description.id == taskDescription.id })
else {
logger.fault(
"Cannot find task to wait for '\(taskDescription.forLogging)' in list of currently executing tasks"
)
withLoggingSubsystemAndScope(subsystem: taskSchedulerSubsystem, scope: nil) {
logger.fault(
"Cannot find task to wait for '\(taskDescription.forLogging)' in list of currently executing tasks"
)
}
return nil
}
return dependency
Expand All @@ -465,9 +445,11 @@ public actor TaskScheduler<TaskDescription: TaskDescriptionProtocol> {
switch taskDependency {
case .cancelAndRescheduleDependency(let taskDescription):
guard let task = self.currentlyExecutingTasks.first(where: { $0.description.id == taskDescription.id }) else {
logger.fault(
"Cannot find task to reschedule \(taskDescription.forLogging) in list of currently executing tasks"
)
withLoggingSubsystemAndScope(subsystem: taskSchedulerSubsystem, scope: nil) {
logger.fault(
"Cannot find task to reschedule \(taskDescription.forLogging) in list of currently executing tasks"
)
}
return nil
}
return task
Expand All @@ -478,6 +460,9 @@ public actor TaskScheduler<TaskDescription: TaskDescriptionProtocol> {
if !rescheduleTasks.isEmpty {
Task.detached(priority: task.priority) {
for task in rescheduleTasks {
withLoggingSubsystemAndScope(subsystem: taskSchedulerSubsystem, scope: nil) {
logger.debug("Suspending \(task.description.forLogging)")
}
await task.cancelToBeRescheduled()
}
}
Expand Down Expand Up @@ -510,7 +495,6 @@ public actor TaskScheduler<TaskDescription: TaskDescriptionProtocol> {
case .terminated: break
case .cancelledToBeRescheduled: pendingTasks.append(task)
}
await self.triggerPriorityUpdateOfQueuedTasks()
self.poke()
}
}
Expand Down
25 changes: 25 additions & 0 deletions Sources/SKSupport/AsyncUtils.swift
Original file line number Diff line number Diff line change
Expand Up @@ -166,3 +166,28 @@ extension Collection where Element: Sendable {
}
}
}

public struct TimeoutError: Error, CustomStringConvertible {
public var description: String { "Timed out" }
}

/// Executes `body`. If it doesn't finish after `duration`, throws a `TimeoutError`.
public func withTimeout<T: Sendable>(
_ duration: Duration,
_ body: @escaping @Sendable () async throws -> T
) async throws -> T {
try await withThrowingTaskGroup(of: T.self) { taskGroup in
taskGroup.addTask {
try await Task.sleep(for: duration)
throw TimeoutError()
}
taskGroup.addTask {
return try await body()
}
for try await value in taskGroup {
taskGroup.cancelAll()
return value
}
throw CancellationError()
}
}
1 change: 1 addition & 0 deletions Sources/SKSupport/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ add_library(SKSupport STATIC
Result.swift
Sequence+AsyncMap.swift
SwitchableProcessResultExitStatus.swift
Task+WithPriorityChangedHandler.swift
ThreadSafeBox.swift
WorkspaceType.swift
)
Expand Down
Loading