Skip to content

[TaskExecutors] Task initializer and withTaskExecutor parameter changes #70783

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 6 commits into from
Jan 20, 2024

Conversation

ktoso
Copy link
Contributor

@ktoso ktoso commented Jan 9, 2024

Follow up to #68793 bases on SE input.

This API revision allows passing (any TaskExecutor)? to all the new APIs. Passing nil as a preference means "no preference" which means that the parent's executor preference is used for structured concurrency, and the means no change in unstructured tasks but is a simple way to pass around an optional preference through APIs if necessary.

We also introduce a new global nonisolated(unsafe) var globalConcurrentExecutor: any TaskExecutor { get }.

Previous revision

Previous API shape required the nil value to be used as "no preference".

During review we came up with an idea that if we make the "default global concurrent executor" an accessible value, we can express it as: withTaskExecutor(.default) and therefore avoid the nil which can be a bit cryptic.

It does mean we have to add public final class DefaultConcurrentExecutor which delegates to the "default global executor" but the APIs can then use it.

The Task(on: .default) reads a bit weird now though. Perhaps we should change it to Task(executorPreference: .default), and do the same to the addTask(executorPreference: ...) and friends now that the .default can be passed?

@ktoso ktoso requested a review from kavon as a code owner January 9, 2024 06:25
@ktoso
Copy link
Contributor Author

ktoso commented Jan 9, 2024

@swift-ci please smoke test

@ktoso
Copy link
Contributor Author

ktoso commented Jan 9, 2024

Related proposal text swiftlang/swift-evolution#2271

@ktoso ktoso requested a review from DougGregor January 9, 2024 07:25
@ktoso ktoso added the concurrency runtime Feature: umbrella label for concurrency runtime features label Jan 9, 2024
@ktoso ktoso force-pushed the wip-cleanup-task-executor-parameters branch from 447b90e to 8db39ed Compare January 9, 2024 07:53
@ktoso
Copy link
Contributor Author

ktoso commented Jan 9, 2024

@swift-ci please smoke test

@ktoso ktoso force-pushed the wip-cleanup-task-executor-parameters branch from 8db39ed to 025b21a Compare January 15, 2024 04:35
@ktoso ktoso force-pushed the wip-cleanup-task-executor-parameters branch from 025b21a to 7152811 Compare January 15, 2024 04:36
}
// TODO: introduce a set {} once we are ready to allow customizing the
// default global executor. This should be done the same for main actor
}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

so here's the new global @DougGregor

@@ -121,7 +121,7 @@ public struct UnownedJob: Sendable {
@available(SwiftStdlib 9999, *)
@_alwaysEmitIntoClient
@inlinable
public func runSynchronously(isolated serialExecutor: UnownedSerialExecutor,
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

aligning this with some discussions on forums in other threads about "isolated" parameters.

FYI @rjmccall @hborla

(this is new API coming in for the TaskExecutors)

@ktoso
Copy link
Contributor Author

ktoso commented Jan 15, 2024

@swift-ci please smoke test

1 similar comment
@ktoso
Copy link
Contributor Author

ktoso commented Jan 16, 2024

@swift-ci please smoke test

@ktoso ktoso merged commit 1dec00a into swiftlang:main Jan 20, 2024
carlos4242 pushed a commit to carlos4242/swift that referenced this pull request May 31, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
concurrency runtime Feature: umbrella label for concurrency runtime features
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant