Skip to content

Commit 14b8546

Browse files
ktosoktoso
committed
[Concurrency] Don't add new task locals copy runtime func
1 parent d536ffa commit 14b8546

File tree

8 files changed

+71
-56
lines changed

8 files changed

+71
-56
lines changed

‎include/swift/ABI/TaskLocal.h

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -129,11 +129,11 @@ class TaskLocal {
129129
return reinterpret_cast<Item *>(next & ~statusMask);
130130
}
131131

132-
void relinkNext(Item* nextOverride) {
132+
void relinkTaskGroupLocalHeadToSafeNext(Item* nextOverride) {
133133
assert(!getNext() &&
134134
"Can only relink task local item that was not pointing at anything yet");
135-
assert(nextOverride->isNextLinkPointer() ||
136-
nextOverride->isParentPointer() &&
135+
assert((nextOverride->isNextLinkPointer() ||
136+
nextOverride->isParentPointer()) &&
137137
"Currently relinking is only done within a task group to "
138138
"avoid within-taskgroup next pointers; attempted to point at "
139139
"task local declared within task group body though!");
@@ -230,6 +230,10 @@ class TaskLocal {
230230

231231
public:
232232

233+
/// Get the "current" task local storage from either the passed in
234+
/// task, or fall back to the *thread* local stored storage.
235+
static Storage* getCurrent(AsyncTask *task);
236+
233237
void initializeLinkParent(AsyncTask *task, AsyncTask *parent);
234238

235239
void pushValue(AsyncTask *task,
@@ -258,7 +262,9 @@ class TaskLocal {
258262
/// "pop" of the `B` value - it was spawned from a scope where only B was observable.
259263
void copyTo(AsyncTask *target);
260264

261-
void copyToOnlyOnlyFromCurrent(AsyncTask *target);
265+
// FIXME(concurrency): We currently copy from "all" task groups we encounter
266+
// however in practice we only
267+
void copyToOnlyOnlyFromCurrentGroup(AsyncTask *target);
262268

263269
/// Destroy and deallocate all items stored by this specific task.
264270
///

‎include/swift/Runtime/Concurrency.h

Lines changed: 0 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -668,18 +668,6 @@ void swift_task_localValuePop();
668668
SWIFT_EXPORT_FROM(swift_Concurrency) SWIFT_CC(swift)
669669
void swift_task_localsCopyTo(AsyncTask* target);
670670

671-
/// Some task local bindings must be copied defensively when a child task is
672-
/// created in a task group. See task creation (swift_task_create_common) for
673-
/// a detailed discussion how and when this is used.
674-
///
675-
/// Its Swift signature is
676-
///
677-
/// \code
678-
/// func swift_task_localsCopyToTaskGroupChildTaskDefensively<Key>(AsyncTask* task)
679-
/// \endcode
680-
SWIFT_EXPORT_FROM(swift_Concurrency) SWIFT_CC(swift)
681-
void swift_task_localsCopyToTaskGroupChildTaskDefensively(AsyncTask* target);
682-
683671
/// Switch the current task to a new executor if we aren't already
684672
/// running on a compatible executor.
685673
///

‎stdlib/public/CompatibilityOverride/CompatibilityOverrideConcurrency.def

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -378,12 +378,6 @@ OVERRIDE_TASK_LOCAL(task_localsCopyTo, void,
378378
(AsyncTask *target),
379379
(target))
380380

381-
OVERRIDE_TASK_LOCAL(task_localsCopyToTaskGroupChildTaskDefensively, void,
382-
SWIFT_EXPORT_FROM(swift_Concurrency), SWIFT_CC(swift),
383-
swift::,
384-
(AsyncTask *target),
385-
(target))
386-
387381
OVERRIDE_TASK_STATUS(task_hasTaskGroupStatusRecord, bool,
388382
SWIFT_EXPORT_FROM(swift_Concurrency), SWIFT_CC(swift),
389383
swift::, , )

‎stdlib/public/Concurrency/Task.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1017,7 +1017,7 @@ swift_task_create_commonImpl(size_t rawTaskCreateFlags,
10171017
if (auto taskLocalHeadLinkType = ParentLocal.peekHeadLinkType()) {
10181018
if (taskLocalHeadLinkType ==
10191019
swift::TaskLocal::NextLinkType::IsNextCreatedInTaskGroupBody) {
1020-
swift_task_localsCopyToTaskGroupChildTaskDefensively(task);
1020+
ParentLocal.copyToOnlyOnlyFromCurrentGroup(task);
10211021
taskLocalStorageInitialized = true;
10221022
}
10231023
}

‎stdlib/public/Concurrency/TaskLocal.cpp

Lines changed: 14 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -149,39 +149,25 @@ static void swift_task_localValuePopImpl() {
149149

150150
SWIFT_CC(swift)
151151
static void swift_task_localsCopyToImpl(AsyncTask *target) {
152-
TaskLocal::Storage *Local = nullptr;
153-
154-
if (AsyncTask *task = swift_task_getCurrent()) {
155-
Local = &task->_private().Local;
156-
} else if (auto *storage = FallbackTaskLocalStorage::get()) {
157-
Local = storage;
158-
} else {
159-
// bail out, there are no values to copy
160-
return;
152+
if (auto Current = TaskLocal::Storage::getCurrent(swift_task_getCurrent())) {
153+
Current->copyTo(target);
161154
}
162-
163-
Local->copyTo(target);
164155
}
165156

166-
SWIFT_CC(swift)
167-
static void swift_task_localsCopyToTaskGroupChildTaskDefensivelyImpl(AsyncTask *target) {
168-
TaskLocal::Storage *Local = nullptr;
157+
// =============================================================================
158+
// ==== Initialization ---------------------------------------------------------
169159

170-
if (AsyncTask *task = swift_task_getCurrent()) {
171-
Local = &task->_private().Local;
160+
TaskLocal::Storage*
161+
TaskLocal::Storage::getCurrent(AsyncTask *current) {
162+
if (current) {
163+
return &current->_private().Local;
172164
} else if (auto *storage = FallbackTaskLocalStorage::get()) {
173-
Local = storage;
174-
} else {
175-
// bail out, there are no values to copy
176-
return;
165+
return storage;
177166
}
178167

179-
Local->copyToOnlyOnlyFromCurrent(target);
168+
return nullptr;
180169
}
181170

182-
// =============================================================================
183-
// ==== Initialization ---------------------------------------------------------
184-
185171
void TaskLocal::Storage::initializeLinkParent(AsyncTask* task,
186172
AsyncTask* parent) {
187173
assert(!head && "initial task local storage was already initialized");
@@ -501,7 +487,9 @@ void TaskLocal::Storage::copyTo(AsyncTask *target) {
501487
}
502488
}
503489

504-
void TaskLocal::Storage::copyToOnlyOnlyFromCurrent(AsyncTask *target) {
490+
// TODO(concurrency): This can be optimized to copy only from the CURRENT group,
491+
// but we need to detect this, e.g. by more flags in the items made from a group?
492+
void TaskLocal::Storage::copyToOnlyOnlyFromCurrentGroup(AsyncTask *target) {
505493
assert(target && "task must not be null when copying values into it");
506494
assert(!(target->_private().Local.head) &&
507495
"Task must not have any task-local values bound before copying into it");
@@ -525,7 +513,7 @@ void TaskLocal::Storage::copyToOnlyOnlyFromCurrent(AsyncTask *target) {
525513
// as we would have within normal child task relationships. E.g. this is
526514
// a parent or next pointer to a "safe" (withValue { withTaskGroup { ... } })
527515
// binding, so we re-link our current head to point at this item.
528-
copiedHead->relinkNext(item);
516+
copiedHead->relinkTaskGroupLocalHeadToSafeNext(item);
529517
break;
530518
}
531519

‎test/Concurrency/Runtime/async_task_locals_in_task_group_may_need_to_copy.swift

Lines changed: 46 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,8 @@ enum TL {
1313
static var two: Int = 2
1414
@TaskLocal
1515
static var three: Int = 3
16+
@TaskLocal
17+
static var four: Int = 4
1618
}
1719

1820
// ==== ------------------------------------------------------------------------
@@ -96,6 +98,7 @@ func test() async {
9698
print("--")
9799
}
98100

101+
// Multiple nested task groups *in the same task*
99102
await TL.$one.withValue(11) {
100103
await withTaskGroup(of: Void.self) { group in
101104
await TL.$three.withValue(33) {
@@ -109,10 +112,52 @@ func test() async {
109112
}
110113
}
111114
}
115+
print("--")
116+
}
117+
}
118+
119+
// Multiple "nested" task groups, however in different child tasks
120+
await withTaskGroup(of: Void.self) { outer in
121+
// Child task inside 'outer':
122+
await TL.$one.withValue(11) { // DOES NOT have to be copied by the 'inner.addTask' (!)
123+
outer.addTask {
124+
await TL.$two.withValue(2222) {
125+
await withTaskGroup(of: Void.self) { inner in
126+
TL.$three.withValue(3333) { // MUST be copied by 'inner.addTask'
127+
inner.addTask {
128+
print("Survived, one: \(TL.one) @ \(#fileID):\(#line)") // CHECK: Survived, one: 11
129+
print("Survived, two: \(TL.two) @ \(#fileID):\(#line)") // CHECK: Survived, two: 2222
130+
print("Survived, three: \(TL.three) @ \(#fileID):\(#line)") // CHECK: Survived, three: 3333
131+
}
132+
}
133+
}
134+
}
135+
print("--")
136+
}
137+
}
138+
}
112139

113-
print("Survived, done") // CHECK: Survived, done
140+
// Multiple "safe" task locals around a withTaskGroup, check if relinking works ok then
141+
await TL.$one.withValue(11) {
142+
await TL.$two.withValue(22) {
143+
await TL.$three.withValue(33) {
144+
await withTaskGroup(of: Void.self) { group in
145+
await TL.$four.withValue(44) { // must copy
146+
group.addTask {
147+
print("Survived, one: \(TL.one) @ \(#fileID):\(#line)") // CHECK: Survived, one: 11
148+
print("Survived, two: \(TL.two) @ \(#fileID):\(#line)") // CHECK: Survived, two: 22
149+
print("Survived, three: \(TL.three) @ \(#fileID):\(#line)") // CHECK: Survived, three: 33
150+
print("Survived, four: \(TL.four) @ \(#fileID):\(#line)") // CHECK: Survived, four: 44
151+
}
152+
}
153+
}
154+
}
114155
}
156+
print("--")
115157
}
158+
159+
160+
print("Survived, done") // CHECK: Survived, done
116161
}
117162

118163
@main struct Main {

‎test/abi/macOS/arm64/concurrency.swift

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -284,9 +284,6 @@ Added: _$ss26withTaskExecutorPreference_9isolation9operationxSch_pSg_ScA_pSgYixy
284284
// async function pointer to Swift.withTaskExecutorPreference<A, B where B: Swift.Error>(_: Swift.TaskExecutor?, isolation: isolated Swift.Actor?, operation: () async throws(B) -> A) async throws(B) -> A
285285
Added: _$ss26withTaskExecutorPreference_9isolation9operationxSch_pSg_ScA_pSgYixyYaq_YKXEtYaq_YKs5ErrorR_r0_lFTu
286286

287-
// === Task groups can now handle (copy) task locals set directly around an addTask
288-
Added: _swift_task_localsCopyToTaskGroupChildTaskDefensively
289-
290287
// === Add #isolation to next() and waitForAll() in task groups
291288
// Swift.TaskGroup.awaitAllRemainingTasks(isolation: isolated Swift.Actor?) async -> ()
292289
Added: _$sScG22awaitAllRemainingTasks9isolationyScA_pSgYi_tYaF

‎test/abi/macOS/x86_64/concurrency.swift

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -284,9 +284,6 @@ Added: _$ss26withTaskExecutorPreference_9isolation9operationxSch_pSg_ScA_pSgYixy
284284
// async function pointer to Swift.withTaskExecutorPreference<A, B where B: Swift.Error>(_: Swift.TaskExecutor?, isolation: isolated Swift.Actor?, operation: () async throws(B) -> A) async throws(B) -> A
285285
Added: _$ss26withTaskExecutorPreference_9isolation9operationxSch_pSg_ScA_pSgYixyYaq_YKXEtYaq_YKs5ErrorR_r0_lFTu
286286

287-
// === Task groups can now handle (copy) task locals set directly around an addTask
288-
Added: _swift_task_localsCopyToTaskGroupChildTaskDefensively
289-
290287
// === Add #isolation to next() and waitForAll() in task groups
291288
// Swift.TaskGroup.awaitAllRemainingTasks(isolation: isolated Swift.Actor?) async -> ()
292289
Added: _$sScG22awaitAllRemainingTasks9isolationyScA_pSgYi_tYaF

0 commit comments

Comments
 (0)