Skip to content

Commit b47a037

Browse files
committed
[Concurrency] StartSynchronously special executor to avoid switching
1 parent d6157fd commit b47a037

File tree

15 files changed

+197
-94
lines changed

15 files changed

+197
-94
lines changed

include/swift/ABI/Executor.h

+13-1
Original file line numberDiff line numberDiff line change
@@ -77,6 +77,8 @@ class SerialExecutorRef {
7777
/// Executor that may need to participate in complex "same context" checks,
7878
/// by invoking `isSameExclusiveExecutionContext` when comparing execution contexts.
7979
ComplexEquality = 0b01,
80+
///
81+
StartSynchronously = 0b10,
8082
};
8183

8284
static_assert(static_cast<uintptr_t>(ExecutorKind::Ordinary) == 0);
@@ -101,6 +103,16 @@ class SerialExecutorRef {
101103
return SerialExecutorRef(actor, 0);
102104
}
103105

106+
static SerialExecutorRef forSynchronousStart() {
107+
auto wtable = reinterpret_cast<uintptr_t>(nullptr) |
108+
static_cast<uintptr_t>(ExecutorKind::StartSynchronously);
109+
return SerialExecutorRef(nullptr, wtable);
110+
}
111+
bool isForSynchronousStart() const {
112+
return getIdentity() == nullptr &&
113+
getExecutorKind() == ExecutorKind::StartSynchronously;
114+
}
115+
104116
/// Given a pointer to a serial executor and its SerialExecutor
105117
/// conformance, return an executor reference for it.
106118
static SerialExecutorRef forOrdinary(HeapObject *identity,
@@ -127,7 +139,7 @@ class SerialExecutorRef {
127139

128140
const char* getIdentityDebugName() const {
129141
return isMainExecutor() ? " (MainActorExecutor)"
130-
: isGeneric() ? " (GenericExecutor)"
142+
: isGeneric() ? (isForSynchronousStart() ? " (GenericExecutor/SynchronousStart)" : " (GenericExecutor)")
131143
: "";
132144
}
133145

include/swift/ABI/MetadataValues.h

+5-2
Original file line numberDiff line numberDiff line change
@@ -2668,13 +2668,13 @@ class TaskCreateFlags : public FlagSet<size_t> {
26682668
Task_EnqueueJob = 12,
26692669
Task_AddPendingGroupTaskUnconditionally = 13,
26702670
Task_IsDiscardingTask = 14,
2671-
26722671
/// The task function is consumed by calling it (@callee_owned).
26732672
/// The context pointer should be treated as opaque and non-copyable;
26742673
/// in particular, it should not be retained or released.
26752674
///
26762675
/// Supported starting in Swift 6.1.
2677-
Task_IsTaskFunctionConsumed = 15,
2676+
Task_IsTaskFunctionConsumed = 15,
2677+
Task_IsStartSynchronouslyTask = 16,
26782678
};
26792679

26802680
explicit constexpr TaskCreateFlags(size_t bits) : FlagSet(bits) {}
@@ -2707,6 +2707,9 @@ class TaskCreateFlags : public FlagSet<size_t> {
27072707
FLAGSET_DEFINE_FLAG_ACCESSORS(Task_IsTaskFunctionConsumed,
27082708
isTaskFunctionConsumed,
27092709
setIsTaskFunctionConsumed)
2710+
FLAGSET_DEFINE_FLAG_ACCESSORS(Task_IsStartSynchronouslyTask,
2711+
isSynchronousStartTask,
2712+
setIsSYnchronousStartTask)
27102713
};
27112714

27122715
/// Flags for schedulable jobs.

stdlib/public/Concurrency/Actor.cpp

+36-7
Original file line numberDiff line numberDiff line change
@@ -134,6 +134,8 @@ class ExecutorTrackingInfo {
134134
/// is `generic`.
135135
TaskExecutorRef TaskExecutor = TaskExecutorRef::undefined();
136136

137+
bool StartedSynchronouslySkipSwitchOnce = false;
138+
137139
/// Whether this context allows switching. Some contexts do not;
138140
/// for example, we do not allow switching from swift_job_run
139141
/// unless the passed-in executor is generic.
@@ -177,7 +179,7 @@ class ExecutorTrackingInfo {
177179
}
178180

179181
bool allowsSwitching() const {
180-
return AllowsSwitching;
182+
return AllowsSwitching && !StartedSynchronouslySkipSwitchOnce;
181183
}
182184

183185
/// Disallow switching in this tracking context. This should only
@@ -186,6 +188,16 @@ class ExecutorTrackingInfo {
186188
AllowsSwitching = false;
187189
}
188190

191+
void markSynchronousStart() {
192+
StartedSynchronouslySkipSwitchOnce = true;
193+
}
194+
bool isSynchronousStart() const {
195+
return StartedSynchronouslySkipSwitchOnce;
196+
}
197+
void withoutStartSynchronously() {
198+
StartedSynchronouslySkipSwitchOnce = false;
199+
}
200+
189201
static ExecutorTrackingInfo *current() {
190202
return ActiveInfoInThread.get();
191203
}
@@ -2151,13 +2163,23 @@ static bool canGiveUpThreadForSwitch(ExecutorTrackingInfo *trackingInfo,
21512163
assert(trackingInfo || currentExecutor.isGeneric());
21522164

21532165
// Some contexts don't allow switching at all.
2154-
if (trackingInfo && !trackingInfo->allowsSwitching())
2166+
if (trackingInfo && !trackingInfo->allowsSwitching()) {
21552167
return false;
2168+
}
21562169

21572170
// We can certainly "give up" a generic executor to try to run
21582171
// a task for an actor.
2159-
if (currentExecutor.isGeneric())
2172+
if (currentExecutor.isGeneric()) {
2173+
if (trackingInfo->isSynchronousStart()) {
2174+
return false;
2175+
}
2176+
2177+
if (currentExecutor.isForSynchronousStart()) {
2178+
return false;
2179+
}
2180+
21602181
return true;
2182+
}
21612183

21622184
// If the current executor is a default actor, we know how to make
21632185
// it give up its thread.
@@ -2278,7 +2300,8 @@ static void swift_task_switchImpl(SWIFT_ASYNC_CONTEXT AsyncContext *resumeContex
22782300
: TaskExecutorRef::undefined());
22792301
auto newTaskExecutor = task->getPreferredTaskExecutor();
22802302
SWIFT_TASK_DEBUG_LOG("Task %p trying to switch executors: executor %p%s to "
2281-
"new serial executor: %p%s; task executor: from %p%s to %p%s",
2303+
"new serial executor: %p%s; task executor: from %p%s to %p%s"
2304+
"%s",
22822305
task,
22832306
currentExecutor.getIdentity(),
22842307
currentExecutor.getIdentityDebugName(),
@@ -2287,13 +2310,15 @@ static void swift_task_switchImpl(SWIFT_ASYNC_CONTEXT AsyncContext *resumeContex
22872310
currentTaskExecutor.getIdentity(),
22882311
currentTaskExecutor.isDefined() ? "" : " (undefined)",
22892312
newTaskExecutor.getIdentity(),
2290-
newTaskExecutor.isDefined() ? "" : " (undefined)");
2313+
newTaskExecutor.isDefined() ? "" : " (undefined)",
2314+
trackingInfo->isSynchronousStart() ? "[synchronous start]" : "[NOT SYNC START]");
22912315

22922316
// If the current executor is compatible with running the new executor,
22932317
// we can just immediately continue running with the resume function
22942318
// we were passed in.
22952319
if (!mustSwitchToRun(currentExecutor, newExecutor, currentTaskExecutor,
22962320
newTaskExecutor)) {
2321+
SWIFT_TASK_DEBUG_LOG("Task %p run inline", task);
22972322
return resumeFunction(resumeContext); // 'return' forces tail call
22982323
}
22992324

@@ -2305,6 +2330,7 @@ static void swift_task_switchImpl(SWIFT_ASYNC_CONTEXT AsyncContext *resumeContex
23052330
// If the current executor can give up its thread, and the new executor
23062331
// can take over a thread, try to do so; but don't do this if we've
23072332
// been asked to yield the thread.
2333+
SWIFT_TASK_DEBUG_LOG("Task %p can give up thread?", task);
23082334
if (currentTaskExecutor.isUndefined() &&
23092335
canGiveUpThreadForSwitch(trackingInfo, currentExecutor) &&
23102336
!shouldYieldThread() &&
@@ -2339,8 +2365,11 @@ static void swift_task_startSynchronouslyImpl(AsyncTask* task) {
23392365
swift_job_run(task, currentExecutor);
23402366
_swift_task_setCurrent(originalTask);
23412367
} else {
2342-
// FIXME: this is not correct; we'll keep reusing this thread when we should not have been. See tests with actors.
2343-
SerialExecutorRef executor = SerialExecutorRef::generic(); // _task_task_getRunSynchronouslyExecutor();
2368+
SerialExecutorRef executor = SerialExecutorRef::forSynchronousStart();
2369+
2370+
ExecutorTrackingInfo trackingInfo;
2371+
// trackingInfo.markSynchronousStart();
2372+
23442373
auto originalTask = ActiveTask::swap(task);
23452374
assert(!originalTask);
23462375

stdlib/public/Concurrency/DiscardingTaskGroup.swift

+16-8
Original file line numberDiff line numberDiff line change
@@ -205,13 +205,15 @@ public struct DiscardingTaskGroup {
205205
let flags = taskCreateFlags(
206206
priority: priority, isChildTask: true, copyTaskLocals: false,
207207
inheritContext: false, enqueueJob: false,
208-
addPendingGroupTaskUnconditionally: true, isDiscardingTask: true
208+
addPendingGroupTaskUnconditionally: true, isDiscardingTask: true,
209+
isSynchronousStart: false
209210
)
210211
#else
211212
let flags = taskCreateFlags(
212213
priority: priority, isChildTask: true, copyTaskLocals: false,
213214
inheritContext: false, enqueueJob: true,
214-
addPendingGroupTaskUnconditionally: true, isDiscardingTask: true
215+
addPendingGroupTaskUnconditionally: true, isDiscardingTask: true,
216+
isSynchronousStart: false
215217
)
216218
#endif
217219

@@ -252,13 +254,15 @@ public struct DiscardingTaskGroup {
252254
let flags = taskCreateFlags(
253255
priority: priority, isChildTask: true, copyTaskLocals: false,
254256
inheritContext: false, enqueueJob: false,
255-
addPendingGroupTaskUnconditionally: false, isDiscardingTask: true
257+
addPendingGroupTaskUnconditionally: false, isDiscardingTask: true,
258+
isSynchronousStart: false
256259
)
257260
#else
258261
let flags = taskCreateFlags(
259262
priority: priority, isChildTask: true, copyTaskLocals: false,
260263
inheritContext: false, enqueueJob: true,
261-
addPendingGroupTaskUnconditionally: false, isDiscardingTask: true
264+
addPendingGroupTaskUnconditionally: false, isDiscardingTask: true,
265+
isSynchronousStart: false
262266
)
263267
#endif
264268

@@ -281,7 +285,8 @@ public struct DiscardingTaskGroup {
281285
let flags = taskCreateFlags(
282286
priority: nil, isChildTask: true, copyTaskLocals: false,
283287
inheritContext: false, enqueueJob: true,
284-
addPendingGroupTaskUnconditionally: true, isDiscardingTask: true
288+
addPendingGroupTaskUnconditionally: true, isDiscardingTask: true,
289+
isSynchronousStart: false
285290
)
286291

287292
// Create the task in this group.
@@ -317,7 +322,8 @@ public struct DiscardingTaskGroup {
317322
let flags = taskCreateFlags(
318323
priority: nil, isChildTask: true, copyTaskLocals: false,
319324
inheritContext: false, enqueueJob: true,
320-
addPendingGroupTaskUnconditionally: false, isDiscardingTask: true
325+
addPendingGroupTaskUnconditionally: false, isDiscardingTask: true,
326+
isSynchronousStart: false
321327
)
322328

323329
// Create the task in this group.
@@ -635,7 +641,8 @@ public struct ThrowingDiscardingTaskGroup<Failure: Error> {
635641
let flags = taskCreateFlags(
636642
priority: priority, isChildTask: true, copyTaskLocals: false,
637643
inheritContext: false, enqueueJob: true,
638-
addPendingGroupTaskUnconditionally: true, isDiscardingTask: true
644+
addPendingGroupTaskUnconditionally: true, isDiscardingTask: true,
645+
isSynchronousStart: false
639646
)
640647

641648
// Create the task in this group.
@@ -666,7 +673,8 @@ public struct ThrowingDiscardingTaskGroup<Failure: Error> {
666673
let flags = taskCreateFlags(
667674
priority: priority, isChildTask: true, copyTaskLocals: false,
668675
inheritContext: false, enqueueJob: true,
669-
addPendingGroupTaskUnconditionally: false, isDiscardingTask: true
676+
addPendingGroupTaskUnconditionally: false, isDiscardingTask: true,
677+
isSynchronousStart: false
670678
)
671679

672680
// Create the task in this group.

stdlib/public/Concurrency/Task+TaskExecutor.swift

+4-4
Original file line numberDiff line numberDiff line change
@@ -240,7 +240,7 @@ extension Task where Failure == Never {
240240
priority: priority, isChildTask: false, copyTaskLocals: true,
241241
inheritContext: true, enqueueJob: true,
242242
addPendingGroupTaskUnconditionally: false,
243-
isDiscardingTask: false)
243+
isDiscardingTask: false, isSynchronousStart: false)
244244

245245
#if $BuiltinCreateAsyncTaskOwnedTaskExecutor
246246
let (task, _) = Builtin.createTask(
@@ -303,7 +303,7 @@ extension Task where Failure == Error {
303303
priority: priority, isChildTask: false, copyTaskLocals: true,
304304
inheritContext: true, enqueueJob: true,
305305
addPendingGroupTaskUnconditionally: false,
306-
isDiscardingTask: false)
306+
isDiscardingTask: false, isSynchronousStart: false)
307307

308308
#if $BuiltinCreateAsyncTaskOwnedTaskExecutor
309309
let (task, _) = Builtin.createTask(
@@ -364,7 +364,7 @@ extension Task where Failure == Never {
364364
priority: priority, isChildTask: false, copyTaskLocals: false,
365365
inheritContext: false, enqueueJob: true,
366366
addPendingGroupTaskUnconditionally: false,
367-
isDiscardingTask: false)
367+
isDiscardingTask: false, isSynchronousStart: false)
368368

369369
#if $BuiltinCreateAsyncTaskOwnedTaskExecutor
370370
let (task, _) = Builtin.createTask(
@@ -425,7 +425,7 @@ extension Task where Failure == Error {
425425
priority: priority, isChildTask: false, copyTaskLocals: false,
426426
inheritContext: false, enqueueJob: true,
427427
addPendingGroupTaskUnconditionally: false,
428-
isDiscardingTask: false)
428+
isDiscardingTask: false, isSynchronousStart: false)
429429

430430
#if $BuiltinCreateAsyncTaskOwnedTaskExecutor
431431
let (task, _) = Builtin.createTask(

stdlib/public/Concurrency/Task.cpp

+2-1
Original file line numberDiff line numberDiff line change
@@ -847,7 +847,8 @@ swift_task_create_commonImpl(size_t rawTaskCreateFlags,
847847
// basePriority should already be the right value
848848

849849
} else if (taskIsUnstructured(taskCreateFlags, jobFlags)) {
850-
SWIFT_TASK_DEBUG_LOG("Creating an unstructured task from %p", currentTask);
850+
SWIFT_TASK_DEBUG_LOG("Creating an unstructured task from %p%s", currentTask,
851+
taskCreateFlags.isSynchronousStartTask() ? " [start synchronously]" : "");
851852

852853
if (isUnspecified(basePriority)) {
853854
// Case 1: No priority specified

0 commit comments

Comments
 (0)