[TaskLocals] Remove fragment, cleanup code organization, use task-local allocator #36210
Conversation
ktoso
force-pushed
the
wip-locals-detached
branch
from
0a5e544 to
11d9785
Compare
|
@swift-ci please smoke test |
| @@ -173,12 +172,16 @@ class AsyncTask : public HeapObject, public Job { | |||
| /// Reserved for the use of the task-local stack allocator. | |||
| void *AllocatorPrivate[4]; | |||
|
|
|||
| /// Task local values storage container. | |||
| TaskLocal::Storage Local; | |||
There was a problem hiding this comment.
Should we future proof it a bit more just in case?
Instruments folks asked for a limited size "carry it always, but only the last 3 most recent values" which I'll be implementing next. but I wonder if it might be necessary to add resilience on this level, or we're ok because task is?
| #include "swift/ABI/Task.h" | ||
| #include "swift/ABI/MetadataValues.h" |
There was a problem hiding this comment.
made includes more consistent; was battling include hell for quite a bit when adding new header files today here
| @@ -122,10 +122,6 @@ void swift_task_cancel(AsyncTask *task); | |||
| SWIFT_EXPORT_FROM(swift_Concurrency) SWIFT_CC(swift) | |||
| void swift_task_cancel_group_child_tasks(AsyncTask *task, TaskGroup *group); | |||
|
|
|||
| /// Get 'active' AsyncTask, depending on platform this may use thread local storage. | |||
| SWIFT_EXPORT_FROM(swift_Concurrency) SWIFT_CC(swift) | |||
| AsyncTask* swift_task_get_active(); | |||
There was a problem hiding this comment.
not used; we're using the getCurrent which does the same
| // Initialize task locals fragment. | ||
| auto taskLocalsFragment = task->localValuesFragment(); | ||
| new (taskLocalsFragment) AsyncTask::TaskLocalValuesFragment(); | ||
| taskLocalsFragment->initializeLinkParent(task, parent); |
There was a problem hiding this comment.
fragment is gone, every task needs the pointer because we store locals in this task so it always has to be ready to accept someone pushing a value onto it
| func hash(into hasher: inout Hasher) { | ||
| hasher.combine(ObjectIdentifier(self.keyType)) | ||
| } | ||
| } |
There was a problem hiding this comment.
not necessary, we use the types as identity in C by raw pointers, so no need for erasure in Swift
| await x3 | ||
| } | ||
|
|
||
| _ = await x1 | ||
| try! await printTaskLocal(\.number) // CHECK: NumberKey: 0 {{.*}} | ||
| printTaskLocal(\.number) // CHECK: NumberKey: 0 {{.*}} |
There was a problem hiding this comment.
now we were able to remove a lot of await which is awesome -- thanks to getCurrent task from sync functions
| @@ -255,9 +260,6 @@ AsyncTaskAndContext swift::swift_task_create_group_future_f( | |||
| // Figure out the size of the header. | |||
| size_t headerSize = sizeof(AsyncTask); | |||
|
|
|||
| /// Every task is able to store task local values. | |||
| headerSize += sizeof(AsyncTask::TaskLocalValuesFragment); | |||
There was a problem hiding this comment.
no need for this, since locals are part of the Task now
| assert(task); | ||
| size_t amountToAllocate = Item::itemSize(valueType); | ||
| // assert(amountToAllocate % MaximumAlignment == 0); // TODO: do we need this? | ||
| void *allocation = swift_task_alloc(task, amountToAllocate); |
There was a problem hiding this comment.
hooray, we're now using swift_task_alloc/swift_task_dealloc, and it all works excellently thanks to the only way to bind a value being withLocal 👍
ktoso
requested review from
rjmccall and
DougGregor
and removed request for
rjmccall
ktoso
changed the title
ktoso
force-pushed
the
wip-locals-detached
branch
from
d6b8834 to
b0df9b0
Compare
ktoso
force-pushed
the
wip-locals-detached
branch
from
b0df9b0 to
fcb1c01
Compare
|
@swift-ci please smoke test |
|
@swift-ci please smoke test macOS |
Task locals must have a pointer inside every task to implement their functionality after all, so as such, we don't need to make them a fragment.
Also proceed towards more cleanly separated out ABI and remove implementation details from Task.h entirely.
rdar://74218679