-
Notifications
You must be signed in to change notification settings - Fork 10.5k
concurrency: alloc an async-let task with the parent's allocator. #36993
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -105,14 +105,13 @@ static void swift_asyncLet_startImpl(AsyncLet *alet, | |||||
flags.task_setIsFuture(true); | ||||||
flags.task_setIsChildTask(true); | ||||||
|
||||||
auto childTaskAndContext = swift_task_create_future_no_escaping( | ||||||
auto childTaskAndContext = swift_task_create_async_let_future( | ||||||
flags, | ||||||
futureResultType, | ||||||
closureEntryPoint, | ||||||
closureContext); | ||||||
|
||||||
AsyncTask *childTask = childTaskAndContext.Task; | ||||||
swift_retain(childTask); | ||||||
|
||||||
assert(childTask->isFuture()); | ||||||
assert(childTask->hasChildFragment()); | ||||||
|
@@ -165,7 +164,13 @@ static void swift_asyncLet_endImpl(AsyncLet *alet) { | |||||
// TODO: we need to implicitly await either before the end or here somehow. | ||||||
|
||||||
// and finally, release the task and free the async-let | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
no releasing anymore (yay!) |
||||||
swift_release(task); | ||||||
AsyncTask *parent = swift_task_getCurrent(); | ||||||
assert(parent && "async-let must have a parent task"); | ||||||
|
||||||
#if SWIFT_TASK_PRINTF_DEBUG | ||||||
fprintf(stderr, "[%p] async let end of task %p, parent: %p\n", pthread_self(), task, parent); | ||||||
#endif | ||||||
_swift_task_dealloc_specific(parent, task); | ||||||
} | ||||||
|
||||||
// ============================================================================= | ||||||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -174,23 +174,31 @@ static void destroyJob(SWIFT_CONTEXT HeapObject *obj) { | |
assert(false && "A non-task job should never be destroyed as heap metadata."); | ||
} | ||
|
||
SWIFT_CC(swift) | ||
static void destroyTask(SWIFT_CONTEXT HeapObject *obj) { | ||
auto task = static_cast<AsyncTask*>(obj); | ||
AsyncTask::~AsyncTask() { | ||
// For a future, destroy the result. | ||
if (task->isFuture()) { | ||
task->futureFragment()->destroy(); | ||
if (isFuture()) { | ||
futureFragment()->destroy(); | ||
} | ||
|
||
// Release any objects potentially held as task local values. | ||
task->Local.destroy(task); | ||
Local.destroy(this); | ||
} | ||
|
||
SWIFT_CC(swift) | ||
static void destroyTask(SWIFT_CONTEXT HeapObject *obj) { | ||
auto task = static_cast<AsyncTask*>(obj); | ||
|
||
task->~AsyncTask(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. no idea if doing a destructor is nicer... am I missing something? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think it's a matter of taste |
||
|
||
// The task execution itself should always hold a reference to it, so | ||
// if we get here, we know the task has finished running, which means | ||
// swift_task_complete should have been run, which will have torn down | ||
// the task-local allocator. There's actually nothing else to clean up | ||
// here. | ||
|
||
#if SWIFT_TASK_PRINTF_DEBUG | ||
fprintf(stderr, "[%p] destroy task %p\n", pthread_self(), task); | ||
#endif | ||
free(task); | ||
} | ||
|
||
|
@@ -250,13 +258,9 @@ static FullMetadata<DispatchClassMetadata> taskHeapMetadata = { | |
const void *const swift::_swift_concurrency_debug_asyncTaskMetadata = | ||
static_cast<Metadata *>(&taskHeapMetadata); | ||
|
||
/// The function that we put in the context of a simple task | ||
/// to handle the final return. | ||
SWIFT_CC(swiftasync) | ||
static void completeTask(SWIFT_ASYNC_CONTEXT AsyncContext *context, | ||
SWIFT_CONTEXT SwiftError *error) { | ||
// Set that there's no longer a running task in the current thread. | ||
auto task = _swift_task_clearCurrent(); | ||
static void completeTaskImpl(AsyncTask *task, | ||
AsyncContext *context, | ||
SwiftError *error) { | ||
assert(task && "completing task, but there is no active task registered"); | ||
|
||
// Store the error result. | ||
|
@@ -277,15 +281,41 @@ static void completeTask(SWIFT_ASYNC_CONTEXT AsyncContext *context, | |
#endif | ||
|
||
// Complete the future. | ||
// Warning: This deallocates the task in case it's an async let task. | ||
// The task must not be accessed afterwards. | ||
if (task->isFuture()) { | ||
task->completeFuture(context); | ||
} | ||
|
||
// TODO: set something in the status? | ||
if (task->hasChildFragment()) { | ||
// if (task->hasChildFragment()) { | ||
// TODO: notify the parent somehow? | ||
// TODO: remove this task from the child-task chain? | ||
} | ||
// } | ||
} | ||
|
||
/// The function that we put in the context of a simple task | ||
/// to handle the final return. | ||
SWIFT_CC(swiftasync) | ||
static void completeTask(SWIFT_ASYNC_CONTEXT AsyncContext *context, | ||
SWIFT_CONTEXT SwiftError *error) { | ||
// Set that there's no longer a running task in the current thread. | ||
auto task = _swift_task_clearCurrent(); | ||
assert(task && "completing task, but there is no active task registered"); | ||
|
||
completeTaskImpl(task, context, error); | ||
} | ||
|
||
/// The function that we put in the context of a simple task | ||
/// to handle the final return. | ||
SWIFT_CC(swiftasync) | ||
static void completeTaskAndRelease(SWIFT_ASYNC_CONTEXT AsyncContext *context, | ||
SWIFT_CONTEXT SwiftError *error) { | ||
// Set that there's no longer a running task in the current thread. | ||
auto task = _swift_task_clearCurrent(); | ||
assert(task && "completing task, but there is no active task registered"); | ||
|
||
completeTaskImpl(task, context, error); | ||
|
||
// Release the task, balancing the retain that a running task has on itself. | ||
// If it was a group child task, it will remain until the group returns it. | ||
|
@@ -304,7 +334,7 @@ static void completeTaskWithClosure(SWIFT_ASYNC_CONTEXT AsyncContext *context, | |
swift_release((HeapObject *)asyncContextPrefix->closureContext); | ||
|
||
// Clean up the rest of the task. | ||
return completeTask(context, error); | ||
return completeTaskAndRelease(context, error); | ||
} | ||
|
||
SWIFT_CC(swiftasync) | ||
|
@@ -332,11 +362,16 @@ static void task_wait_throwing_resume_adapter(SWIFT_ASYNC_CONTEXT AsyncContext * | |
} | ||
|
||
/// All `swift_task_create*` variants funnel into this common implementation. | ||
/// | ||
/// If \p isAsyncLetTask is true, the \p closureContext is not heap allocated, | ||
/// but stack-allocated (and must not be ref-counted). | ||
/// Also, async-let tasks are not heap allcoated, but allcoated with the parent | ||
/// task's stack allocator. | ||
static AsyncTaskAndContext swift_task_create_group_future_commonImpl( | ||
JobFlags flags, TaskGroup *group, | ||
const Metadata *futureResultType, | ||
FutureAsyncSignature::FunctionType *function, | ||
void *closureContext, bool owningClosureContext, | ||
void *closureContext, bool isAsyncLetTask, | ||
size_t initialContextSize) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ABI wise my impression was we wanted something like But maybe we want to do that in a separate PR? (totally find by me to do later on!) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yeah, let's do that in a separate PR There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. okey 👍 |
||
assert((futureResultType != nullptr) == flags.task_isFuture()); | ||
assert(!flags.task_isFuture() || | ||
|
@@ -378,10 +413,19 @@ static AsyncTaskAndContext swift_task_create_group_future_commonImpl( | |
|
||
assert(amountToAllocate % MaximumAlignment == 0); | ||
|
||
// TODO: allow optionally passing in an allocation+sizeOfIt to reuse for the task | ||
// if the necessary space is enough, we can initialize into it rather than malloc. | ||
// this would allow us to stack-allocate async-let related tasks. | ||
void *allocation = malloc(amountToAllocate); | ||
constexpr unsigned initialSlabSize = 512; | ||
|
||
void *allocation = nullptr; | ||
if (isAsyncLetTask) { | ||
assert(parent); | ||
allocation = _swift_task_alloc_specific(parent, | ||
amountToAllocate + initialSlabSize); | ||
} else { | ||
allocation = malloc(amountToAllocate); | ||
} | ||
#if SWIFT_TASK_PRINTF_DEBUG | ||
fprintf(stderr, "[%p] allocate task %p, parent = %p\n", pthread_self(), allocation, parent); | ||
#endif | ||
|
||
AsyncContext *initialContext = | ||
reinterpret_cast<AsyncContext*>( | ||
|
@@ -417,9 +461,17 @@ static AsyncTaskAndContext swift_task_create_group_future_commonImpl( | |
|
||
// Initialize the task so that resuming it will run the given | ||
// function on the initial context. | ||
AsyncTask *task = | ||
new(allocation) AsyncTask(&taskHeapMetadata, flags, | ||
function, initialContext); | ||
AsyncTask *task = nullptr; | ||
if (isAsyncLetTask) { | ||
// Initialize the refcount bits to "immortal", so that | ||
// ARC operations don't have any effect on the task. | ||
task = new(allocation) AsyncTask(&taskHeapMetadata, | ||
InlineRefCounts::Immortal, flags, | ||
function, initialContext); | ||
} else { | ||
task = new(allocation) AsyncTask(&taskHeapMetadata, flags, | ||
function, initialContext); | ||
} | ||
|
||
// Initialize the child fragment if applicable. | ||
if (parent) { | ||
|
@@ -471,15 +523,21 @@ static AsyncTaskAndContext swift_task_create_group_future_commonImpl( | |
// as if they might be null, even though the only time they ever might | ||
// be is the final hop. Store a signed null instead. | ||
initialContext->Parent = nullptr; | ||
initialContext->ResumeParent = reinterpret_cast<TaskContinuationFunction *>( | ||
(closureContext && owningClosureContext) ? &completeTaskWithClosure : | ||
&completeTask); | ||
initialContext->Flags = AsyncContextKind::Ordinary; | ||
initialContext->Flags.setShouldNotDeallocateInCallee(true); | ||
|
||
// Initialize the task-local allocator. | ||
// TODO: consider providing an initial pre-allocated first slab to the allocator. | ||
_swift_task_alloc_initialize(task); | ||
if (isAsyncLetTask) { | ||
initialContext->ResumeParent = reinterpret_cast<TaskContinuationFunction *>( | ||
&completeTask); | ||
assert(parent); | ||
void *initialSlab = (char*)allocation + amountToAllocate; | ||
_swift_task_alloc_initialize_with_slab(task, initialSlab, initialSlabSize); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. cool! so even a bunch of tiny task-local allocs inside the child have a chance to stay in the initial slab... <3 |
||
} else { | ||
initialContext->ResumeParent = reinterpret_cast<TaskContinuationFunction *>( | ||
closureContext ? &completeTaskWithClosure : &completeTaskAndRelease); | ||
_swift_task_alloc_initialize(task); | ||
} | ||
|
||
// TODO: if the allocator would be prepared earlier we could do this in some | ||
// other existing if-parent if rather than adding another one here. | ||
|
@@ -494,7 +552,7 @@ static AsyncTaskAndContext swift_task_create_group_future_commonImpl( | |
static AsyncTaskAndContext swift_task_create_group_future_common( | ||
JobFlags flags, TaskGroup *group, const Metadata *futureResultType, | ||
FutureAsyncSignature::FunctionType *function, | ||
void *closureContext, bool owningClosureContext, | ||
void *closureContext, bool isAsyncLetTask, | ||
size_t initialContextSize); | ||
|
||
AsyncTaskAndContext | ||
|
@@ -524,7 +582,7 @@ AsyncTaskAndContext swift::swift_task_create_group_future_f( | |
return swift_task_create_group_future_common(flags, group, | ||
futureResultType, | ||
function, nullptr, | ||
/*owningClosureContext*/ false, | ||
/*isAsyncLetTask*/ false, | ||
initialContextSize); | ||
} | ||
|
||
|
@@ -560,11 +618,11 @@ AsyncTaskAndContext swift::swift_task_create_future(JobFlags flags, | |
return swift_task_create_group_future_common( | ||
flags, nullptr, futureResultType, | ||
taskEntry, closureContext, | ||
/*owningClosureContext*/ true, | ||
/*isAsyncLetTask*/ false, | ||
initialContextSize); | ||
} | ||
|
||
AsyncTaskAndContext swift::swift_task_create_future_no_escaping(JobFlags flags, | ||
AsyncTaskAndContext swift::swift_task_create_async_let_future(JobFlags flags, | ||
const Metadata *futureResultType, | ||
void *closureEntry, | ||
void *closureContext) { | ||
|
@@ -579,7 +637,7 @@ AsyncTaskAndContext swift::swift_task_create_future_no_escaping(JobFlags flags, | |
return swift_task_create_group_future_common( | ||
flags, nullptr, futureResultType, | ||
taskEntry, closureContext, | ||
/*owningClosureContext*/ false, | ||
/*isAsyncLetTask*/ true, | ||
initialContextSize); | ||
} | ||
|
||
|
@@ -599,7 +657,7 @@ swift::swift_task_create_group_future( | |
return swift_task_create_group_future_common( | ||
flags, group, futureResultType, | ||
taskEntry, closureContext, | ||
/*owningClosureContext*/ true, | ||
/*isAsyncLetTask*/ false, | ||
initialContextSize); | ||
} | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nice, thanks for the comment!