westonpace commented on a change in pull request #8680:
URL: https://github.com/apache/arrow/pull/8680#discussion_r545214296
##########
File path: cpp/src/arrow/util/task_group.cc
##########
@@ -116,25 +109,15 @@ class ThreadedTaskGroup : public TaskGroup {
cv_.wait(lock, [&]() { return nremaining_.load() == 0; });
// Current tasks may start other tasks, so only set this when done
finished_ = true;
- if (parent_) {
- parent_->OneTaskDone();
- }
+ completion_future_.MarkFinished(status_);
}
return status_;
}
int parallelism() override { return executor_->GetCapacity(); }
- std::shared_ptr<TaskGroup> MakeSubGroup() override {
- std::lock_guard<std::mutex> lock(mutex_);
- auto child = new ThreadedTaskGroup(executor_);
- child->parent_ = this;
- nremaining_.fetch_add(1, std::memory_order_acquire);
- return std::shared_ptr<TaskGroup>(child);
- }
-
protected:
- void UpdateStatus(Status&& st) {
+ void UpdateStatus(const Status& st) {
Review comment:
Changed back to `Status&& st`. Not sure what happened here.
##########
File path: cpp/src/arrow/util/task_group.cc
##########
@@ -159,13 +142,13 @@ class ThreadedTaskGroup : public TaskGroup {
Executor* executor_;
std::atomic<int32_t> nremaining_;
std::atomic<bool> ok_;
+ Future<> completion_future_ = Future<>::Make();
Review comment:
Cleaned up
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
