[RFC] task_group_dynamic_dependencies

[vossmjp]

Description

A proposal to extend task_group:

1. Extend semantics and useful lifetime of task_handle. We propose task_handle to represent tasks for the purpose of adding dependencies. The useful lifetime and semantics of task_handle will need to be extended to include tasks that have been submitted, are currently executing, or have been completed.
2. Add functions to set task dependencies. In the current task_group, tasks can only be waited for as a group and there is no direct way to add any before-after relationships between individual tasks. We will discuss options for spelling.
3. Add a function to move successors from a currently executing task to a new task. This functionality is necessary for recursively generated task graphs. This case represents a situation where it is safe to modify dependencies for an already submitted task.

Type of change

• 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

[tobiasweinzierl80]

I don't get this grammar. And I think awkward is likely not a good wording.

[tobiasweinzierl80]

Suggestion: However, there is one use case which is not straightforward to express by the revised API: Dynamic task graphs which are not trees.

[vossmjp]

Fixed

[tobiasweinzierl80]

Can we add an "existing" before defer to highlight that this exists already?

[vossmjp]

Fixed

[tobiasweinzierl80]

I notice that task_handle is not extensively documented atm. The docu says what it does, but not why. Can we add more docu there?

[vossmjp]

Yes, I will open a task in internal tracking to improve documentation related to task_handle.

[vossmjp]

Done

[tobiasweinzierl80]

Would it make sense to call this deferred?

[tobiasweinzierl80]

Alternatively, call it "created" in line with the text below

[vossmjp]

I like "deferred" since it matches the defer function name. I think it will accurately imply that it is not yet submitted. I had been concerned that a submitted task that has not-yet-ready-to-run because of dependencies could also be thought of as "deferred", but on second thought, I think deferred is different enough.

[tobiasweinzierl]

I suggest that we are careful with the term "deferred". A deferred task has a specific meaning in OpenMP. While OpenMP is not the "bible", I think it is clever to stick to their semantics or to explicitly avoid their terms where we disagree with their usage. So in this sense, my idea with "deferred" was wrong. I should have thought about this more carefully.

[vossmjp]

Already changed it. Do you have a better suggestion instead? So maybe "created" then?

[tobiasweinzierl]

Yes, created is better. So the story is: People create tasks. By default, they sit within a task group and are deferred or they are part of a task graph and are deferred, too. This is in line with TBB's terminology as well: https://www.intel.com/content/www/us/en/docs/onetbb/developer-guide-api-reference/2021-12/migrating-from-low-level-task-api.html#DEFERRED-TASK-CREATION. With the new interface, they are not deferred but actually immediately spawned even though we add further dependencies later on. Does that make sense?

[tobiasweinzierl80]

The addition "for dependence tracking" is confusing

[vossmjp]

I was trying, again, to differentiate with ready-to-run. Submitted to what? Submitted to the scheduler to be tracked before executing or submitted to the ready queue. But again, I was probably over thinking this. I agree that states of 1. deferred, 2. submitted, 3. executing and 4. completed are clear enough.

[tobiasweinzierl80]

I think this is not 100% correct: If a task is already running and we add a new predecessor, we can read this as a suspend/interrupt. Isn't that what you want to add? We don't need it in our code btw.

[vossmjp]

Hmm, I was not intending to support a suspend. But yes, the language "doesn't make sense" is inaccurate since, as you just pointed out, it can make sense. I'll fix this to match my intention and not be overly broad.

[pavelkumbrasev]

I think this proposal is lacking the final definition of class task_handle including all the new methods.

[akukanov]

Please split the long lines.

[akukanov]

Here it is better to refer to a certain version of the specification, rather than to "current" and "latest" which will change over time. I would also add a link to (or a [short.name] of) the respective section of the spec

[aleksei-fedotov]

A typo:

Suggested change

	that has predecessors that must complete before it can executed, and so passing
	that has predecessors that must complete before it can execute, and so passing

[aleksei-fedotov]

A typo:

Suggested change

	create work while executing a task and insert this newly create work before
	create work while executing a task and insert this newly created work before

[aleksei-fedotov]

Consider being more precise here to avoid variations in interpretation of the meaning:

Suggested change

	this function from outside a task, or passing anything other than a task in
	the created state is undefined behavior.
	this function from outside a task, or passing anything other than a `task_handle`
	representing a task in the created state is undefined behavior.

OR, perhaps, we might want to give more flexibility to the user (see my suggestion about API below). In this case, something like the following may work:

Suggested change

	this function from outside a task, or passing anything other than a task in
	the created state is undefined behavior.
	this function from outside a task, or passing anything other than an object
	representing a task in the created state is undefined behavior.

[aleksei-fedotov]

Since this method is related to the currently executing task, what about including this API into tbb::this_task:: namespace? By analogy with tbb::this_task_arena:: namespace.

[aleksei-fedotov]

A typo:

Suggested change

	Adds `th` as a predecessor that must complete before the task represnted by
	Adds `th` as a predecessor that must complete before the task represented by

[aleksei-fedotov]

These are interesting questions.

While reading this RFC, I kind of rushed proposing additional syntax sugar that besides being more user-friendly, since it seems to represent popular use cases, can save some CPU cycles. So I am posting them here for a discussion.

1. Would it be useful to have a method that simultaneously joins (or even fuses?) the instantiation of a task and transferring of the current task successors?
   Something like:

    template <typename Func>
    task_handle transfer_successors_to(Func&& f);

For the recursive decomposition scenario, instantiating a new task within an executing task and transferring successors to that new task seems to be the main model of writing such algorithms. Although, it seems to be not saving much (only one call to the library and assign to a couple of pointers?), there is always(?) going to be such a sequence in the code. Otherwise, how else an execution of already submitted tasks can be postponed?

Shall we also consider a question of having that API instead or in addition to the one proposed?

2. As for add_predecessor(s) and add_successor(s) I have a couple of thoughts.
   a. It seems again that it might be useful to merge instantiation of the new task handles and adding them as successors/predecessors:

       template <typename Func>
       void add_predecessor(Func&& f);
   
       template <typename Func>
       void add_successor(Func&& f);

   b. Also, I think having an API that would allow adding more than one predecessor/successor at once can be useful, since usually a number of successors/predecessors are instantiated. I only think that we don't need to limit ourselves to only two parameters as it was proposed optionally, but allow passing of an arbitrary size of task handles or even user lambdas. Of course, a pattern of having a single task producer might be viewed as a limiting one, but there actually might be the cases where tasks cannot be made available to the scheduler (i.e. spawned) until all of them are gathered together from different producers, which essentially represents a barrier in the execution pipeline. Not to mention that the spawning of a bunch of tasks all together are done faster than regular one by one spawnings. Spawning of a bunch of tasks at once was implemented in the old TBB, as far as I remember.
   So here I suggest to have something like:

       template <typename... Func>
       void add_predecessors(Func&& ...f);
   
       template <typename... Func>
       void add_successor(Func&& ...f);

   However, this also seems to ask for having the task_group::run() method to accept an arbitrary size of task handles and/or functors. So, perhaps, it is more related to another RFC/extension...