RFC for Waiting for Individual Tasks in task_group

[kboyarinov]

Description

Add an RFC for new functions that allow waiting for Individual Tasks in task_group:

namespace oneapi {
namespace tbb {
enum task_group_status { // Existing API
    not_complete,        // Existing API
    complete,               // Existing API
    canceled,                // Existing API
    task_complete        // Proposed API 
}

class task_group {
    task_group_status wait_for_task(task_completion_handle&);
    task_group_status run_and_wait_for_task(task_handle&&);
};

class task_arena {
   task_group_status wait_for(task_completion_handle&);
};

} // namespace tbb
} // namespace oneapi

Fixes # - issue number(s) if exists

Type of change

Choose one or multiple, leave empty if none of the other choices apply

Add a respective label(s) to PR if you have permissions

• bug fix - change that fixes an issue
• new feature - change that adds functionality
• tests - change in tests
• infrastructure - change in infrastructure and CI
• documentation - documentation update

Tests

• added - required for new features and some bug fixes
• not needed

Documentation

• updated in # - add PR number
• needs to be updated
• not needed

Breaks backward compatibility

• Yes
• No
• Unknown

Notify the following users

List users with @ to send notifications

Other information

[vossmjp]

Will transfer of a task_completion lead to any confusion around cancellation, if for example, the original task has executed but the task that was transfered to is canceled? The user will see canceled as a status even though the initial task did complete. I think it's fine and will just need to be well documented in transfer.

[kboyarinov]

I think that if the completion of the task was transferred to another task, the task status after the wait is also transferred. I agree that it should be clearly documented. Added explicit mention in the RFC.

[akukanov]

I would consider extending the existing enum with one more value, named e.g. task_complete, which would be returned by the single-task waiting functions instead of complete.

[kboyarinov]

When other waiting functions return task_group_status::canceled, they rely on the cancellation of the group (i.e. cancellation of the associated task_group_context). Individual tasks can be both executed or canceled.
For single-task waiting functions, this flag would mean different thing - that the task we are waiting for was canceled.

And even if the task group execution was canceled, the single-task wait can return task_complete. And the returned task_group_status knows nothing about the status of task group.

I think separating the statuses would be more obvious for users.

[akukanov]

For single-task waiting functions, this flag would mean different thing - that the task we are waiting for was canceled.

No, it would mean the same thing - that the task group, which task we are waiting for, was cancelled. There is no way to cancel a single task.

And even if the task group execution was canceled, the single-task wait can return task_complete. And the returned task_group_status knows nothing about the status of task group.

Sure, and I see no problem with that. The task was complete, while the status of the whole group is unknown. On the other hand, if the task was cancelled, then the whole group was cancelled.

[kboyarinov]

For canceled, I agree, for run_and_wait_task it means the awaited task was not executed because the cancellation of the task_group execution.
For complete, I find it a bit misleading to have a status of task_group that knows nothing about the actual status of the group and serves the status of the task instead.

[vossmjp]

Talking with @kboyarinov earlier today, we discussed that for an individual task, a good set of status values would be "executed" and "canceled", instead of complete and canceled. A task might finish before a task group is canceled. Or a long-running task could execute and then discover while executing, by querying its task_group_context, that its group has been canceled and short-cut its execution. So for a specific task, "executed" simply means that the scheduler executed the task but does not imply anything about completion of the work or other work in the task group. And then "canceled" means the task never started to execute.

[akukanov]

I would not object to a different status name, but then I think we would need to adjust task_completion_handle and the transfer method name accordingly - to me, these all are aligned and speak of the same thing from different angles.

What do you mean by "short-circuited"? If you mean a user action within the task to check the group status and adjust the task behavior, there is no way for task_group to know that - for all we know, it was executed and either signaled completion or transferred it to another task. And I do not see it different for the task group complete status, really - it also does not mean "a full and valid result", it only means all tasks in the group have been executed - even though they may have short-circuited due to some external condition.

[akukanov]

What I think makes sense to clarify is when/how the group cancellation status is checked during the task waiting call. For example, can the implementation signal all the pending task waiting calls as "canceled" once the group is cancelled - even if a particular task is in the middle of execution? What if the task transfers its completion, and then the group is canceled and the "continuation" task is not executed?

[kboyarinov]

I don't believe it is correct for semantics and implementation to "early-exit" the wait call with the canceled returned state if the group execution is cancelled. It seems for me that since the state of the particular task (or task tree) is important for the user, the implementation should wait to get cancelation signal from the task, not a group. It may be important for the user to know that the task is completed even if the whole group is canceled.
The proposed implementation follows this pattern - if task::execute was called for the awaited task, the wait function returns "task_complete", if task::cancel - "canceled". If the task's completion was transferred, it just waits for such a signal from the task, receiving the completion.

[vossmjp]

I think it is important to distinguish between whether a task (or its transferred completion) has been skipped due to cancellation or has executed. My concern comes from the existence of task_group_status::complete that means the work in a task_group is done and was not interrupted by an exception or cancellation that set the task group status to canceled. The status task_group_status::task_complete means the task or transferred completion was executed and was not interrupted by an exception. Its logic may have been affected (which I have called short-circuited) by cancellation since it can query cancellation and bail out early. Querying of the task_group cancellation status and the performing an early exit is a pattern we endorse. I do agree that we should NOT override the result in the task waiting call by looking at the task group's status after the task has returned. I just think it would be better if we didn't call it task_complete. But if I'm the only one who thinks that task_complete might be confusing, then I won't press it.

[akukanov]

There can be three situations:

1. the task group is not canceled at the moment the last task in the transfer chain has executed

• it can be additionally subdivided, depending on whether the task group is canceled later.

