-
Notifications
You must be signed in to change notification settings - Fork 4k
ARROW-13680: [C++] Create an asynchronous nursery to simplify capture logic #11084
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
Conversation
|
Drafting this because I want to take a quick look to see if I can reuse the existing task group or if the semantics aren't quite right. |
lidavidm
left a comment
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.
Looks good to me. I left a couple small comments on the tests.
cde0d86 to
b6ca920
Compare
|
I've moved this back to ready for review. AsyncTaskGroup is similar but not the same as the TaskGroups. SerialTaskGroup and ThreadedTaskGroup take care of scheduling the task (submitting it to the executor) as well as tracking the task. The AsyncTaskGroup is only responsible for tracking a set of tasks that were scheduled elsewhere. ThreadedTaskGroup could maybe use AsyncTaskGroup through composition but that could be done in a future PR if needed. The scheduling in the dataset writer case happens in the file queue which doesn't quite fit the SerialTaskGroup / ThreadedTaskGroup model because each task should run serially (like SerialTaskGroup) but asynchronously (like ThreadedTaskGroup). I could potentially extract the logic in DatasetWriterFileQueue into a AsyncSerialTaskGroup but for the moment I think what this PR and the dataset writer PR proposes is fine. |
lidavidm
left a comment
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.
LGTM, thanks.
… logic This PR adds two new utilities. The first is an asynchronous smart pointer which makes it easier to ensure that asynchronous tasks are finished before an object is destroyed (and generally makes it safe to capture `this`) The second is an asynchronous task group which collects futures and can help to ensure that all futures are completed. It is similar to AllComplete except it doesn't require collecting all the futures at once. Combined, these two things can help give structured concurrency / nursery type control over asynchronous operations. I have used these utilities in apache#10955 if you would like to see an example of them in action. Closes apache#11084 from westonpace/experiment/async-smart-ptr Authored-by: Weston Pace <weston.pace@gmail.com> Signed-off-by: David Li <li.davidm96@gmail.com>
This PR adds two new utilities.
The first is an asynchronous smart pointer which makes it easier to ensure that asynchronous tasks are finished before an object is destroyed (and generally makes it safe to capture
this)The second is an asynchronous task group which collects futures and can help to ensure that all futures are completed. It is similar to AllComplete except it doesn't require collecting all the futures at once.
Combined, these two things can help give structured concurrency / nursery type control over asynchronous operations. I have used these utilities in #10955 if you would like to see an example of them in action.