Skip to content

Conversation

@ktoso
Copy link
Contributor

@ktoso ktoso commented Oct 29, 2025

This is mostly right but sometimes hitting a lockup somewhere that I was not able to figure out yet.

This needs much cleanup and is mostly for sharing Work in Progress for discussion right now.


Work in progress table of behaviors

current SerialExecutor current TaskExecutor new SerialExecutor new TaskExecutor CSE?NSE CTE?NTE Switch Decision Discussion
generic undefined generic undefined == x fast-path global async functions calling one another. Continue calling directly.
default actor T1 default actor T2     enqueue Enqueue on the new task executor, this must be a hop as the requirement has changed.
custom S1 undefined custom S1 undefined == x fast-path Continue executing on the same actor.
custom S1   custom S2   !=   enqueue  
default actor undefined custom S1 undefined != x enqueue  
default actor undefined default actor undefined != != release, acquire release the old actor; acquire the new one; take ownership of the calling thread.
custom S1 undefined default actor undefined != != fast-path + acquire? fastpath execute, but do acquire the target default actor lock.
               
               
               
default actor TE default actor undefined, compare as global TE   == fast-path we can fastpath if we're on a defined task executor that happens to be equal to the GlobalTaskExecutor.
default actor undefined, compare as global TE default actor TE   == fast-path when we're coming from "actor on global executor", and entering TE, we may need to check if the global one is equal to TE.
               
               
               

This is mostly right but sometimes hitting a lockup somewhere that I was
not able to figure out yet.

This needs much cleanup and is mostly for sharing Work in Progress for
discussion right now.
// ABI Note: We need to use @abi here because the ABI of this function otherwise conflicts with the legacy
// @_unsafeInheritExecutor declaration, as none of them have (or mangle) the implicit
@abi(
nonisolated(nonsending) func withCheckedContinuationNonisolatedNonsending<T>(
Copy link
Contributor Author

Choose a reason for hiding this comment

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

These are done in #84944

#define SWIFT_TASK_DEBUG_LOG_ENABLED 1
#define SWIFT_TASK_DEBUG_LOG(fmt, ...) \
fprintf(stderr, "[%#lx] [%s:%d](%s) " fmt "\n", \
fprintf(stdout, "[%#lx] [%s:%d](%s) " fmt "\n", \
Copy link
Contributor Author

Choose a reason for hiding this comment

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

FIXME: just for debugging, undo this

@ktoso ktoso changed the title WIP: Rework task_switch with task executors, don't enqueue unless super necessary [Concurrency] WIP: Rework task_switch with task executors, don't enqueue unless necessary Oct 29, 2025
@ktoso
Copy link
Contributor Author

ktoso commented Oct 29, 2025

@swift-ci please smoke test

@ktoso ktoso force-pushed the wip-fix-task-AND-serial-executor branch from 0407798 to 79bc38a Compare October 29, 2025 14:04
Comment on lines +2603 to +2606
TaskExecutorRef currentTaskExecutor = (trackingInfo ? trackingInfo->getTaskExecutor()
: TaskExecutorRef::undefined());
if (!trackingInfo) SWIFT_TASK_DEBUG_LOG("Missing executor tracking, assume currentTaskExecutor = %s.", "undefined");
TaskExecutorRef newTaskExecutor = task->getPreferredTaskExecutor();
Copy link
Member

Choose a reason for hiding this comment

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

I might have missed this and it is solved differently but are we still keeping track of the default executors as part of the tracking info? In particular, our default executor are conforming to TaskExecutor and they will call runSynchronously with the TaskExecutor overload. I want to be sure that if there is only a custom default executor, no isolation and no task executor preference that we are also hitting the fast path.

@ktoso
Copy link
Contributor Author

ktoso commented Nov 20, 2025

@swift-ci please smoke test

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants