From 965c211100deb74337b6aa897d6c7805e4a24465 Mon Sep 17 00:00:00 2001 From: pavelkumbrasev Date: Tue, 19 Sep 2023 14:23:49 +0100 Subject: [PATCH 01/10] Fix race on continuation finalization Signed-off-by: pavelkumbrasev --- .../task_emulation_layer.h | 49 +++++++++++++------ 1 file changed, 33 insertions(+), 16 deletions(-) diff --git a/examples/migration/recursive_fibonacci/task_emulation_layer.h b/examples/migration/recursive_fibonacci/task_emulation_layer.h index 3a38712765..917167f8c4 100644 --- a/examples/migration/recursive_fibonacci/task_emulation_layer.h +++ b/examples/migration/recursive_fibonacci/task_emulation_layer.h @@ -47,22 +47,28 @@ class base_task { public: base_task() = default; - base_task(const base_task& t) : m_parent(t.m_parent), m_child_counter(t.m_child_counter.load()) + base_task(const base_task& t) : m_parent(t.m_parent), m_ref_counter(t.m_ref_counter.load()) {} virtual ~base_task() = default; void operator() () const { base_task* parent_snapshot = m_parent; + task_type type_snapshot = m_type; + const_cast(this)->execute(); - if (m_parent && parent_snapshot == m_parent && m_child_counter == 0) { - if (m_parent->remove_reference() == 0) { + + if (m_parent && parent_snapshot == m_parent && type_snapshot == m_type) { + m_parent->add_self_ref(); + auto child_ref = m_parent->remove_child_reference() & (m_self_ref - 1); + if (child_ref == 0) { m_parent->operator()(); + } else if (m_parent->remove_self_ref() == 0) { delete m_parent; } } - if (m_child_counter == 0 && m_type == task_type::allocated) { + if (m_type != task_type::stack_based && const_cast(this)->remove_self_ref() == 0) { delete this; } } @@ -74,7 +80,7 @@ class base_task { C* continuation = new C{std::forward(args)...}; continuation->m_type = task_type::continuation; continuation->reset_parent(reset_parent()); - continuation->m_child_counter = ref; + continuation->m_ref_counter = ref; return continuation; } @@ -85,7 +91,7 @@ class base_task { template F create_child_and_increment(Args&&... args) { - add_reference(); + add_child_reference(); return create_child_impl(std::forward(args)...); } @@ -96,7 +102,7 @@ class base_task { template F* allocate_child_and_increment(Args&&... args) { - add_reference(); + add_child_reference(); return allocate_child_impl(std::forward(args)...); } @@ -109,17 +115,17 @@ class base_task { m_type = task_type::continuation; } - void add_reference() { - ++m_child_counter; + void add_child_reference() { + ++m_ref_counter; } - std::uint64_t remove_reference() { - return --m_child_counter; + std::uint64_t remove_child_reference() { + return --m_ref_counter; } protected: enum class task_type { - created, + stack_based, allocated, continuation }; @@ -136,7 +142,7 @@ class base_task { template F create_child_impl(Args&&... args) { F obj{std::forward(args)...}; - obj.m_type = task_type::created; + obj.m_type = task_type::stack_based; obj.reset_parent(this); return obj; } @@ -145,6 +151,7 @@ class base_task { F* allocate_child_impl(Args&&... args) { F* obj = new F{std::forward(args)...}; obj->m_type = task_type::allocated; + obj->add_self_ref(); obj->reset_parent(this); return obj; } @@ -155,14 +162,23 @@ class base_task { return p; } + void add_self_ref() { + m_ref_counter.fetch_add(m_self_ref); + } + + std::uint64_t remove_self_ref() { + return m_ref_counter.fetch_sub(m_self_ref) - m_self_ref; + } + base_task* m_parent{nullptr}; - std::atomic m_child_counter{0}; + static constexpr std::uint64_t m_self_ref = std::uint64_t(1) << 48; + std::atomic m_ref_counter{0}; }; class root_task : public base_task { public: root_task(tbb::task_group& tg) : m_tg(tg), m_callback(m_tg.defer([] { /* Create empty callback to preserve reference for wait. */})) { - add_reference(); + add_child_reference(); m_type = base_task::task_type::continuation; } @@ -178,7 +194,7 @@ class root_task : public base_task { template F create_root_task(tbb::task_group& tg, Args&&... args) { F obj{std::forward(args)...}; - obj.m_type = base_task::task_type::created; + obj.m_type = base_task::task_type::stack_based; obj.reset_parent(new root_task{tg}); return obj; } @@ -187,6 +203,7 @@ template F* allocate_root_task(tbb::task_group& tg, Args&&... args) { F* obj = new F{std::forward(args)...}; obj->m_type = base_task::task_type::allocated; + obj->add_self_ref(); obj->reset_parent(new root_task{tg}); return obj; } From 6210016ab556184d2afd6223ad420cbf0f0d21bf Mon Sep 17 00:00:00 2001 From: pavelkumbrasev Date: Tue, 19 Sep 2023 15:57:49 +0100 Subject: [PATCH 02/10] Move reference counter increment Signed-off-by: pavelkumbrasev --- examples/migration/recursive_fibonacci/task_emulation_layer.h | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/examples/migration/recursive_fibonacci/task_emulation_layer.h b/examples/migration/recursive_fibonacci/task_emulation_layer.h index 917167f8c4..ed9be5eedf 100644 --- a/examples/migration/recursive_fibonacci/task_emulation_layer.h +++ b/examples/migration/recursive_fibonacci/task_emulation_layer.h @@ -59,12 +59,9 @@ class base_task { const_cast(this)->execute(); if (m_parent && parent_snapshot == m_parent && type_snapshot == m_type) { - m_parent->add_self_ref(); auto child_ref = m_parent->remove_child_reference() & (m_self_ref - 1); if (child_ref == 0) { m_parent->operator()(); - } else if (m_parent->remove_self_ref() == 0) { - delete m_parent; } } @@ -112,6 +109,7 @@ class base_task { } void recycle_as_continuation() { + add_self_ref(); m_type = task_type::continuation; } From ba797b98259568c244199b94e1f02edff5419e12 Mon Sep 17 00:00:00 2001 From: Dmitri Mokhov Date: Tue, 19 Sep 2023 22:35:50 -0500 Subject: [PATCH 03/10] Copy m_type in base_task copy constructor. Use add_self_ref in allocated continuations and root_task to prevent leaks. --- .../task_emulation_layer.h | 20 ++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/examples/migration/recursive_fibonacci/task_emulation_layer.h b/examples/migration/recursive_fibonacci/task_emulation_layer.h index ed9be5eedf..fd8f3cc2b7 100644 --- a/examples/migration/recursive_fibonacci/task_emulation_layer.h +++ b/examples/migration/recursive_fibonacci/task_emulation_layer.h @@ -47,7 +47,7 @@ class base_task { public: base_task() = default; - base_task(const base_task& t) : m_parent(t.m_parent), m_ref_counter(t.m_ref_counter.load()) + base_task(const base_task& t) : m_type(t.m_type), m_parent(t.m_parent), m_ref_counter(t.m_ref_counter.load()) {} virtual ~base_task() = default; @@ -78,6 +78,7 @@ class base_task { continuation->m_type = task_type::continuation; continuation->reset_parent(reset_parent()); continuation->m_ref_counter = ref; + continuation->add_self_ref(); return continuation; } @@ -122,6 +123,14 @@ class base_task { } protected: + void add_self_ref() { + m_ref_counter.fetch_add(m_self_ref); + } + + std::uint64_t remove_self_ref() { + return m_ref_counter.fetch_sub(m_self_ref) - m_self_ref; + } + enum class task_type { stack_based, allocated, @@ -160,14 +169,6 @@ class base_task { return p; } - void add_self_ref() { - m_ref_counter.fetch_add(m_self_ref); - } - - std::uint64_t remove_self_ref() { - return m_ref_counter.fetch_sub(m_self_ref) - m_self_ref; - } - base_task* m_parent{nullptr}; static constexpr std::uint64_t m_self_ref = std::uint64_t(1) << 48; std::atomic m_ref_counter{0}; @@ -177,6 +178,7 @@ class root_task : public base_task { public: root_task(tbb::task_group& tg) : m_tg(tg), m_callback(m_tg.defer([] { /* Create empty callback to preserve reference for wait. */})) { add_child_reference(); + add_self_ref(); m_type = base_task::task_type::continuation; } From d3b65b18aa83ea4ab7ac5810a8a1662fc8c7c597 Mon Sep 17 00:00:00 2001 From: Dmitri Mokhov Date: Wed, 20 Sep 2023 00:05:34 -0500 Subject: [PATCH 04/10] Recycle the task one more time --- .../recursive_fibonacci/fibonacci_single_task.h | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/examples/migration/recursive_fibonacci/fibonacci_single_task.h b/examples/migration/recursive_fibonacci/fibonacci_single_task.h index 2467f862b0..c87a3c010f 100644 --- a/examples/migration/recursive_fibonacci/fibonacci_single_task.h +++ b/examples/migration/recursive_fibonacci/fibonacci_single_task.h @@ -32,7 +32,8 @@ long serial_fib_1(int n) { struct single_fib_task : task_emulation::base_task { enum class state { compute, - sum + sum, + print_status }; single_fib_task(int n, int* x) : n(n), x(x), s(state::compute) @@ -46,6 +47,16 @@ struct single_fib_task : task_emulation::base_task { } case state::sum : { *x = x_l + x_r; + + // Recycling + this->s = state::print_status; + this->recycle_as_continuation(); + this->operator()(); + + break; + } + case state::print_status: { + std::cout << std::to_string(n) + "\n"; break; } } From 14ab39bb91c9914edcdf12bb41675e87291582f4 Mon Sep 17 00:00:00 2001 From: pavelkumbrasev Date: Wed, 20 Sep 2023 13:46:36 +0100 Subject: [PATCH 05/10] Enable multiple recycles Signed-off-by: pavelkumbrasev --- .../task_emulation_layer.h | 24 ++++++++++--------- 1 file changed, 13 insertions(+), 11 deletions(-) diff --git a/examples/migration/recursive_fibonacci/task_emulation_layer.h b/examples/migration/recursive_fibonacci/task_emulation_layer.h index fd8f3cc2b7..b1e96f0b78 100644 --- a/examples/migration/recursive_fibonacci/task_emulation_layer.h +++ b/examples/migration/recursive_fibonacci/task_emulation_layer.h @@ -47,25 +47,28 @@ class base_task { public: base_task() = default; - base_task(const base_task& t) : m_type(t.m_type), m_parent(t.m_parent), m_ref_counter(t.m_ref_counter.load()) + base_task(const base_task& t) : m_type(t.m_type.load()), m_parent(t.m_parent), m_ref_counter(t.m_ref_counter.load()) {} virtual ~base_task() = default; void operator() () const { base_task* parent_snapshot = m_parent; - task_type type_snapshot = m_type; + std::uint64_t type_snapshot = m_type; const_cast(this)->execute(); - if (m_parent && parent_snapshot == m_parent && type_snapshot == m_type) { + bool is_task_recycled_as_child = parent_snapshot != m_parent; + bool is_task_recycled_as_continuation = type_snapshot != m_type; + + if (m_parent && !is_task_recycled_as_child && !is_task_recycled_as_continuation) { auto child_ref = m_parent->remove_child_reference() & (m_self_ref - 1); if (child_ref == 0) { m_parent->operator()(); } } - if (m_type != task_type::stack_based && const_cast(this)->remove_self_ref() == 0) { + if (type_snapshot != task_type::stack_based && const_cast(this)->remove_self_ref() == 0) { delete this; } } @@ -111,7 +114,7 @@ class base_task { void recycle_as_continuation() { add_self_ref(); - m_type = task_type::continuation; + m_type += task_type::continuation; } void add_child_reference() { @@ -131,13 +134,13 @@ class base_task { return m_ref_counter.fetch_sub(m_self_ref) - m_self_ref; } - enum class task_type { - stack_based, - allocated, - continuation + struct task_type { + static constexpr std::uint64_t stack_based = 1; + static constexpr std::uint64_t allocated = 1 << 1; + static constexpr std::uint64_t continuation = 1 << 2; }; - task_type m_type; + std::atomic m_type; private: template @@ -178,7 +181,6 @@ class root_task : public base_task { public: root_task(tbb::task_group& tg) : m_tg(tg), m_callback(m_tg.defer([] { /* Create empty callback to preserve reference for wait. */})) { add_child_reference(); - add_self_ref(); m_type = base_task::task_type::continuation; } From 0cba293e58da0a2c5258614968a1dfcb796012bf Mon Sep 17 00:00:00 2001 From: pavelkumbrasev Date: Wed, 20 Sep 2023 14:18:53 +0100 Subject: [PATCH 06/10] Extend fibonacci with continuation recursion Signed-off-by: pavelkumbrasev --- .../recursive_fibonacci/CMakeLists.txt | 2 +- .../recursive_fibonacci/fibonacci.cpp | 2 ++ .../fibonacci_single_task.h | 19 +++++++++---------- 3 files changed, 12 insertions(+), 11 deletions(-) diff --git a/examples/migration/recursive_fibonacci/CMakeLists.txt b/examples/migration/recursive_fibonacci/CMakeLists.txt index 5032da2330..3967a0a2a9 100644 --- a/examples/migration/recursive_fibonacci/CMakeLists.txt +++ b/examples/migration/recursive_fibonacci/CMakeLists.txt @@ -33,7 +33,7 @@ set(EXECUTABLE "$") # `N` - specifies the fibonacci number which would be calculated. # `C` - cutoff that will be used to stop recursive split. # `I` - number of iteration to measure benchmark time. -set(ARGS 30 16 20) +set(ARGS 30 16 20 1) set(PERF_ARGS 50 5 20) add_execution_target(run_recursive_fibonacci recursive_fibonacci ${EXECUTABLE} "${ARGS}") diff --git a/examples/migration/recursive_fibonacci/fibonacci.cpp b/examples/migration/recursive_fibonacci/fibonacci.cpp index acf22a497a..a2f20c4424 100644 --- a/examples/migration/recursive_fibonacci/fibonacci.cpp +++ b/examples/migration/recursive_fibonacci/fibonacci.cpp @@ -22,6 +22,7 @@ #include int cutoff; +bool tesing_enabled; template std::pair measure(F&& f, @@ -48,6 +49,7 @@ int main(int argc, char* argv[]) { int numbers = argc > 1 ? strtol(argv[1], nullptr, 0) : 50; cutoff = argc > 2 ? strtol(argv[2], nullptr, 0) : 16; unsigned long ntrial = argc > 3 ? (unsigned long)strtoul(argv[3], nullptr, 0) : 20; + tesing_enabled = argc > 4 ? (bool)strtol(argv[4], nullptr, 0) : false; auto res = measure(fibonacci_two_tasks, numbers, ntrial); std::cout << "Fibonacci two tasks impl N = " << res.first << " Avg time = " << res.second diff --git a/examples/migration/recursive_fibonacci/fibonacci_single_task.h b/examples/migration/recursive_fibonacci/fibonacci_single_task.h index c87a3c010f..12be6644d7 100644 --- a/examples/migration/recursive_fibonacci/fibonacci_single_task.h +++ b/examples/migration/recursive_fibonacci/fibonacci_single_task.h @@ -24,6 +24,7 @@ #include extern int cutoff; +extern bool tesing_enabled; long serial_fib_1(int n) { return n < 2 ? n : serial_fib_1(n - 1) + serial_fib_1(n - 2); @@ -32,8 +33,7 @@ long serial_fib_1(int n) { struct single_fib_task : task_emulation::base_task { enum class state { compute, - sum, - print_status + sum }; single_fib_task(int n, int* x) : n(n), x(x), s(state::compute) @@ -48,17 +48,15 @@ struct single_fib_task : task_emulation::base_task { case state::sum : { *x = x_l + x_r; - // Recycling - this->s = state::print_status; - this->recycle_as_continuation(); - this->operator()(); + if (tesing_enabled) { + if (n == cutoff && num_recycles > 0) { + --num_recycles; + compute_impl(); + } + } break; } - case state::print_status: { - std::cout << std::to_string(n) + "\n"; - break; - } } } @@ -90,6 +88,7 @@ struct single_fib_task : task_emulation::base_task { state s; int x_l{ 0 }, x_r{ 0 }; + int num_recycles{5}; }; int fibonacci_single_task(int n) { From b4572f0d1c493d7c10cda067456493d7f23d8bfe Mon Sep 17 00:00:00 2001 From: pavelkumbrasev Date: Wed, 20 Sep 2023 14:56:12 +0100 Subject: [PATCH 07/10] Remove extra whitespace Signed-off-by: pavelkumbrasev --- examples/migration/recursive_fibonacci/fibonacci.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/examples/migration/recursive_fibonacci/fibonacci.cpp b/examples/migration/recursive_fibonacci/fibonacci.cpp index a2f20c4424..deb587662e 100644 --- a/examples/migration/recursive_fibonacci/fibonacci.cpp +++ b/examples/migration/recursive_fibonacci/fibonacci.cpp @@ -49,7 +49,7 @@ int main(int argc, char* argv[]) { int numbers = argc > 1 ? strtol(argv[1], nullptr, 0) : 50; cutoff = argc > 2 ? strtol(argv[2], nullptr, 0) : 16; unsigned long ntrial = argc > 3 ? (unsigned long)strtoul(argv[3], nullptr, 0) : 20; - tesing_enabled = argc > 4 ? (bool)strtol(argv[4], nullptr, 0) : false; + tesing_enabled = argc > 4 ? (bool)strtol(argv[4], nullptr, 0) : false; auto res = measure(fibonacci_two_tasks, numbers, ntrial); std::cout << "Fibonacci two tasks impl N = " << res.first << " Avg time = " << res.second From 4107ceda64558ea215136a6e71c4aa0fd9ebd9cc Mon Sep 17 00:00:00 2001 From: pavelkumbrasev Date: Thu, 21 Sep 2023 13:36:07 +0100 Subject: [PATCH 08/10] Bypass task to operator()() to remove extra synchronization complexities Signed-off-by: pavelkumbrasev --- .../fibonacci_single_task.h | 22 +++---- .../recursive_fibonacci/fibonacci_two_tasks.h | 16 ++--- .../task_emulation_layer.h | 58 +++++++++---------- 3 files changed, 44 insertions(+), 52 deletions(-) diff --git a/examples/migration/recursive_fibonacci/fibonacci_single_task.h b/examples/migration/recursive_fibonacci/fibonacci_single_task.h index 12be6644d7..61b96056af 100644 --- a/examples/migration/recursive_fibonacci/fibonacci_single_task.h +++ b/examples/migration/recursive_fibonacci/fibonacci_single_task.h @@ -39,10 +39,11 @@ struct single_fib_task : task_emulation::base_task { single_fib_task(int n, int* x) : n(n), x(x), s(state::compute) {} - void execute() override { + task_emulation::base_task* execute() override { + task_emulation::base_task* bypass = nullptr; switch (s) { case state::compute : { - compute_impl(); + bypass = compute_impl(); break; } case state::sum : { @@ -51,35 +52,30 @@ struct single_fib_task : task_emulation::base_task { if (tesing_enabled) { if (n == cutoff && num_recycles > 0) { --num_recycles; - compute_impl(); + bypass = compute_impl(); } } break; } } + return bypass; } - void compute_impl() { + task_emulation::base_task* compute_impl() { + task_emulation::base_task* bypass = nullptr; if (n < cutoff) { *x = serial_fib_1(n); } else { - auto bypass = this->allocate_child_and_increment(n - 2, &x_r); + bypass = this->allocate_child_and_increment(n - 2, &x_r); task_emulation::run_task(this->allocate_child_and_increment(n - 1, &x_l)); // Recycling this->s = state::sum; this->recycle_as_continuation(); - - // Bypass is not supported by task_emulation and next_task executed directly. - // However, the old-TBB bypass behavior can be achieved with - // `return task_group::defer()` (check Migration Guide). - // Consider submit another task if recursion call is not acceptable - // i.e. instead of Direct Body call - // submit task_emulation::run_task(this->allocate_child_and_increment(n - 2, &x_r)); - bypass->operator()(); } + return bypass; } diff --git a/examples/migration/recursive_fibonacci/fibonacci_two_tasks.h b/examples/migration/recursive_fibonacci/fibonacci_two_tasks.h index 9123662522..5d7fd02292 100644 --- a/examples/migration/recursive_fibonacci/fibonacci_two_tasks.h +++ b/examples/migration/recursive_fibonacci/fibonacci_two_tasks.h @@ -33,8 +33,9 @@ long serial_fib(int n) { struct fib_continuation : task_emulation::base_task { fib_continuation(int& s) : sum(s) {} - void execute() override { + task_emulation::base_task* execute() override { sum = x + y; + return nullptr; } int x{ 0 }, y{ 0 }; @@ -44,7 +45,8 @@ struct fib_continuation : task_emulation::base_task { struct fib_computation : task_emulation::base_task { fib_computation(int n, int* x) : n(n), x(x) {} - void execute() override { + task_emulation::base_task* execute() override { + task_emulation::base_task* bypass = nullptr; if (n < cutoff) { *x = serial_fib(n); } @@ -57,15 +59,9 @@ struct fib_computation : task_emulation::base_task { this->recycle_as_child_of(c); n = n - 2; x = &c.y; - - // Bypass is not supported by task_emulation and next_task executed directly. - // However, the old-TBB bypass behavior can be achieved with - // `return task_group::defer()` (check Migration Guide). - // Consider submit another task if recursion call is not acceptable - // i.e. instead of Recycling + Direct Body call - // submit task_emulation::run_task(c.create_child(n - 2, &c.y)); - this->operator()(); + bypass = this; } + return bypass; } int n; diff --git a/examples/migration/recursive_fibonacci/task_emulation_layer.h b/examples/migration/recursive_fibonacci/task_emulation_layer.h index b1e96f0b78..b387a6fde1 100644 --- a/examples/migration/recursive_fibonacci/task_emulation_layer.h +++ b/examples/migration/recursive_fibonacci/task_emulation_layer.h @@ -47,41 +47,51 @@ class base_task { public: base_task() = default; - base_task(const base_task& t) : m_type(t.m_type.load()), m_parent(t.m_parent), m_ref_counter(t.m_ref_counter.load()) + base_task(const base_task& t) : m_type(t.m_type), m_parent(t.m_parent), m_ref_counter(t.m_ref_counter.load()) {} virtual ~base_task() = default; void operator() () const { base_task* parent_snapshot = m_parent; - std::uint64_t type_snapshot = m_type; + task_type type_snapshot = m_type; - const_cast(this)->execute(); + base_task* bypass = const_cast(this)->execute(); bool is_task_recycled_as_child = parent_snapshot != m_parent; bool is_task_recycled_as_continuation = type_snapshot != m_type; if (m_parent && !is_task_recycled_as_child && !is_task_recycled_as_continuation) { - auto child_ref = m_parent->remove_child_reference() & (m_self_ref - 1); - if (child_ref == 0) { + if (m_parent->remove_child_reference() == 0) { m_parent->operator()(); } } - if (type_snapshot != task_type::stack_based && const_cast(this)->remove_self_ref() == 0) { + if (type_snapshot != task_type::stack_based && !is_task_recycled_as_child && !is_task_recycled_as_continuation) { delete this; } + + if (bypass != nullptr) { + m_type = type_snapshot; + + // Bypass is not supported by task_emulation and next_task executed directly. + // However, the old-TBB bypass behavior can be achieved with + // `return task_group::defer()` (check Migration Guide). + // Consider submit another task if recursion call is not acceptable + // i.e. instead of Direct Body call + // submit task_emulation::run_task(); + bypass->operator()(); + } } - virtual void execute() = 0; + virtual base_task* execute() = 0; template C* allocate_continuation(std::uint64_t ref, Args&&... args) { C* continuation = new C{std::forward(args)...}; - continuation->m_type = task_type::continuation; + continuation->m_type = task_type::allocated; continuation->reset_parent(reset_parent()); continuation->m_ref_counter = ref; - continuation->add_self_ref(); return continuation; } @@ -109,12 +119,12 @@ class base_task { template void recycle_as_child_of(C& c) { + m_type = task_type::recycled; reset_parent(&c); } void recycle_as_continuation() { - add_self_ref(); - m_type += task_type::continuation; + m_type = task_type::recycled; } void add_child_reference() { @@ -126,21 +136,13 @@ class base_task { } protected: - void add_self_ref() { - m_ref_counter.fetch_add(m_self_ref); - } - - std::uint64_t remove_self_ref() { - return m_ref_counter.fetch_sub(m_self_ref) - m_self_ref; - } - - struct task_type { - static constexpr std::uint64_t stack_based = 1; - static constexpr std::uint64_t allocated = 1 << 1; - static constexpr std::uint64_t continuation = 1 << 2; + enum task_type { + stack_based, + allocated, + recycled }; - std::atomic m_type; + mutable task_type m_type; private: template @@ -161,7 +163,6 @@ class base_task { F* allocate_child_impl(Args&&... args) { F* obj = new F{std::forward(args)...}; obj->m_type = task_type::allocated; - obj->add_self_ref(); obj->reset_parent(this); return obj; } @@ -173,7 +174,6 @@ class base_task { } base_task* m_parent{nullptr}; - static constexpr std::uint64_t m_self_ref = std::uint64_t(1) << 48; std::atomic m_ref_counter{0}; }; @@ -181,12 +181,13 @@ class root_task : public base_task { public: root_task(tbb::task_group& tg) : m_tg(tg), m_callback(m_tg.defer([] { /* Create empty callback to preserve reference for wait. */})) { add_child_reference(); - m_type = base_task::task_type::continuation; + m_type = base_task::task_type::allocated; } private: - void execute() override { + base_task* execute() override { m_tg.run(std::move(m_callback)); + return nullptr; } tbb::task_group& m_tg; @@ -205,7 +206,6 @@ template F* allocate_root_task(tbb::task_group& tg, Args&&... args) { F* obj = new F{std::forward(args)...}; obj->m_type = base_task::task_type::allocated; - obj->add_self_ref(); obj->reset_parent(new root_task{tg}); return obj; } From 7df5840417fc837580a4f07736d3d5f50019341c Mon Sep 17 00:00:00 2001 From: pavelkumbrasev Date: Fri, 22 Sep 2023 14:57:44 +0100 Subject: [PATCH 09/10] Apply review comments Signed-off-by: pavelkumbrasev --- examples/migration/recursive_fibonacci/README.md | 5 +++-- .../recursive_fibonacci/task_emulation_layer.h | 12 ++++++------ 2 files changed, 9 insertions(+), 8 deletions(-) diff --git a/examples/migration/recursive_fibonacci/README.md b/examples/migration/recursive_fibonacci/README.md index bc66c5d814..5bb6f80fbb 100644 --- a/examples/migration/recursive_fibonacci/README.md +++ b/examples/migration/recursive_fibonacci/README.md @@ -9,14 +9,15 @@ cmake --build . ## Running the sample ### Predefined make targets -* `make run_recursive_fibonacci` - executes the example with predefined parameters. +* `make run_recursive_fibonacci` - executes the example with predefined parameters (extended testing enabled). * `make perf_run_recursive_fibonacci` - executes the example with suggested parameters to measure the oneTBB performance. ### Application parameters Usage: ``` -recursive_fibonacci N C I +recursive_fibonacci N C I T ``` * `N` - specifies the fibonacci number which would be calculated. * `C` - cutoff that will be used to stop recursive split. * `I` - number of iteration to measure benchmark time. +* `T` - enables extended testing diff --git a/examples/migration/recursive_fibonacci/task_emulation_layer.h b/examples/migration/recursive_fibonacci/task_emulation_layer.h index b387a6fde1..efac52b8e5 100644 --- a/examples/migration/recursive_fibonacci/task_emulation_layer.h +++ b/examples/migration/recursive_fibonacci/task_emulation_layer.h @@ -47,7 +47,7 @@ class base_task { public: base_task() = default; - base_task(const base_task& t) : m_type(t.m_type), m_parent(t.m_parent), m_ref_counter(t.m_ref_counter.load()) + base_task(const base_task& t) : m_type(t.m_type), m_parent(t.m_parent), m_child_counter(t.m_child_counter.load()) {} virtual ~base_task() = default; @@ -91,7 +91,7 @@ class base_task { C* continuation = new C{std::forward(args)...}; continuation->m_type = task_type::allocated; continuation->reset_parent(reset_parent()); - continuation->m_ref_counter = ref; + continuation->m_child_counter = ref; return continuation; } @@ -128,15 +128,15 @@ class base_task { } void add_child_reference() { - ++m_ref_counter; + ++m_child_counter; } std::uint64_t remove_child_reference() { - return --m_ref_counter; + return --m_child_counter; } protected: - enum task_type { + enum class task_type { stack_based, allocated, recycled @@ -174,7 +174,7 @@ class base_task { } base_task* m_parent{nullptr}; - std::atomic m_ref_counter{0}; + std::atomic m_child_counter{0}; }; class root_task : public base_task { From f40e0fac53fbd3b27c4346811ced9c49dfa29c51 Mon Sep 17 00:00:00 2001 From: pavelkumbrasev Date: Mon, 25 Sep 2023 10:46:38 +0100 Subject: [PATCH 10/10] Apply review comments Signed-off-by: pavelkumbrasev --- examples/migration/recursive_fibonacci/README.md | 2 +- examples/migration/recursive_fibonacci/fibonacci.cpp | 4 ++-- .../migration/recursive_fibonacci/fibonacci_single_task.h | 4 ++-- .../migration/recursive_fibonacci/task_emulation_layer.h | 8 ++------ 4 files changed, 7 insertions(+), 11 deletions(-) diff --git a/examples/migration/recursive_fibonacci/README.md b/examples/migration/recursive_fibonacci/README.md index 5bb6f80fbb..1f0341c1ea 100644 --- a/examples/migration/recursive_fibonacci/README.md +++ b/examples/migration/recursive_fibonacci/README.md @@ -20,4 +20,4 @@ recursive_fibonacci N C I T * `N` - specifies the fibonacci number which would be calculated. * `C` - cutoff that will be used to stop recursive split. * `I` - number of iteration to measure benchmark time. -* `T` - enables extended testing +* `T` - enables extended testing (recycle task in a loop). diff --git a/examples/migration/recursive_fibonacci/fibonacci.cpp b/examples/migration/recursive_fibonacci/fibonacci.cpp index deb587662e..e4a7c12eb7 100644 --- a/examples/migration/recursive_fibonacci/fibonacci.cpp +++ b/examples/migration/recursive_fibonacci/fibonacci.cpp @@ -22,7 +22,7 @@ #include int cutoff; -bool tesing_enabled; +bool testing_enabled; template std::pair measure(F&& f, @@ -49,7 +49,7 @@ int main(int argc, char* argv[]) { int numbers = argc > 1 ? strtol(argv[1], nullptr, 0) : 50; cutoff = argc > 2 ? strtol(argv[2], nullptr, 0) : 16; unsigned long ntrial = argc > 3 ? (unsigned long)strtoul(argv[3], nullptr, 0) : 20; - tesing_enabled = argc > 4 ? (bool)strtol(argv[4], nullptr, 0) : false; + testing_enabled = argc > 4 ? (bool)strtol(argv[4], nullptr, 0) : false; auto res = measure(fibonacci_two_tasks, numbers, ntrial); std::cout << "Fibonacci two tasks impl N = " << res.first << " Avg time = " << res.second diff --git a/examples/migration/recursive_fibonacci/fibonacci_single_task.h b/examples/migration/recursive_fibonacci/fibonacci_single_task.h index 61b96056af..dae8895b8c 100644 --- a/examples/migration/recursive_fibonacci/fibonacci_single_task.h +++ b/examples/migration/recursive_fibonacci/fibonacci_single_task.h @@ -24,7 +24,7 @@ #include extern int cutoff; -extern bool tesing_enabled; +extern bool testing_enabled; long serial_fib_1(int n) { return n < 2 ? n : serial_fib_1(n - 1) + serial_fib_1(n - 2); @@ -49,7 +49,7 @@ struct single_fib_task : task_emulation::base_task { case state::sum : { *x = x_l + x_r; - if (tesing_enabled) { + if (testing_enabled) { if (n == cutoff && num_recycles > 0) { --num_recycles; bypass = compute_impl(); diff --git a/examples/migration/recursive_fibonacci/task_emulation_layer.h b/examples/migration/recursive_fibonacci/task_emulation_layer.h index efac52b8e5..7252d447a0 100644 --- a/examples/migration/recursive_fibonacci/task_emulation_layer.h +++ b/examples/migration/recursive_fibonacci/task_emulation_layer.h @@ -53,21 +53,17 @@ class base_task { virtual ~base_task() = default; void operator() () const { - base_task* parent_snapshot = m_parent; task_type type_snapshot = m_type; base_task* bypass = const_cast(this)->execute(); - bool is_task_recycled_as_child = parent_snapshot != m_parent; - bool is_task_recycled_as_continuation = type_snapshot != m_type; - - if (m_parent && !is_task_recycled_as_child && !is_task_recycled_as_continuation) { + if (m_parent && m_type != task_type::recycled) { if (m_parent->remove_child_reference() == 0) { m_parent->operator()(); } } - if (type_snapshot != task_type::stack_based && !is_task_recycled_as_child && !is_task_recycled_as_continuation) { + if (m_type == task_type::allocated) { delete this; }