Flow Graph - remove C++03 legacy #945

kboyarinov · 2022-10-25T10:10:48Z

Description

Rewrite Flow Graph join_node and indexer_node to use variadic templates.
Follow up for #437

Fixes # - issue number(s) if exists

- git commit message contains an appropriate signed-off-by string (see CONTRIBUTING.md for details)

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

knoepfel · 2023-12-06T19:28:37Z

What is the status of this PR?

kboyarinov · 2023-12-07T12:12:55Z

@lin72h @knoepfel

Thank you for expressing your interest into this PR.
Current status of this change for us is a low priority enhancement of the FG implementation and there are still things that should be investigated, e.g., backward compatibility.
Could you please share your use cases that finds this change useful if any? It would be very helpful for pushing this activity forward.

knoepfel · 2023-12-13T17:47:20Z

@kboyarinov, thanks for reaching out. I have been using oneTBB flow for a while, and have greatly enjoyed it. One of the use cases I have is that some algorithms/nodes invoke library code that is not thread-safe. It is not sufficient to simply specify a concurrency level of 1 for that node, because there are other nodes that might also call the same library at the same time. Instead, for nodes that have to call the same thread-unsafe resource, I use a join node where one of the input ports receives a serialization token--only one token exists in the full graph for a given thread-unsafe library, and the node will not execute until it receives the token. And sometimes more than one token is required if there are more than one thread-unsafe libraries that must be invoked by the node.

I also use join nodes, though, with tag-matching to ensure that all of the arguments required by a particular algorithm are available before the algorithm is executed. If the algorithm requires $m$ serialization tokens, with the current implementation of the join node, that leaves $10-m$ ports for algorithm arguments. Hopefully, an algorithm will not need a significant number of serialization tokens, and a large number of algorithm arguments, but with the current implementation, I have to think about the 10-port max restriction, instead of just letting the compiler specify a maximum number of arguments as allowed in a variadic parameter pack.

Does that help explain the situation? I can also produce a diagram if that would be helpful.

aleksei-fedotov · 2025-11-04T11:47:46Z

test/conformance/conformance_flowgraph.h

+template <std::size_t N>
+struct edge_maker {
+    template <typename Sender, typename NodeType>
+    static void make(Sender& sender, NodeType& node) {
+        oneapi::tbb::flow::make_edge(sender, oneapi::tbb::flow::input_port<N - 1>(node));
+        edge_maker<N - 1>::make(sender, node);
+    }
+
+    template <typename Sender, typename NodeType>
+    static void make(std::vector<Sender>& senders, NodeType& node) {
+        oneapi::tbb::flow::make_edge(senders[N - 1], oneapi::tbb::flow::input_port<N - 1>(node));
+        edge_maker<N - 1>::make(senders, node);
+    }
+};
+
+template <>
+struct edge_maker<0> {
+    template <typename Sender, typename NodeType>
+    static void make(Sender&, NodeType&) {}
+};


Maybe move graph-related helpers into common "graph-helpers" header?

Moved to common/graph_utils.h

aleksei-fedotov · 2025-11-04T11:49:42Z

test/conformance/conformance_join_node.cpp

+    for (auto& buffer : buffers) {
+        make_edge(submitter, buffer);
+    }


Add a helper in edge_maker for this as well?

I would prefer to keep it as-is since it can be implemented as a runtime loop and edge_maker is intended to implement a compile-time loop since input_port<N> should be used. I don't think it makes sense to overcomplicate it by implementing using a compile-time logic.

aleksei-fedotov · 2025-11-04T11:58:52Z

test/conformance/conformance_join_node.cpp

