-
Notifications
You must be signed in to change notification settings - Fork 1k
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
Flow Graph scalability improvements #1348
Conversation
af785df
to
bdd7ba6
Compare
aee5fb2
to
3fe11f3
Compare
bdd7ba6
to
bd9a34c
Compare
Signed-off-by: pavelkumbrasev <[email protected]>
Signed-off-by: pavelkumbrasev <[email protected]>
Signed-off-by: pavelkumbrasev <[email protected]>
Signed-off-by: pavelkumbrasev <[email protected]>
…educe Signed-off-by: pavelkumbrasev <[email protected]>
Signed-off-by: pavelkumbrasev <[email protected]>
7936fbb
to
412c8de
Compare
Co-authored-by: Pavel Kumbrasev <[email protected]>
// and the task should be associated with the graph wait context itself | ||
// TODO: consider how reference counting can be improved for such a use case. Most common example is the async_node | ||
d1::wait_context_vertex* graph_wait_context_vertex = &my_graph.get_wait_context_vertex(); | ||
my_reference_vertex = is_thread_in_graph_arena(g) ? r1::get_thread_reference_vertex(graph_wait_context_vertex) |
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.
I believe this can happen even without async_node
. Consider the following case:
std::thread t1([]() {
tbb::task_arena& gta = get_graph_task_arena();
gta.execute([]() {
auto& some_graph_node = get_graph_node();
some_graph_node.try_put(/*node message*/1); // spawn of task A
});
});
t1.join();
// task A starts its execution
graph.wait_for_all();
Don't we support such cases anymore?
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.
In this case it is guaranteed that t1
will either join the gta
and execute the task if there is an available slot or send a task and it would be executed by another worker in gta
. In each case, the thread executing the lambda and calling try_put
is guaranteed to be in gta
and hence has its reference node in the TLS (the code will choose the true
branch).
If t1
would submit work in the graph without explicitly calling task_arena::execute
, the false
branch would be chosen and the task reference counting would work as before this PR by adding a reference to graph wait_context.
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.
This reverts commit 4ae4781.
Signed-off-by: pavelkumbrasev <[email protected]> Co-authored-by: pavelkumbrasev <[email protected]>
Description
Add a comprehensive description of proposed changes
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
Tests
Documentation
Breaks backward compatibility
Notify the following users
List users with
@
to send notificationsOther information