2. the task group has been canceled to the moment the last task in the transfer chain has executed
3. the task group is canceled and at least one task in the transfer chain has not executed

It's only the second case where short-circuiting via task_group status query is in theory possible. And I expect it to be a rare case, comparing to the normal execution. So if we want to indicate a possibility of this case, I think we need a third status value for that, rather than deciding that the case 2 defines the status name also for 1.

I prefer to use task_complete for both, but I would also not object to distinct statuses.

[akukanov]

Are we able to get the task group out of a completion handle? If not, are we able to implement the function without requiring a task group to be also provided by the caller?

[kboyarinov]

With our current implementation, task_group is not required for a single-task waiting. All we need is a task_dynamic_state and the associated task_group_context, both can be obtained from the task object.

In theory, we can implement task_group::wait_task (and even run_and_wait_task if we omit the "same task group" check) as a static function. But I have proposed member functions for consistency with other waiting functions.

I am not sure if implementing task_arena::wait_for wihout the task group is possible for any other TBB implementation. From the perspective of the further inclusion into oneTBB specification, it may make sense to add task_group argument into this function and keep it unused in our implementation. What do you think?

[akukanov]

Sure it is possible for any implementation, under the assumption that a task_completion_handle (and perhaps also task_handle) is always "bound" to a certain task group (that is, can keep a pointer/reference to the group). We just need to be clear about that, as well as at which point the binding happens (I guess that is at task creation, and not at submission). Otherwise, it is a mystery where tg comes from in the "equivalent" expression.

[kboyarinov]

I have added the clarification about what tg is in this case.

I agree that this can be approached by binding the handle to the task_group. But I think that we should not specify how binding should be implemented (to ensure the validity of our implementation).

[akukanov]

True, we should not specify how the binding is to be implemented. When I said a task handle "can keep a pointer/reference", I did not mean it should, just that an implementation that takes this approach is valid and safe.

Specifying when binding happens is different, as it affects which task group to wait for, Waiting on a wrong task group could result in a program hang.

[akukanov]

What are benefits and downsides of the alternative approaches to the recommended one?

[kboyarinov]

The main benefit of the recommended implementation approach is that waiting for completion is guaranteed to be implemented using a single r1::wait or r1::run_and_wait in case of transferring.

For both alternative approaches, we will need to switch to another waiting in case of transferring:

task_dynamic_state* state = comp_handle.get_dynamic_state();

r1::wait(state->get_wait_context());

while (state->was_transferred()) {
    state = state->get_new_completion_point();
    r1::wait(state->get_wait_context());
}

With the recommended approach, in case of transferring the wait context pointer is migrating between tasks, hence we don't need to double check if we completion was transferred. If the wait context was released, the completion is guaranteed to happen (no matter of which "final" task in the transfer chain).

Another benefit is that the wait context is created only when the wait was requested (that is not true for the first alternative approach).

I will add more details on the benefits and downsides into the RFC.

[kboyarinov]

Updated each section with the pros and cons.

[akukanov]

So, are the alternative implementation approaches considered and rejected, or is a further discussion necessary?

[kboyarinov]

I believe that the natural support for completion transferring in the proposed approach provides a strong rationale for its implementation over other alternatives. If reviewers see it differently, we can discuss further.

[akukanov]

It's a well-elaborated proposal. I still have some questions and suggestions, though.

[akukanov]

So, are the alternative implementation approaches considered and rejected, or is a further discussion necessary?

[vossmjp]

I can't remember if we previously discussed this but wait_task seems to me like a getter that returns the wait task. Shouldn't this be something like wait_for_task.

[akukanov]

See #1862 (comment)
I am OK with wait_for_task

[kboyarinov]

Agree, renamed to wait_for_task

[vossmjp]

Same issue here, why not run_and_wait_for_task

[kboyarinov]

Done

[vossmjp]

Ok so, the task_arena::wait_for that receives a task completion handle is an overload of task_arena::wait_for that receives a task group. So a consistent name there. Do we want consistency with names in task_group, which now has task_group::wait_for_task and task_group::wait. Even though I recently suggested task_group::wait_for_task, would it be better as task_group::wait_for? And then task_group::run_and_wait_for?

[kboyarinov]

As Alexey noted in #1862 (comment), run_and_wait_for and the existing run_and_wait appear quite similar, so using a more explicit name run_and_wait_for_task would help avoid misinterpretation. I agree with this suggestion.

[aleksei-fedotov]

Thanks, Mike! I also wanted to suggest such consistent naming.
In my opinion that this _for suffix is what helps to differentiate. Without that suffix, it waits for all the tasks inside an entity the method is called on - task_group::wait() without parameters signifies just that and helps to understand the semantics of run_and_wait. While the _for suffix indicates that it is the passed argument the method is going to wait for.
Besides becoming inconsistent with the task_arena, the structures like tg.wait_for_task(task) is too repetitive on the word task.

[akukanov]

In my opinion that this _for suffix is what helps to differentiate.

Maybe for wait vs. wait_for, which also differ in their arguments, but not quite enough for run_and_wait vs. run_and_wait_for, which look the same except for the name.

Besides becoming inconsistent with the task_arena

Names in task_group and task_arena were never consistent :)
The former class has run, run_and_wait, and wait, while in the latter we have execute, enqueue, wait_for.

the structures like tg.wait_for_task(task) is too repetitive on the word task.

Remember the argument of it is not a task but a completion handle, and so is quite unlikely to be called just task; I would expect either some meaningful name or an abbreviation like tch.

I guess we will have to just disagree there and find the way to resolve that disagreement. I strongly prefer semantical clarity over consistency with a loosely related method of another class.