+
+    std::size_t body_counter = 0;
+    function_node<output_tuple> function(g, serial, [&](const output_tuple& tuple) {
+        tuple_assert<N>(tuple, message);


Rename to something more readable? For example,

Suggested change

tuple_assert<N>(tuple, message);

assert_all_items_equal_to<N>(tuple, message);

Similarly to edge_maker, this can also be extended to support other containers.
By the way, std::get already supports other collections: std::array, std::pair, std::variant, etc. So, such helper will automatically support them.
Therefore, consider renaming (since tuple could already be not necessarily a tuple) and moving this useful helper to some common place.

Renamed tuple_assert, moved to common header and renamed first argument to be tuple-like.
I would not add tuple-like support to edge_maker now since it will require adding constraint that std::get is valid to disambiguate with the existing overload of edge_maker::make. It is not used in the existing tests, so I would it as-is for now.

include/oneapi/tbb/flow_graph.h

include/oneapi/tbb/detail/_flow_graph_join_impl.h

aleksei-fedotov · 2025-11-04T16:18:47Z

include/oneapi/tbb/detail/_flow_graph_join_impl.h

+        unfolded_join_node(graph &g)
+            : base_type(g,
+                        func_initializer_type(new type_to_key_function_body_leaf<Types, K,
+                                                                                 key_from_message_body<K, Types>>(key_from_message_body<K, Types>())...))
+        {}


Can we also replace type_to_key_function_body[_leaf] types with std::function?

I think it is a good idea, but I would consider it as part of the separate PR.
Otherwise, this one would be overcomplicated

Co-authored-by: Aleksei Fedotov <[email protected]>

kboyarinov added 2 commits October 24, 2022 08:52

Get rid of C++03 rudiments

cb8fa21

Fix indexer_node implementation, add test

0baaad6

akukanov mentioned this pull request Aug 1, 2025

WIP: Rewrite join_node with variadic templates #437

Closed

kboyarinov added 2 commits October 30, 2025 16:50

Merge remote-tracking branch 'origin/master' into variadic_indexer_node

d4f3515

Add test for join_node, polish the implementation

5b19907

kboyarinov added this to the 2022.4.0 milestone Oct 31, 2025

kboyarinov requested review from aleksei-fedotov, dnmokhov, isaevil and vossmjp October 31, 2025 10:21

Fix copyright

c39a554

kboyarinov changed the title ~~WIP: Flow Graph - remove C++03 legacy~~ Flow Graph - remove C++03 legacy Oct 31, 2025

aleksei-fedotov reviewed Nov 4, 2025

View reviewed changes

kboyarinov and others added 14 commits November 4, 2025 19:42

Update include/oneapi/tbb/flow_graph.h

a95622b

Co-authored-by: Aleksei Fedotov <[email protected]>

Update include/oneapi/tbb/flow_graph.h

0ee0ac0

Co-authored-by: Aleksei Fedotov <[email protected]>

Update include/oneapi/tbb/flow_graph.h

48e40ec

Co-authored-by: Aleksei Fedotov <[email protected]>

Update include/oneapi/tbb/flow_graph.h

32989bf

Co-authored-by: Aleksei Fedotov <[email protected]>

Update include/oneapi/tbb/flow_graph.h

235a329

Co-authored-by: Aleksei Fedotov <[email protected]>

Update include/oneapi/tbb/flow_graph.h

faaa241

Co-authored-by: Aleksei Fedotov <[email protected]>

Update include/oneapi/tbb/flow_graph.h

7321b09

Co-authored-by: Aleksei Fedotov <[email protected]>

Update include/oneapi/tbb/flow_graph.h

d8458c7

Co-authored-by: Aleksei Fedotov <[email protected]>

Update include/oneapi/tbb/flow_graph.h

b6056bc

Co-authored-by: Aleksei Fedotov <[email protected]>

Update include/oneapi/tbb/detail/_flow_graph_indexer_impl.h

d3c8d01

Co-authored-by: Aleksei Fedotov <[email protected]>

Update include/oneapi/tbb/detail/_flow_graph_join_impl.h

0ea72ea

Co-authored-by: Aleksei Fedotov <[email protected]>

Update include/oneapi/tbb/detail/_flow_graph_join_impl.h

ea71d40

Co-authored-by: Aleksei Fedotov <[email protected]>

Update include/oneapi/tbb/detail/_flow_graph_join_impl.h

74f0043

Co-authored-by: Aleksei Fedotov <[email protected]>

Update include/oneapi/tbb/detail/_flow_graph_join_impl.h

a5b5304

Co-authored-by: Aleksei Fedotov <[email protected]>

Apply review comments

78213c7

	tuple_assert<N>(tuple, message);
	assert_all_items_equal_to<N>(tuple, message);

Flow Graph - remove C++03 legacy #945

Are you sure you want to change the base?

Flow Graph - remove C++03 legacy #945

Uh oh!

Conversation

kboyarinov commented Oct 25, 2022

Description

Type of change

Tests

Documentation

Breaks backward compatibility

Notify the following users

Other information

Uh oh!

knoepfel commented Dec 6, 2023

Uh oh!

kboyarinov commented Dec 7, 2023

Uh oh!

knoepfel commented Dec 13, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

knoepfel commented Dec 13, 2023 •

edited

Loading