Skip to content

Task Executors: Prepare for new TaskExecutor protocol #69568

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 5 commits into from
Nov 2, 2023

Conversation

ktoso
Copy link
Contributor

@ktoso ktoso commented Nov 1, 2023

This prepares for the addition of a TaskExecutor protocol that will enable us to "run on the thread of the task executor, however be isolated to the actor".

Next we'll start tracking BOTH in executor tracking and adjust enqueue / actor drain code to respect these as well.

The APIs to set one of these are outlined in swiftlang/swift-evolution#2187 which details the withExecutor and Task(on: TaskExecutor) performance tuning APIs. They allow to "not switch off the task's preferred executor".

@ktoso ktoso force-pushed the wip-prepare-task-executor-protocol branch from b13c21c to 9e75142 Compare November 1, 2023 07:02
@ktoso ktoso added the concurrency Feature: umbrella label for concurrency language features label Nov 1, 2023
@ktoso
Copy link
Contributor Author

ktoso commented Nov 1, 2023

@swift-ci please smoke test

@ktoso ktoso requested a review from a team as a code owner November 1, 2023 09:40
@ktoso
Copy link
Contributor Author

ktoso commented Nov 1, 2023

@swift-ci please smoke test


/// Deprecated name for "SerialExecutorRef"; When it was first introduced it was ExecutorRef,
/// but it always meant specifically a serial executor.
typedef SerialExecutorRef ExecutorRef;
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we need this? Is there code using this header outside our repository?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Probably not, let me remove this.

@rokhinip I don't think you're using the actual header from Swift anywhere right? If anything it would have been a copy I expect.

/// reference.
BUILTIN_MISC_OPERATION_WITH_SILGEN(BuildOrdinaryTaskExecutorRef,
"buildOrdinaryTaskExecutorRef", "n", Special)

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't know that you need to copy this stuff about builtins. Let's wait until we have a need to build task executors into the compiler.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Okey yeah we can add when necessary, thanks!

@ktoso
Copy link
Contributor Author

ktoso commented Nov 2, 2023

@swift-ci please smoke test and merge

@ktoso
Copy link
Contributor Author

ktoso commented Nov 2, 2023

Addressed comments, this is just the rename and adding the new type then

@ktoso ktoso requested a review from rjmccall November 2, 2023 07:59
@ktoso ktoso changed the title Task Executors: Prepare for new TaskExecutor protocol & builtins Task Executors: Prepare for new TaskExecutor Nov 2, 2023
@ktoso ktoso changed the title Task Executors: Prepare for new TaskExecutor Task Executors: Prepare for new TaskExecutor protocol Nov 2, 2023
@swift-ci swift-ci merged commit 4363459 into swiftlang:main Nov 2, 2023
@ktoso ktoso deleted the wip-prepare-task-executor-protocol branch November 2, 2023 09:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
concurrency Feature: umbrella label for concurrency language features
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants