Initial Task Executor implementation Task(on:), addTask(on:) etc. #68793

ktoso · 2023-09-27T05:28:54Z

Pending proposal and Swift Evolution process, do not merge yet.

This generally will allow tasks to be launched/spawned immediately on a specific executor or actor, and handle all the associated isolation.

Resolves rdar://107072140

test/Concurrency/Runtime/async_custom_task_executor.swift

ktoso · 2023-11-10T08:47:23Z

@swift-ci please build toolchain

ktoso · 2023-11-13T22:54:47Z

@swift-ci please build toolchain macOS

ktoso · 2023-11-14T14:30:38Z

@swift-ci please smoke test

ktoso · 2023-11-14T14:30:44Z

@swift-ci please build toolchain macOS

ktoso · 2023-11-17T09:40:36Z

@swift-ci please smoke test

rjmccall

Leaving off there for now

include/swift/ABI/MetadataValues.h

include/swift/ABI/Task.h

rjmccall · 2023-11-17T22:45:12Z

include/swift/ABI/Task.h

+  /// setting it during creation (`Task(on:...)`).
+  ///
+  /// This means that during task tear down the record should be deallocated
+  /// because it was not set with a


Another unfinished comme

Thanks, fixed in 63c8f01 and looked at all other comments as well.

include/swift/ABI/TaskOptions.h

include/swift/ABI/TaskStatus.h

rjmccall · 2023-11-17T22:49:28Z

include/swift/AST/Builtins.def

+/// Build a Builtin.Executor value from an "ordinary" task executor
+/// reference.
+BUILTIN_MISC_OPERATION_WITH_SILGEN(BuildOrdinaryTaskExecutorRef,
+    "buildOrdinaryTaskExecutorRef", "n", Special)


Did you end up needing these builtins for some reason?

edit; Yes we need those because:

struct UnownedTaskExecutor { // ... @inlinable public init<E: TaskExecutor>(ordinary executor: __shared E) { // ... self.executor = Builtin.buildOrdinaryTaskExecutorRef(executor)

which is different from the UnownedSerialExecutor; even though they both just store the "...Ref" style reference that's opaque in Swift. If we had different flags for Task executor than for Serial executor or something this entry points matters.

Github seems to have lost my comment -- yes, we need these because:

struct UnownedTaskExecutor { @inlinable public init<E: TaskExecutor>(ordinary executor: __shared E) { #if $BuiltinBuildTaskExecutor self.executor = Builtin.buildOrdinaryTaskExecutorRef(executor) #else fatalError("Swift compiler is incompatible with this SDK version") #endif } }

We arguably could "wing it" and use the buildOrdinaryExecutorRef though I'm unsure about the proliferation of pretending as if they're the same... Internally the representation is the same though, that is true -- the memory layout of SerialExecutorRef and TaskExecutorRef is the same, at least currently.

rjmccall · 2023-11-17T22:51:43Z

include/swift/Runtime/Concurrency.h

@@ -900,6 +918,20 @@ SerialExecutorRef swift_task_getCurrentExecutor(void);
 SWIFT_EXPORT_FROM(swift_Concurrency) SWIFT_CC(swift)
 SerialExecutorRef swift_task_getMainExecutor(void);

+/// Return the generic (default global concurrent) executor reference.
+SWIFT_EXPORT_FROM(swift_Concurrency) SWIFT_CC(swift)
+SerialExecutorRef swift_task_getGenericExecutor(void);


If you're adding these new functions to the runtime ABI, please note which Swift release added them.

But this function in particular I think we don't need — there's an ABI-guaranteed way of constructing this already (IIRC it's the all-zeroes value).

Okey, yeah I wasn't sure if we want to hardcode this assumption here -- you are right that it is guaranteed currently to be a {0, 0} basically (SerialExecutorRef), so okey if we want to assume this I removed the function and replaced it with:

@available(SwiftStdlib 9999, *) @usableFromInline internal func _getGenericSerialExecutor() -> Builtin.Executor { // The `SerialExecutorRef` to "default generic executor" is guaranteed // to be a zero value; // // As the runtime relies on this in multiple places, // so instead of a runtime call to get this executor ref, we bitcast a "zero" // of expected size to the builtin executor type. unsafeBitCast((UInt(0), UInt(0)), to: Builtin.Executor.self) }

Is that the right UInt to use here...? In C++ it is:

HeapObject *Identity; // Not necessarily Swift reference-countable uintptr_t Implementation;

I did the same for _getUndefinedTaskExecutor as well, to avoid runtime calls: 1318d3e

rjmccall · 2023-11-17T22:52:34Z

include/swift/Runtime/Concurrency.h

+/// It can be used used to store a record indicating that
+/// there is no task executor preference in the current scope.
+SWIFT_EXPORT_FROM(swift_Concurrency) SWIFT_CC(swift)
+TaskExecutorRef swift_task_getUndefinedTaskExecutor(void);


Similarly, I don't think we need a function for this, because we really want this to be guaranteed to be the all-zeroes value.

Okey gotcha so I'll just bitcast zeroes and avoid the runtime call. Thanks!

Done, and removed the runtime func -- explained more here: #68793 (comment)

rjmccall · 2023-11-17T22:53:15Z

include/swift/Runtime/Concurrency.h

+TaskExecutorRef swift_task_getUndefinedTaskExecutor(void);
+
+SWIFT_EXPORT_FROM(swift_Concurrency) SWIFT_CC(swift)
+TaskExecutorRef swift_task_getPreferredTaskExecutor(void);


This really needs documentation explaining the constraints on how it can be used.

Added in: 10cadfa I think that covers the "be careful" side of things. Anything more we should add?

These were added in earlier commits of the initial task executor work, however they are not necessary after all -- we get the right hop behavior just from the task executor stored and picked up in runtime functions already.

ktoso · 2023-12-11T11:00:58Z

Added _ to all APIs exposing the new features.
added Task._detached(on:) that we missed earlier.

ktoso · 2023-12-11T11:01:26Z

@swift-ci please smoke test

ktoso · 2023-12-12T01:33:23Z

@swift-ci please smoke test macOS

…iftlang#68793) Co-authored-by: John McCall <[email protected]>

ktoso requested review from kavon, hborla, slavapestov, xedin and tshortli as code owners September 27, 2023 05:28

ktoso commented Sep 27, 2023

View reviewed changes

test/Concurrency/Runtime/async_custom_task_executor.swift Outdated Show resolved Hide resolved

ktoso requested review from rjmccall and DougGregor September 27, 2023 05:29

ktoso added concurrency Feature: umbrella label for concurrency language features concurrency runtime Feature: umbrella label for concurrency runtime features labels Sep 27, 2023

ktoso force-pushed the wip-task-executors branch from 0536cc0 to 887f9f6 Compare September 27, 2023 07:54

ktoso force-pushed the wip-task-executors branch from 887f9f6 to c0f8daf Compare October 5, 2023 07:58

ktoso marked this pull request as draft October 6, 2023 08:10

ktoso force-pushed the wip-task-executors branch 2 times, most recently from f18f19d to e6ad086 Compare October 17, 2023 13:26

ktoso force-pushed the wip-task-executors branch 2 times, most recently from 8d7549b to 5bcff28 Compare November 10, 2023 08:45

ktoso force-pushed the wip-task-executors branch from 5bcff28 to 77371a9 Compare November 10, 2023 09:56

ktoso force-pushed the wip-task-executors branch 2 times, most recently from 313a0bc to 16e138e Compare November 14, 2023 14:29

ktoso changed the title ~~[WIP] Initial Task Executor implementation Task(on:), addTask(on:) etc.~~ Initial Task Executor implementation Task(on:), addTask(on:) etc. Nov 15, 2023

ktoso force-pushed the wip-task-executors branch from 16e138e to c164dc1 Compare November 16, 2023 09:53

ktoso marked this pull request as ready for review November 17, 2023 09:40

ktoso requested a review from a team as a code owner November 17, 2023 09:40

rjmccall requested changes Nov 17, 2023

View reviewed changes

ktoso and others added 19 commits December 11, 2023 13:12

fix various warnings in tests

c4f3a71

allow an executor to be BOTH Serial and Task

c79816e

correct sil test

ec2336e

docs: runtime availability

51c46b6

cleanup unused code, and undo formatting changes

9496eaf

Optimize: status flag to avoid scanning records for preference

47039f0

add missing dispatch import flag

046703c

fix syntax in compatibility override test

c960923

add task executor test with distributed actor func

b5e73b6

stability-concurrency-abi required explicit availability on accessor

adbabc7

embedded cannot support task executors, make unavailable

e28cfe3

remove warnings from async_task_executor_structured_concurrency.swift

a9ae2ca

fix test in CompatibilityOverrideConcurrency

25f3fcc

compatibility overrides for the new task executor push/pop/get methods

d8b438e

Remove not used RuntimeFunc definition

e8d1563

No assertion API for TaskExecutor

baf019c

Underscore _TaskExecutor, addTask, and related APIs

bbfe034

Comment formatting and add cut off sentence

8af2bd9

ktoso force-pushed the wip-task-executors branch from 53ef9d7 to 43a8924 Compare December 11, 2023 10:59

add Task.detached(_on:) which was missing

907b0e6

ktoso force-pushed the wip-task-executors branch from 43a8924 to 907b0e6 Compare December 11, 2023 11:00

ktoso merged commit 828f589 into swiftlang:main Dec 12, 2023

ktoso deleted the wip-task-executors branch December 12, 2023 08:14

kateinoigakukun mentioned this pull request Dec 19, 2023

[DiscardingTG] Remove reabstraction thunk for () -> Void to () -> T #70537

Merged

ktoso mentioned this pull request Jan 9, 2024

[TaskExecutors] Task initializer and withTaskExecutor parameter changes #70783

Merged

Catfish-Man pushed a commit to Catfish-Man/swift that referenced this pull request Jan 19, 2024

Initial Task Executor implementation Task(on:), addTask(on:) etc. (sw…

419e9b9

…iftlang#68793) Co-authored-by: John McCall <[email protected]>

Initial Task Executor implementation Task(on:), addTask(on:) etc. #68793

Initial Task Executor implementation Task(on:), addTask(on:) etc. #68793

Uh oh!

Conversation

ktoso commented Sep 27, 2023

Uh oh!

Uh oh!

ktoso commented Nov 10, 2023

Uh oh!

ktoso commented Nov 13, 2023

Uh oh!

ktoso commented Nov 14, 2023

Uh oh!

ktoso commented Nov 14, 2023

Uh oh!

ktoso commented Nov 17, 2023

Uh oh!

rjmccall left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ktoso Nov 20, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ktoso Nov 20, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ktoso Nov 20, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ktoso commented Dec 11, 2023

Uh oh!

ktoso commented Dec 11, 2023

Uh oh!

ktoso commented Dec 12, 2023

Uh oh!

Uh oh!

ktoso Nov 20, 2023 •

edited

Loading

ktoso Nov 20, 2023 •

edited

Loading

ktoso Nov 20, 2023 •

edited

Loading