From e70f9a240681186fac8ac62954df163fc2989b04 Mon Sep 17 00:00:00 2001 From: Hartmut Kaiser Date: Tue, 9 Jan 2024 15:50:33 -0600 Subject: [PATCH 1/9] Tightening up collective operation semantics - flyby: small_vector tweaks --- .../datastructures/detail/small_vector.hpp | 38 ++-- .../tests/unit/small_vector.cpp | 75 ++++---- libs/core/futures/src/future_data.cpp | 53 +++--- .../include/hpx/lcos_local/and_gate.hpp | 25 ++- .../include/hpx/collectives/all_gather.hpp | 12 +- .../include/hpx/collectives/all_reduce.hpp | 12 +- .../include/hpx/collectives/all_to_all.hpp | 12 +- .../include/hpx/collectives/broadcast.hpp | 14 +- .../hpx/collectives/detail/communicator.hpp | 168 +++++++++++++++--- .../hpx/collectives/exclusive_scan.hpp | 14 +- .../include/hpx/collectives/gather.hpp | 16 +- .../hpx/collectives/inclusive_scan.hpp | 14 +- .../include/hpx/collectives/reduce.hpp | 17 +- .../include/hpx/collectives/scatter.hpp | 16 +- .../collectives/src/create_communicator.cpp | 7 +- .../tests/unit/communication_set.cpp | 2 +- 16 files changed, 331 insertions(+), 164 deletions(-) diff --git a/libs/core/datastructures/include/hpx/datastructures/detail/small_vector.hpp b/libs/core/datastructures/include/hpx/datastructures/detail/small_vector.hpp index ea7e69202f58..d6379378d138 100644 --- a/libs/core/datastructures/include/hpx/datastructures/detail/small_vector.hpp +++ b/libs/core/datastructures/include/hpx/datastructures/detail/small_vector.hpp @@ -175,7 +175,7 @@ namespace hpx::detail { } // only void* is allowed to be converted to uintptr_t - void* ptr = ::operator new(offset_to_data + sizeof(T) * capacity); + void* ptr = ::operator new(mem); if (nullptr == ptr) { throw std::bad_alloc(); @@ -319,9 +319,13 @@ namespace hpx::detail { { // indirect -> direct auto* storage = indirect(); - uninitialized_move_and_destroy( - storage->data(), direct_data(), storage->size()); - set_direct_and_size(storage->size()); + auto const data_size = storage->size(); + if (data_size != 0) + { + uninitialized_move_and_destroy( + storage->data(), direct_data(), data_size); + } + set_direct_and_size(data_size); detail::storage::dealloc(storage); } } @@ -332,16 +336,26 @@ namespace hpx::detail { if (is_direct()) { // direct -> indirect - uninitialized_move_and_destroy(data(), - storage->data(), size()); - storage->size(size()); + auto const data_size = size(); + if (data_size != 0) + { + uninitialized_move_and_destroy( + data(), storage->data(), + data_size); + } + storage->size(data_size); } else { // indirect -> indirect - uninitialized_move_and_destroy(data(), - storage->data(), size()); - storage->size(size()); + auto const data_size = size(); + if (data_size != 0) + { + uninitialized_move_and_destroy( + data(), storage->data(), + data_size); + } + storage->size(data_size); detail::storage::dealloc(indirect()); } set_indirect(storage); @@ -657,7 +671,7 @@ namespace hpx::detail { set_direct_and_size(0); } - // performs a const_cast so we don't need this implementation twice + // performs a const_cast, so we don't need this implementation twice template [[nodiscard]] auto at(std::size_t idx) const -> T& { @@ -785,7 +799,7 @@ namespace hpx::detail { if (&other == this) { // It doesn't seem to be required to do self-check, but let's do - // it anyways to be safe + // it anyway to be safe return *this; } diff --git a/libs/core/datastructures/tests/unit/small_vector.cpp b/libs/core/datastructures/tests/unit/small_vector.cpp index 765005d12d27..d743d0b8a158 100644 --- a/libs/core/datastructures/tests/unit/small_vector.cpp +++ b/libs/core/datastructures/tests/unit/small_vector.cpp @@ -1,4 +1,4 @@ -// Copyright (c) 2021-2022 Hartmut Kaiser +// Copyright (c) 2021-2024 Hartmut Kaiser // // SPDX-License-Identifier: BSL-1.0 // Distributed under the Boost Software License, Version 1.0. (See accompanying @@ -36,18 +36,20 @@ namespace test { simple_allocator() noexcept = default; template - simple_allocator(simple_allocator const&) noexcept + explicit simple_allocator(simple_allocator const&) noexcept { } - T* allocate(std::size_t n) + static T* allocate(std::size_t n) { return reinterpret_cast(::new char[sizeof(T) * n]); } - void deallocate(T* p, std::size_t) noexcept + static void deallocate(T* p, std::size_t) noexcept { - delete[](reinterpret_cast(p)); + // clang-format off + delete[] (reinterpret_cast(p)); + // clang-format on } friend bool operator==( @@ -204,7 +206,7 @@ namespace test { void small_vector_test() { - // basic test with less elements than static size + // basic test with fewer elements than static size { using sm5_t = hpx::detail::small_vector; static_assert( @@ -214,7 +216,7 @@ namespace test { sm5.push_back(1); HPX_TEST_EQ(sm5[0], 1); - sm5_t sm5_copy(sm5); + sm5_t const sm5_copy(sm5); HPX_TEST(sm5 == sm5_copy); } { @@ -226,7 +228,7 @@ namespace test { sm7.push_back(1); HPX_TEST_EQ(sm7[0], 1); - sm7_t sm7_copy(sm7); + sm7_t const sm7_copy(sm7); HPX_TEST(sm7 == sm7_copy); } { @@ -240,7 +242,7 @@ namespace test { sm5.push_back(2); HPX_TEST_EQ(sm5[1], 2); - HPX_TEST_EQ(sm5.size(), std::size_t(2)); + HPX_TEST_EQ(sm5.size(), static_cast(2)); sm5_copy = sm5; HPX_TEST(sm5 == sm5_copy); @@ -274,7 +276,7 @@ namespace test { sm2.push_back(3); HPX_TEST_EQ(sm2[1], 2); HPX_TEST_EQ(sm2[2], 3); - HPX_TEST_EQ(sm2.size(), std::size_t(3)); + HPX_TEST_EQ(sm2.size(), static_cast(3)); sm2_copy = sm2; HPX_TEST(sm2 == sm2_copy); @@ -307,13 +309,13 @@ namespace test { for (std::size_t i = 0, max = v.capacity() + 1; i != max; ++i) { - v.push_back(int(i)); + v.push_back(static_cast(i)); } vec w; - vec v_copy(v); - vec w_copy(w); + vec const v_copy(v); + vec const w_copy(w); v.swap(w); HPX_TEST(v == w_copy); @@ -324,13 +326,13 @@ namespace test { vec v; for (std::size_t i = 0, max = v.capacity() - 1; i != max; ++i) { - v.push_back(int(i)); + v.push_back(static_cast(i)); } vec w; - vec v_copy(v); - vec w_copy(w); + vec const v_copy(v); + vec const w_copy(w); v.swap(w); HPX_TEST(v == w_copy); @@ -341,17 +343,17 @@ namespace test { vec v; for (std::size_t i = 0, max = v.capacity() - 1; i != max; ++i) { - v.push_back(int(i)); + v.push_back(static_cast(i)); } vec w; for (std::size_t i = 0, max = v.capacity() / 2; i != max; ++i) { - w.push_back(int(i)); + w.push_back(static_cast(i)); } - vec v_copy(v); - vec w_copy(w); + vec const v_copy(v); + vec const w_copy(w); v.swap(w); HPX_TEST(v == w_copy); @@ -362,17 +364,17 @@ namespace test { vec v; for (std::size_t i = 0, max = v.capacity() + 1; i != max; ++i) { - v.push_back(int(i)); + v.push_back(static_cast(i)); } vec w; for (std::size_t i = 0, max = v.capacity() * 2; i != max; ++i) { - w.push_back(int(i)); + w.push_back(static_cast(i)); } - vec v_copy(v); - vec w_copy(w); + vec const v_copy(v); + vec const w_copy(w); v.swap(w); HPX_TEST(v == w_copy); @@ -448,7 +450,6 @@ namespace test { void vector_test() { using value_type = typename Vector::value_type; - constexpr int max = 100; test_range_insertion(); @@ -480,6 +481,7 @@ namespace test { test::check_equal_containers(vector2, v); } { + constexpr int max = 100; Vector vector; std::vector v; @@ -534,8 +536,9 @@ namespace test { auto insert_it = vector.insert(vector.end(), &aux_vect[0], aux_vect + 50); - HPX_TEST_EQ(std::size_t(std::distance(insert_it, vector.end())), - std::size_t(50)); + HPX_TEST_EQ(static_cast( + std::distance(insert_it, vector.end())), + static_cast(50)); v.insert(v.end(), aux_vect2, aux_vect2 + 50); test::check_equal_containers(vector, v); @@ -581,9 +584,9 @@ namespace test { { //push_back with not enough capacity value_type push_back_this(1); vector.push_back(std::move(push_back_this)); - v.push_back(int(1)); + v.push_back(static_cast(1)); vector.push_back(value_type(1)); - v.push_back(int(1)); + v.push_back(static_cast(1)); test::check_equal_containers(vector, v); } @@ -601,9 +604,9 @@ namespace test { value_type push_back_this(1); vector.push_back(std::move(push_back_this)); - v.push_back(int(1)); + v.push_back(static_cast(1)); vector.push_back(value_type(1)); - v.push_back(int(1)); + v.push_back(static_cast(1)); test::check_equal_containers(vector, v); } @@ -617,7 +620,7 @@ namespace test { vector.insert(vector.begin(), std::move(insert_this)); v.insert(v.begin(), i); vector.insert(vector.begin(), value_type(i)); - v.insert(v.begin(), int(i)); + v.insert(v.begin(), static_cast(i)); } test::check_equal_containers(vector, v); @@ -631,7 +634,7 @@ namespace test { // Test insertion from list { - std::list l(50, int(1)); + std::list l(50, static_cast(1)); auto it_insert = vector.insert(vector.begin(), l.begin(), l.end()); HPX_TEST(vector.begin() == it_insert); @@ -643,7 +646,7 @@ namespace test { v.assign(l.begin(), l.end()); test::check_equal_containers(vector, v); - std::forward_list fl(50, int(1)); + std::forward_list fl(50, static_cast(1)); vector.clear(); v.clear(); vector.assign(fl.begin(), fl.end()); @@ -663,7 +666,7 @@ namespace test { class emplace_int { public: - emplace_int( + explicit emplace_int( int a = 0, int b = 0, int c = 0, int d = 0, int e = 0) noexcept : a_(a) , b_(b) @@ -726,7 +729,7 @@ namespace test { } int a_, b_, c_, d_, e_; - int padding[6]; + int padding[6] = {}; }; static emplace_int expected[10]; diff --git a/libs/core/futures/src/future_data.cpp b/libs/core/futures/src/future_data.cpp index a05f4d577be7..28d8eda865d3 100644 --- a/libs/core/futures/src/future_data.cpp +++ b/libs/core/futures/src/future_data.cpp @@ -1,4 +1,4 @@ -// Copyright (c) 2015-2023 Hartmut Kaiser +// Copyright (c) 2015-2024 Hartmut Kaiser // // SPDX-License-Identifier: BSL-1.0 // Distributed under the Boost Software License, Version 1.0. (See accompanying @@ -22,13 +22,14 @@ #include #include -#include #include #include namespace hpx::lcos::detail { - static run_on_completed_error_handler_type run_on_completed_error_handler; + namespace { + run_on_completed_error_handler_type run_on_completed_error_handler; + } void set_run_on_completed_error_handler( run_on_completed_error_handler_type f) @@ -66,16 +67,12 @@ namespace hpx::lcos::detail { /////////////////////////////////////////////////////////////////////////// template - static void run_on_completed_on_new_thread(Callback&& f) + void run_on_completed_on_new_thread(Callback&& f) { lcos::local::futures_factory p(HPX_FORWARD(Callback, f)); - bool const is_hpx_thread = nullptr != hpx::threads::get_self_ptr(); + HPX_ASSERT(nullptr != hpx::threads::get_self_ptr()); hpx::launch policy = launch::fork; - if (!is_hpx_thread) - { - policy = launch::async; - } policy.set_priority(threads::thread_priority::boost); policy.set_stacksize(threads::thread_stacksize::current); @@ -84,17 +81,12 @@ namespace hpx::lcos::detail { threads::thread_id_ref_type const tid = //-V821 p.post("run_on_completed_on_new_thread", policy); - // wait for the task to run - if (is_hpx_thread) - { - // make sure this thread is executed last - this_thread::suspend( - threads::thread_schedule_state::pending, tid.noref()); - return p.get_future().get(); - } + // make sure this thread is executed last + this_thread::suspend( + threads::thread_schedule_state::pending, tid.noref()); - // If we are not on a HPX thread, we need to return immediately, to - // allow the newly spawned thread to execute. + // wait for the task to run + return p.get_future().get(); } /////////////////////////////////////////////////////////////////////////// @@ -124,15 +116,13 @@ namespace hpx::lcos::detail { } auto const state = this->state_.load(std::memory_order_acquire); - if (state != this->empty) + if (state != future_data_base::empty) { return false; } // this thread would block on the future - - auto* thrd = get_thread_id_data(runs_child); - HPX_UNUSED(thrd); // might be unused + [[maybe_unused]] auto* thrd = get_thread_id_data(runs_child); LTM_(debug).format("task_object::get_result_void: attempting to " "directly execute child({}), description({})", @@ -161,8 +151,6 @@ namespace hpx::lcos::detail { return false; } - static util::unused_type unused_; - util::unused_type* future_data_base::get_result_void( void const* storage, error_code& ec) @@ -190,6 +178,7 @@ namespace hpx::lcos::detail { if (s == value) { + static util::unused_type unused_; return &unused_; } @@ -232,12 +221,12 @@ namespace hpx::lcos::detail { hpx::scoped_annotation annotate(on_completed); HPX_MOVE(on_completed)(); }, - [&](std::exception_ptr ep) { + [&](std::exception_ptr const& ep) { // If the completion handler throws an exception, there's // nothing we can do, report the exception and terminate. if (run_on_completed_error_handler) { - run_on_completed_error_handler(HPX_MOVE(ep)); + run_on_completed_error_handler(ep); } else { @@ -272,7 +261,9 @@ namespace hpx::lcos::detail { cnt.count_ > HPX_CONTINUATION_MAX_RECURSION_DEPTH || (hpx::threads::get_self_ptr() == nullptr); #endif - if (!recurse_asynchronously) + + bool const is_hpx_thread = nullptr != hpx::threads::get_self_ptr(); + if (!is_hpx_thread || !recurse_asynchronously) { // directly execute continuation on this thread run_on_completed(HPX_FORWARD(Callback, on_completed)); @@ -289,17 +280,17 @@ namespace hpx::lcos::detail { run_on_completed_on_new_thread(util::deferred_call( p, HPX_FORWARD(Callback, on_completed))); }, - [&](std::exception_ptr ep) { + [&](std::exception_ptr const& ep) { // If an exception while creating the new task or inside the // completion handler is thrown, there is nothing we can do... // ... but terminate and report the error if (run_on_completed_error_handler) { - run_on_completed_error_handler(HPX_MOVE(ep)); + run_on_completed_error_handler(ep); } else { - std::rethrow_exception(HPX_MOVE(ep)); + std::rethrow_exception(ep); } }); } diff --git a/libs/core/lcos_local/include/hpx/lcos_local/and_gate.hpp b/libs/core/lcos_local/include/hpx/lcos_local/and_gate.hpp index deeaeffc5e3b..8ee7ebf0a133 100644 --- a/libs/core/lcos_local/include/hpx/lcos_local/and_gate.hpp +++ b/libs/core/lcos_local/include/hpx/lcos_local/and_gate.hpp @@ -170,7 +170,7 @@ namespace hpx::lcos::local { protected: // Set the data which has to go into the segment \a which. template - bool set(std::size_t which, OuterLock outer_lock, F&& f, + bool set(std::size_t which, OuterLock& outer_lock, F&& f, error_code& ec = throws) { HPX_ASSERT_OWNS_LOCK(outer_lock); @@ -224,15 +224,12 @@ namespace hpx::lcos::local { std::decay_t>) { // invoke callback with the outer lock being held - HPX_FORWARD(F, f)(outer_lock, *this); + HPX_FORWARD(F, f)(outer_lock, *this, ec); } - outer_lock.unlock(); return true; } } - - outer_lock.unlock(); return false; } @@ -242,7 +239,7 @@ namespace hpx::lcos::local { { hpx::no_mutex mtx; std::unique_lock lk(mtx); - return set(which, HPX_MOVE(lk), HPX_FORWARD(F, f), ec); + return set(which, lk, HPX_FORWARD(F, f), ec); } protected: @@ -324,7 +321,8 @@ namespace hpx::lcos::local { public: template - std::size_t next_generation(Lock& l, std::size_t new_generation) + std::size_t next_generation( + Lock& l, std::size_t new_generation, error_code& ec = throws) { HPX_ASSERT_OWNS_LOCK(l); @@ -335,10 +333,11 @@ namespace hpx::lcos::local { if (new_generation < generation_) { l.unlock(); - HPX_THROW_EXCEPTION(hpx::error::invalid_status, + HPX_THROWS_IF(ec, hpx::error::invalid_status, "and_gate::next_generation", "sequencing error, new generational counter value too " "small"); + return generation_; } generation_ = new_generation; } @@ -351,10 +350,11 @@ namespace hpx::lcos::local { } std::size_t next_generation( - std::size_t new_generation = static_cast(-1)) + std::size_t new_generation = static_cast(-1), + error_code& ec = throws) { std::unique_lock l(mtx_); - return next_generation(l, new_generation); + return next_generation(l, new_generation, ec); } template @@ -441,11 +441,10 @@ namespace hpx::lcos::local { } template - bool set(std::size_t which, Lock l, F&& f = nullptr, + bool set(std::size_t which, Lock& l, F&& f = nullptr, error_code& ec = hpx::throws) { - return this->base_type::set( - which, HPX_MOVE(l), HPX_FORWARD(F, f), ec); + return this->base_type::set(which, l, HPX_FORWARD(F, f), ec); } template diff --git a/libs/full/collectives/include/hpx/collectives/all_gather.hpp b/libs/full/collectives/include/hpx/collectives/all_gather.hpp index 519547a15eab..59c3aa2bf7b0 100644 --- a/libs/full/collectives/include/hpx/collectives/all_gather.hpp +++ b/libs/full/collectives/include/hpx/collectives/all_gather.hpp @@ -1,4 +1,4 @@ -// Copyright (c) 2019-2023 Hartmut Kaiser +// Copyright (c) 2019-2024 Hartmut Kaiser // // SPDX-License-Identifier: BSL-1.0 // Distributed under the Boost Software License, Version 1.0. (See accompanying @@ -149,11 +149,15 @@ namespace hpx::traits { std::size_t generation, T&& t) { return communicator.template handle_data>( + communication::communicator_name< + communication::all_gather_tag>(), which, generation, // step function (invoked for each get) - [&](auto& data) { data[which] = HPX_FORWARD(T, t); }, + [&t](auto& data, std::size_t which) { + data[which] = HPX_FORWARD(T, t); + }, // finalizer (invoked after all data has been received) - [](auto& data, auto&) { return data; }); + [](auto& data, auto&, std::size_t) { return data; }); } }; } // namespace hpx::traits @@ -199,7 +203,7 @@ namespace hpx::collectives { { // make sure id is kept alive as long as the returned future traits::detail::get_shared_state(result)->set_on_completed( - [client = HPX_MOVE(c)]() { HPX_UNUSED(client); }); + [client = HPX_MOVE(c)] { HPX_UNUSED(client); }); } return result; diff --git a/libs/full/collectives/include/hpx/collectives/all_reduce.hpp b/libs/full/collectives/include/hpx/collectives/all_reduce.hpp index 7f2c5abaf80f..cb659bd2006b 100644 --- a/libs/full/collectives/include/hpx/collectives/all_reduce.hpp +++ b/libs/full/collectives/include/hpx/collectives/all_reduce.hpp @@ -1,4 +1,4 @@ -// Copyright (c) 2019-2023 Hartmut Kaiser +// Copyright (c) 2019-2024 Hartmut Kaiser // // SPDX-License-Identifier: BSL-1.0 // Distributed under the Boost Software License, Version 1.0. (See accompanying @@ -156,13 +156,17 @@ namespace hpx::traits { std::size_t generation, T&& t, F&& op) { return communicator.template handle_data>( + communication::communicator_name< + communication::all_reduce_tag>(), which, generation, // step function (invoked for each get) - [&](auto& data) { data[which] = HPX_FORWARD(T, t); }, + [&t](auto& data, std::size_t which) { + data[which] = HPX_FORWARD(T, t); + }, // finalizer (invoked non-concurrently after all data has been // received) [op = HPX_FORWARD(F, op)]( - auto& data, bool& data_available) mutable { + auto& data, bool& data_available, std::size_t) mutable { HPX_ASSERT(!data.empty()); if (!data_available && data.size() > 1) { @@ -219,7 +223,7 @@ namespace hpx::collectives { { // make sure id is kept alive as long as the returned future traits::detail::get_shared_state(result)->set_on_completed( - [client = HPX_MOVE(c)]() { HPX_UNUSED(client); }); + [client = HPX_MOVE(c)] { HPX_UNUSED(client); }); } return result; diff --git a/libs/full/collectives/include/hpx/collectives/all_to_all.hpp b/libs/full/collectives/include/hpx/collectives/all_to_all.hpp index 1f3c8597ad2b..3f7467939e1b 100644 --- a/libs/full/collectives/include/hpx/collectives/all_to_all.hpp +++ b/libs/full/collectives/include/hpx/collectives/all_to_all.hpp @@ -1,4 +1,4 @@ -// Copyright (c) 2019-2023 Hartmut Kaiser +// Copyright (c) 2019-2024 Hartmut Kaiser // // SPDX-License-Identifier: BSL-1.0 // Distributed under the Boost Software License, Version 1.0. (See accompanying @@ -151,11 +151,15 @@ namespace hpx::traits { std::size_t generation, std::vector&& t) { return communicator.template handle_data>( + communication::communicator_name< + communication::all_to_all_tag>(), which, generation, // step function (invoked for each get) - [&](auto& data) { data[which] = HPX_MOVE(t); }, + [&t](auto& data, std::size_t which) { + data[which] = HPX_MOVE(t); + }, // finalizer (invoked after all data has been received) - [which](auto& data, auto&) { + [](auto& data, auto&, std::size_t which) { // slice the overall data based on the locality id of the // requesting site std::vector result; @@ -209,7 +213,7 @@ namespace hpx::collectives { { // make sure id is kept alive as long as the returned future traits::detail::get_shared_state(result)->set_on_completed( - [client = HPX_MOVE(c)]() { HPX_UNUSED(client); }); + [client = HPX_MOVE(c)] { HPX_UNUSED(client); }); } return result; diff --git a/libs/full/collectives/include/hpx/collectives/broadcast.hpp b/libs/full/collectives/include/hpx/collectives/broadcast.hpp index 6a8afdfa3197..b424b6ddfaa1 100644 --- a/libs/full/collectives/include/hpx/collectives/broadcast.hpp +++ b/libs/full/collectives/include/hpx/collectives/broadcast.hpp @@ -1,4 +1,4 @@ -// Copyright (c) 2020-2023 Hartmut Kaiser +// Copyright (c) 2020-2024 Hartmut Kaiser // // SPDX-License-Identifier: BSL-1.0 // Distributed under the Boost Software License, Version 1.0. (See accompanying @@ -233,11 +233,13 @@ namespace hpx::traits { using data_type = typename Result::result_type; return communicator.template handle_data( + communication::communicator_name< + communication::broadcast_tag>(), which, generation, // no step function nullptr, // finalizer (invoked after all sites have checked in) - [](auto& data, auto&) { + [](auto& data, auto&, std::size_t) { return Communicator::template handle_bool( data[0]); }, @@ -249,11 +251,13 @@ namespace hpx::traits { std::size_t generation, T&& t) { return communicator.template handle_data>( + communication::communicator_name< + communication::broadcast_tag>(), which, generation, // step function (invoked once for set) - [&](auto& data) { data[0] = HPX_FORWARD(T, t); }, + [&t](auto& data, std::size_t) { data[0] = HPX_FORWARD(T, t); }, // finalizer (invoked after all sites have checked in) - [](auto& data, auto&) { + [](auto& data, auto&, std::size_t) { return Communicator::template handle_bool>( data[0]); }, @@ -298,7 +302,7 @@ namespace hpx::collectives { { // make sure id is kept alive as long as the returned future traits::detail::get_shared_state(result)->set_on_completed( - [client = HPX_MOVE(c)]() { HPX_UNUSED(client); }); + [client = HPX_MOVE(c)] { HPX_UNUSED(client); }); } return result; diff --git a/libs/full/collectives/include/hpx/collectives/detail/communicator.hpp b/libs/full/collectives/include/hpx/collectives/detail/communicator.hpp index d4d06612f5ba..28d85536bc42 100644 --- a/libs/full/collectives/include/hpx/collectives/detail/communicator.hpp +++ b/libs/full/collectives/include/hpx/collectives/detail/communicator.hpp @@ -1,4 +1,4 @@ -// Copyright (c) 2020-2023 Hartmut Kaiser +// Copyright (c) 2020-2024 Hartmut Kaiser // // SPDX-License-Identifier: BSL-1.0 // Distributed under the Boost Software License, Version 1.0. (See accompanying @@ -21,6 +21,7 @@ #include #include +#include #include #include #include @@ -89,6 +90,8 @@ namespace hpx::collectives::detail { std::size_t, std::size_t, char const*) noexcept { } + + ~logging_helper() = default; #endif logging_helper(logging_helper const&) = delete; @@ -165,6 +168,12 @@ namespace hpx::collectives::detail { }; private: + std::size_t get_num_sites(std::size_t num_values) const noexcept + { + return num_values == static_cast(-1) ? num_sites_ : + num_values; + } + // re-initialize data template void reinitialize_data(std::size_t num_values) @@ -174,10 +183,8 @@ namespace hpx::collectives::detail { needs_initialization_ = false; data_available_ = false; - auto const new_size = - num_values == static_cast(-1) ? num_sites_ : - num_values; - auto* data = hpx::any_cast>(&data_); + auto const new_size = get_num_sites(num_values); + auto const* data = hpx::any_cast>(&data_); if (data == nullptr || data->size() < new_size) { data_ = std::vector(new_size); @@ -201,6 +208,8 @@ namespace hpx::collectives::detail { { needs_initialization_ = true; data_available_ = false; + on_ready_count_ = 0; + current_operation_ = nullptr; } } @@ -212,8 +221,7 @@ namespace hpx::collectives::detail { auto sf = gate_.get_shared_future(l); traits::detail::get_shared_state(sf)->reserve_callbacks( - capacity == static_cast(-1) ? num_sites_ : - capacity); + get_num_sites(capacity)); auto fut = sf.then(hpx::launch::sync, HPX_FORWARD(F, f)); @@ -225,30 +233,94 @@ namespace hpx::collectives::detail { return fut; } + template + struct on_exit + { + explicit constexpr on_exit(F&& f_) noexcept + : f(HPX_MOVE(f_)) + { + } + + on_exit(on_exit const&) = delete; + on_exit(on_exit&&) = delete; + on_exit& operator=(on_exit const&) = delete; + on_exit& operator=(on_exit&&) = delete; + + ~on_exit() + { + f(); + } + + F f; + }; + // Step will be invoked under lock for each site that checks in (either // set or get). // // Finalizer will be invoked under lock after all sites have checked in. template - auto handle_data(std::size_t which, std::size_t generation, - [[maybe_unused]] Step&& step, Finalizer&& finalizer, + auto handle_data(char const* operation, std::size_t which, + std::size_t generation, [[maybe_unused]] Step&& step, + Finalizer&& finalizer, std::size_t num_values = static_cast(-1)) { - auto on_ready = [this, num_values, + auto on_ready = [this, operation, which, num_values, finalizer = HPX_FORWARD(Finalizer, finalizer)]( shared_future&& f) mutable { + // This callback will be invoked once for each participating + // site after all sites have checked in. + f.get(); // propagate any exceptions + // It does not matter whether the lock will be acquired here. It + // either is still being held by the surrounding logic or is + // re-acquired here (if `on_ready` happens to run on a new + // thread asynchronously). + std::unique_lock l(mtx_, std::try_to_lock); + + // Verify that there is no overlap between different types of + // operations on the same communicator. + if (current_operation_ == nullptr || + std::strcmp(current_operation_, operation) != 0) + { + l.unlock(); + HPX_THROW_EXCEPTION(hpx::error::invalid_status, + "communicator::handle_data::on_ready", + "sequencing error, operation type mismatch: invoked " + "for {}, ongoing operation {}", + operation, + current_operation_ ? current_operation_ : "unknown"); + } + + // Verify that the number of invocations of this callback is in + // the expected range. + if (on_ready_count_ >= num_sites_) + { + l.unlock(); + HPX_THROW_EXCEPTION(hpx::error::invalid_status, + "communicator::handle_data::on_ready", + "sequencing error, an excessive number of on_ready " + "callbacks have been invoked before the end of the " + "collective {} operation. Expected count {}, received " + "count {}.", + operation, on_ready_count_, num_sites_); + } + + // On exit, keep track of number of invocations of this + // callback. + on_exit _([this] { ++on_ready_count_; }); + if constexpr (!std::is_same_v>) { // call provided finalizer return HPX_FORWARD(Finalizer, finalizer)( - access_data(num_values), data_available_); + access_data(num_values), data_available_, which); } else { HPX_UNUSED(this); + HPX_UNUSED(which); HPX_UNUSED(num_values); HPX_UNUSED(finalizer); } @@ -257,23 +329,73 @@ namespace hpx::collectives::detail { std::unique_lock l(mtx_); [[maybe_unused]] util::ignore_while_checking il(&l); + // Verify that there is no overlap between different types of + // operations on the same communicator. + if (current_operation_ == nullptr) + { + if (on_ready_count_ != 0) + { + l.unlock(); + HPX_THROW_EXCEPTION(hpx::error::invalid_status, + "communicator::handle_data", + "sequencing error, on_ready callback was already " + "invoked before the start of the collective {} " + "operation", + operation); + } + current_operation_ = operation; + } + else if (std::strcmp(current_operation_, operation) != 0) + { + l.unlock(); + HPX_THROW_EXCEPTION(hpx::error::invalid_status, + "communicator::handle_data", + "sequencing error, operation type mismatch: invoked for " + "{}, ongoing operation {}", + operation, current_operation_); + } + auto f = get_future_and_synchronize( generation, num_values, HPX_MOVE(on_ready), l); if constexpr (!std::is_same_v>) { // call provided step function for each invocation site - HPX_FORWARD(Step, step)(access_data(num_values)); + HPX_FORWARD(Step, step)(access_data(num_values), which); } // Make sure next generation is enabled only after previous // generation has finished executing. - // - // set() consumes the lock - gate_.set( - which, HPX_MOVE(l), [this, generation](auto& l, auto& gate) { - gate.next_generation(l, generation); - this->invalidate_data(l); + gate_.set(which, l, + [this, operation, generation]( + auto& l, auto& gate, error_code& ec) { + // This callback is invoked synchronously once for each + // collective operation after all data has been received and + // all (shared) futures were triggered. + + HPX_ASSERT_OWNS_LOCK(l); + + // Verify that all `on_ready` callbacks have finished + // executing at this point. + if (on_ready_count_ != num_sites_) + { + l.unlock(); + HPX_THROWS_IF(ec, hpx::error::invalid_status, + "communicator::handle_data", + "sequencing error, not all on_ready callbacks have " + "been invoked at the end of the collective {} " + "operation. Expected count {}, received count {}.", + operation, on_ready_count_, num_sites_); + return; + } + + // Reset communicator state before proceeding to the next + // generation. + invalidate_data(l); + + // Release threads possibly waiting for the next generation + // to be handled. + gate.next_generation(l, generation, ec); }); return f; @@ -281,7 +403,7 @@ namespace hpx::collectives::detail { // protect against vector idiosyncrasies template - static constexpr decltype(auto) handle_bool(Data&& data) + static constexpr decltype(auto) handle_bool(Data&& data) noexcept { if constexpr (std::is_same_v) { @@ -298,10 +420,12 @@ namespace hpx::collectives::detail { mutex_type mtx_; hpx::unique_any_nonser data_; - lcos::local::and_gate gate_; + hpx::lcos::local::and_gate gate_; std::size_t const num_sites_; - bool needs_initialization_; - bool data_available_; + std::size_t on_ready_count_ = 0; + char const* current_operation_ = nullptr; + bool needs_initialization_ = true; + bool data_available_ = false; }; } // namespace hpx::collectives::detail diff --git a/libs/full/collectives/include/hpx/collectives/exclusive_scan.hpp b/libs/full/collectives/include/hpx/collectives/exclusive_scan.hpp index c6bfbbf157fb..121bd9e7983c 100644 --- a/libs/full/collectives/include/hpx/collectives/exclusive_scan.hpp +++ b/libs/full/collectives/include/hpx/collectives/exclusive_scan.hpp @@ -1,4 +1,4 @@ -// Copyright (c) 2019-2023 Hartmut Kaiser +// Copyright (c) 2019-2024 Hartmut Kaiser // // SPDX-License-Identifier: BSL-1.0 // Distributed under the Boost Software License, Version 1.0. (See accompanying @@ -169,13 +169,17 @@ namespace hpx::traits { std::size_t generation, T&& t, F&& op) { return communicator.template handle_data>( + communication::communicator_name< + communication::exclusive_scan_tag>(), which, generation, // step function (invoked for each get) - [&](auto& data) { data[which] = HPX_FORWARD(T, t); }, + [&t](auto& data, std::size_t which) { + data[which] = HPX_FORWARD(T, t); + }, // finalizer (invoked non-concurrently after all data has been // received) - [which, op = HPX_FORWARD(F, op)]( - auto& data, bool& data_available) mutable { + [op = HPX_FORWARD(F, op)](auto& data, bool& data_available, + std::size_t which) mutable { if (!data_available) { std::vector> dest; @@ -236,7 +240,7 @@ namespace hpx::collectives { { // make sure id is kept alive as long as the returned future traits::detail::get_shared_state(result)->set_on_completed( - [client = HPX_MOVE(c)]() { HPX_UNUSED(client); }); + [client = HPX_MOVE(c)] { HPX_UNUSED(client); }); } return result; diff --git a/libs/full/collectives/include/hpx/collectives/gather.hpp b/libs/full/collectives/include/hpx/collectives/gather.hpp index 0409833cc7fe..ac85cea4e128 100644 --- a/libs/full/collectives/include/hpx/collectives/gather.hpp +++ b/libs/full/collectives/include/hpx/collectives/gather.hpp @@ -1,4 +1,4 @@ -// Copyright (c) 2014-2023 Hartmut Kaiser +// Copyright (c) 2014-2024 Hartmut Kaiser // // SPDX-License-Identifier: BSL-1.0 // Distributed under the Boost Software License, Version 1.0. (See accompanying @@ -250,11 +250,14 @@ namespace hpx::traits { std::size_t generation, T&& t) { return communicator.template handle_data>( + communication::communicator_name(), which, generation, // step function (invoked once for get) - [&](auto& data) { data[which] = HPX_FORWARD(T, t); }, + [&t](auto& data, std::size_t which) { + data[which] = HPX_FORWARD(T, t); + }, // finalizer (invoked once after all data has been received) - [](auto& data, bool&) { return HPX_MOVE(data); }); + [](auto& data, bool&, std::size_t) { return HPX_MOVE(data); }); } template @@ -262,9 +265,12 @@ namespace hpx::traits { std::size_t generation, T&& t) { return communicator.template handle_data>( + communication::communicator_name(), which, generation, // step function (invoked for each set) - [&](auto& data) { data[which] = HPX_FORWARD(T, t); }, + [&t](auto& data, std::size_t which) { + data[which] = HPX_FORWARD(T, t); + }, // no finalizer nullptr); } @@ -312,7 +318,7 @@ namespace hpx::collectives { { // make sure id is kept alive as long as the returned future traits::detail::get_shared_state(result)->set_on_completed( - [client = HPX_MOVE(c)]() { HPX_UNUSED(client); }); + [client = HPX_MOVE(c)] { HPX_UNUSED(client); }); } return result; diff --git a/libs/full/collectives/include/hpx/collectives/inclusive_scan.hpp b/libs/full/collectives/include/hpx/collectives/inclusive_scan.hpp index 63adff323451..df3c5ce7ec87 100644 --- a/libs/full/collectives/include/hpx/collectives/inclusive_scan.hpp +++ b/libs/full/collectives/include/hpx/collectives/inclusive_scan.hpp @@ -1,4 +1,4 @@ -// Copyright (c) 2019-2023 Hartmut Kaiser +// Copyright (c) 2019-2024 Hartmut Kaiser // // SPDX-License-Identifier: BSL-1.0 // Distributed under the Boost Software License, Version 1.0. (See accompanying @@ -157,12 +157,16 @@ namespace hpx::traits { std::size_t generation, T&& t, F&& op) { return communicator.template handle_data>( + communication::communicator_name< + communication::inclusive_scan_tag>(), which, generation, // step function (invoked for each get) - [&](auto& data) { data[which] = HPX_FORWARD(T, t); }, + [&t](auto& data, std::size_t which) { + data[which] = HPX_FORWARD(T, t); + }, // finalizer (invoked after all data has been received) - [which, op = HPX_FORWARD(F, op)]( - auto& data, bool& data_available) mutable { + [op = HPX_FORWARD(F, op)](auto& data, bool& data_available, + std::size_t which) mutable { if (!data_available) { std::vector> dest; @@ -221,7 +225,7 @@ namespace hpx::collectives { { // make sure id is kept alive as long as the returned future traits::detail::get_shared_state(result)->set_on_completed( - [client = HPX_MOVE(c)]() { HPX_UNUSED(client); }); + [client = HPX_MOVE(c)] { HPX_UNUSED(client); }); } return result; diff --git a/libs/full/collectives/include/hpx/collectives/reduce.hpp b/libs/full/collectives/include/hpx/collectives/reduce.hpp index 670500e948fd..a0a28b8eb8d6 100644 --- a/libs/full/collectives/include/hpx/collectives/reduce.hpp +++ b/libs/full/collectives/include/hpx/collectives/reduce.hpp @@ -1,4 +1,4 @@ -// Copyright (c) 2019-2023 Hartmut Kaiser +// Copyright (c) 2019-2024 Hartmut Kaiser // // SPDX-License-Identifier: BSL-1.0 // Distributed under the Boost Software License, Version 1.0. (See accompanying @@ -253,11 +253,15 @@ namespace hpx::traits { std::size_t generation, T&& t, F&& op) { return communicator.template handle_data>( + communication::communicator_name(), which, generation, // step function (invoked once for get) - [&](auto& data) { data[which] = HPX_FORWARD(T, t); }, + [&t](auto& data, std::size_t which) { + data[which] = HPX_FORWARD(T, t); + }, // finalizer (invoked once after all data has been received) - [op = HPX_FORWARD(F, op)](auto& data, bool&) mutable { + [op = HPX_FORWARD(F, op)]( + auto& data, bool&, std::size_t) mutable { HPX_ASSERT(!data.empty()); if (data.size() > 1) { @@ -276,9 +280,12 @@ namespace hpx::traits { std::size_t generation, T&& t) { return communicator.template handle_data>( + communication::communicator_name(), which, generation, // step function (invoked for each set) - [&](auto& data) { data[which] = HPX_FORWARD(T, t); }, + [t = HPX_FORWARD(T, t)](auto& data, std::size_t which) mutable { + data[which] = HPX_FORWARD(T, t); + }, // no finalizer nullptr); } @@ -326,7 +333,7 @@ namespace hpx::collectives { { // make sure id is kept alive as long as the returned future traits::detail::get_shared_state(result)->set_on_completed( - [client = HPX_MOVE(c)]() { HPX_UNUSED(client); }); + [client = HPX_MOVE(c)] { HPX_UNUSED(client); }); } return result; diff --git a/libs/full/collectives/include/hpx/collectives/scatter.hpp b/libs/full/collectives/include/hpx/collectives/scatter.hpp index 85f2908f02a4..e8d3985105b9 100644 --- a/libs/full/collectives/include/hpx/collectives/scatter.hpp +++ b/libs/full/collectives/include/hpx/collectives/scatter.hpp @@ -1,4 +1,4 @@ -// Copyright (c) 2014-2023 Hartmut Kaiser +// Copyright (c) 2014-2024 Hartmut Kaiser // // SPDX-License-Identifier: BSL-1.0 // Distributed under the Boost Software License, Version 1.0. (See accompanying @@ -244,12 +244,13 @@ namespace hpx::traits { { using data_type = typename Result::result_type; - return communicator.template handle_data(which, - generation, + return communicator.template handle_data( + communication::communicator_name(), + which, generation, // step function (invoked once for get) nullptr, // finalizer (invoked after all sites have checked in) - [which](auto& data, bool&) { + [](auto& data, bool&, std::size_t which) { return Communicator::template handle_bool( HPX_MOVE(data[which])); }); @@ -260,11 +261,12 @@ namespace hpx::traits { std::size_t generation, std::vector&& t) { return communicator.template handle_data( + communication::communicator_name(), which, generation, // step function (invoked once for set) - [&](auto& data) { data = HPX_MOVE(t); }, + [&t](auto& data, std::size_t) { data = HPX_MOVE(t); }, // finalizer (invoked after all sites have checked in) - [which](auto& data, bool&) { + [](auto& data, bool&, std::size_t which) { return Communicator::template handle_bool( HPX_MOVE(data[which])); }); @@ -306,7 +308,7 @@ namespace hpx::collectives { { // make sure id is kept alive as long as the returned future traits::detail::get_shared_state(result)->set_on_completed( - [client = HPX_MOVE(c)]() { HPX_UNUSED(client); }); + [client = HPX_MOVE(c)] { HPX_UNUSED(client); }); } return result; diff --git a/libs/full/collectives/src/create_communicator.cpp b/libs/full/collectives/src/create_communicator.cpp index 7f13d2a17b36..c2f97c9cb0e5 100644 --- a/libs/full/collectives/src/create_communicator.cpp +++ b/libs/full/collectives/src/create_communicator.cpp @@ -50,8 +50,6 @@ namespace hpx::collectives { communicator_server::communicator_server() noexcept //-V730 : num_sites_(0) - , needs_initialization_(false) - , data_available_(false) { HPX_ASSERT(false); // shouldn't ever be called } @@ -59,10 +57,9 @@ namespace hpx::collectives { communicator_server::communicator_server(std::size_t num_sites) noexcept : gate_(num_sites) , num_sites_(num_sites) - , needs_initialization_(true) - , data_available_(false) { - HPX_ASSERT(num_sites != 0); + HPX_ASSERT( + num_sites != 0 && num_sites != static_cast(-1)); } } // namespace detail diff --git a/libs/full/collectives/tests/unit/communication_set.cpp b/libs/full/collectives/tests/unit/communication_set.cpp index e4b55f16ef14..4083353c85ef 100644 --- a/libs/full/collectives/tests/unit/communication_set.cpp +++ b/libs/full/collectives/tests/unit/communication_set.cpp @@ -119,7 +119,7 @@ namespace hpx::traits { l); which = communicator.which_++; - communicator.gate_.set(which, std::move(l)); + communicator.gate_.set(which, l); return f; } From f3b4641021d869bd784678284f07289e884c2456 Mon Sep 17 00:00:00 2001 From: Hartmut Kaiser Date: Sat, 13 Jan 2024 10:25:09 -0600 Subject: [PATCH 2/9] Using unique ids to identify collective operations --- libs/full/collectives/CMakeLists.txt | 11 +++++- .../include/hpx/collectives/all_gather.hpp | 16 +++++--- .../include/hpx/collectives/all_reduce.hpp | 15 +++++--- .../include/hpx/collectives/all_to_all.hpp | 15 +++++--- .../include/hpx/collectives/broadcast.hpp | 19 ++++++---- .../hpx/collectives/detail/communicator.hpp | 37 +++++++++++++------ .../hpx/collectives/exclusive_scan.hpp | 15 +++++--- .../include/hpx/collectives/gather.hpp | 17 ++++++--- .../hpx/collectives/inclusive_scan.hpp | 15 +++++--- .../include/hpx/collectives/reduce.hpp | 17 ++++++--- .../include/hpx/collectives/scatter.hpp | 17 ++++++--- libs/full/collectives/src/all_gather.cpp | 26 +++++++++++++ libs/full/collectives/src/all_reduce.cpp | 26 +++++++++++++ libs/full/collectives/src/all_to_all.cpp | 26 +++++++++++++ libs/full/collectives/src/broadcast.cpp | 26 +++++++++++++ libs/full/collectives/src/exclusive_scan.cpp | 26 +++++++++++++ libs/full/collectives/src/gather.cpp | 26 +++++++++++++ libs/full/collectives/src/inclusive_scan.cpp | 26 +++++++++++++ libs/full/collectives/src/reduce.cpp | 26 +++++++++++++ libs/full/collectives/src/scatter.cpp | 26 +++++++++++++ 20 files changed, 368 insertions(+), 60 deletions(-) create mode 100644 libs/full/collectives/src/all_gather.cpp create mode 100644 libs/full/collectives/src/all_reduce.cpp create mode 100644 libs/full/collectives/src/all_to_all.cpp create mode 100644 libs/full/collectives/src/broadcast.cpp create mode 100644 libs/full/collectives/src/exclusive_scan.cpp create mode 100644 libs/full/collectives/src/gather.cpp create mode 100644 libs/full/collectives/src/inclusive_scan.cpp create mode 100644 libs/full/collectives/src/reduce.cpp create mode 100644 libs/full/collectives/src/scatter.cpp diff --git a/libs/full/collectives/CMakeLists.txt b/libs/full/collectives/CMakeLists.txt index b2a95b67c932..0c5910f9043d 100644 --- a/libs/full/collectives/CMakeLists.txt +++ b/libs/full/collectives/CMakeLists.txt @@ -54,14 +54,23 @@ set(collectives_compat_headers # Default location is $HPX_ROOT/libs/collectives/src set(collectives_sources + all_gather.cpp + all_reduce.cpp + all_to_all.cpp barrier.cpp + broadcast.cpp create_communication_set.cpp channel_communicator.cpp create_communicator.cpp - latch.cpp detail/barrier_node.cpp detail/channel_communicator_server.cpp detail/communication_set_node.cpp + exclusive_scan.cpp + gather.cpp + inclusive_scan.cpp + latch.cpp + reduce.cpp + scatter.cpp ) include(HPX_AddModule) diff --git a/libs/full/collectives/include/hpx/collectives/all_gather.hpp b/libs/full/collectives/include/hpx/collectives/all_gather.hpp index 59c3aa2bf7b0..0e83de83217f 100644 --- a/libs/full/collectives/include/hpx/collectives/all_gather.hpp +++ b/libs/full/collectives/include/hpx/collectives/all_gather.hpp @@ -130,13 +130,19 @@ namespace hpx { namespace collectives { namespace hpx::traits { namespace communication { + struct all_gather_tag; template <> - constexpr char const* communicator_name() noexcept + struct communicator_data { - return "all_gather"; - } + static constexpr char const* name() noexcept + { + return "all_gather"; + } + + HPX_EXPORT static operation_id_type id() noexcept; + }; } // namespace communication /////////////////////////////////////////////////////////////////////////// @@ -149,8 +155,8 @@ namespace hpx::traits { std::size_t generation, T&& t) { return communicator.template handle_data>( - communication::communicator_name< - communication::all_gather_tag>(), + communication::communicator_data< + communication::all_gather_tag>::id(), which, generation, // step function (invoked for each get) [&t](auto& data, std::size_t which) { diff --git a/libs/full/collectives/include/hpx/collectives/all_reduce.hpp b/libs/full/collectives/include/hpx/collectives/all_reduce.hpp index cb659bd2006b..43bdc4ce0d01 100644 --- a/libs/full/collectives/include/hpx/collectives/all_reduce.hpp +++ b/libs/full/collectives/include/hpx/collectives/all_reduce.hpp @@ -140,10 +140,15 @@ namespace hpx::traits { struct all_reduce_tag; template <> - constexpr char const* communicator_name() noexcept + struct communicator_data { - return "all_reduce"; - } + static constexpr char const* name() noexcept + { + return "all_reduce"; + } + + HPX_EXPORT static operation_id_type id() noexcept; + }; } // namespace communication /////////////////////////////////////////////////////////////////////////// @@ -156,8 +161,8 @@ namespace hpx::traits { std::size_t generation, T&& t, F&& op) { return communicator.template handle_data>( - communication::communicator_name< - communication::all_reduce_tag>(), + communication::communicator_data< + communication::all_reduce_tag>::id(), which, generation, // step function (invoked for each get) [&t](auto& data, std::size_t which) { diff --git a/libs/full/collectives/include/hpx/collectives/all_to_all.hpp b/libs/full/collectives/include/hpx/collectives/all_to_all.hpp index 3f7467939e1b..055f326e7be3 100644 --- a/libs/full/collectives/include/hpx/collectives/all_to_all.hpp +++ b/libs/full/collectives/include/hpx/collectives/all_to_all.hpp @@ -135,10 +135,15 @@ namespace hpx::traits { struct all_to_all_tag; template <> - constexpr char const* communicator_name() noexcept + struct communicator_data { - return "all_to_all"; - } + static constexpr char const* name() noexcept + { + return "all_to_all"; + } + + HPX_EXPORT static operation_id_type id() noexcept; + }; } // namespace communication /////////////////////////////////////////////////////////////////////////// @@ -151,8 +156,8 @@ namespace hpx::traits { std::size_t generation, std::vector&& t) { return communicator.template handle_data>( - communication::communicator_name< - communication::all_to_all_tag>(), + communication::communicator_data< + communication::all_to_all_tag>::id(), which, generation, // step function (invoked for each get) [&t](auto& data, std::size_t which) { diff --git a/libs/full/collectives/include/hpx/collectives/broadcast.hpp b/libs/full/collectives/include/hpx/collectives/broadcast.hpp index b424b6ddfaa1..b70fa309ca28 100644 --- a/libs/full/collectives/include/hpx/collectives/broadcast.hpp +++ b/libs/full/collectives/include/hpx/collectives/broadcast.hpp @@ -217,10 +217,15 @@ namespace hpx::traits { struct broadcast_tag; template <> - constexpr char const* communicator_name() noexcept + struct communicator_data { - return "broadcast"; - } + static constexpr char const* name() noexcept + { + return "broadcast"; + } + + HPX_EXPORT static operation_id_type id() noexcept; + }; } // namespace communication template @@ -233,8 +238,8 @@ namespace hpx::traits { using data_type = typename Result::result_type; return communicator.template handle_data( - communication::communicator_name< - communication::broadcast_tag>(), + communication::communicator_data< + communication::broadcast_tag>::id(), which, generation, // no step function nullptr, @@ -251,8 +256,8 @@ namespace hpx::traits { std::size_t generation, T&& t) { return communicator.template handle_data>( - communication::communicator_name< - communication::broadcast_tag>(), + communication::communicator_data< + communication::broadcast_tag>::id(), which, generation, // step function (invoked once for set) [&t](auto& data, std::size_t) { data[0] = HPX_FORWARD(T, t); }, diff --git a/libs/full/collectives/include/hpx/collectives/detail/communicator.hpp b/libs/full/collectives/include/hpx/collectives/detail/communicator.hpp index 28d85536bc42..ec21d34f192f 100644 --- a/libs/full/collectives/include/hpx/collectives/detail/communicator.hpp +++ b/libs/full/collectives/include/hpx/collectives/detail/communicator.hpp @@ -21,7 +21,6 @@ #include #include -#include #include #include #include @@ -35,12 +34,22 @@ namespace hpx::traits { namespace communication { + using operation_id_type = void const*; + // Retrieve name of the current communicator template - constexpr char const* communicator_name() noexcept + struct communicator_data { - return ""; - } + static constexpr char const* name() noexcept + { + return ""; + } + + static constexpr operation_id_type id() noexcept + { + return nullptr; + } + }; } // namespace communication } // namespace hpx::traits @@ -70,7 +79,8 @@ namespace hpx::collectives::detail { { LHPX_(info, " [COL] ") .format("{}(>>> {}): which({}), generation({})", op, - traits::communication::communicator_name(), + traits::communication::communicator_data< + Operation>::name(), which, generation); } @@ -78,7 +88,8 @@ namespace hpx::collectives::detail { { LHPX_(info, " [COL] ") .format("{}(<<< {}): which({}), generation({})", op_, - traits::communication::communicator_name(), + traits::communication::communicator_data< + Operation>::name(), which_, generation_); } @@ -259,9 +270,10 @@ namespace hpx::collectives::detail { // // Finalizer will be invoked under lock after all sites have checked in. template - auto handle_data(char const* operation, std::size_t which, - std::size_t generation, [[maybe_unused]] Step&& step, - Finalizer&& finalizer, + auto handle_data( + hpx::traits::communication::operation_id_type operation, + std::size_t which, std::size_t generation, + [[maybe_unused]] Step&& step, Finalizer&& finalizer, std::size_t num_values = static_cast(-1)) { auto on_ready = [this, operation, which, num_values, @@ -281,7 +293,7 @@ namespace hpx::collectives::detail { // Verify that there is no overlap between different types of // operations on the same communicator. if (current_operation_ == nullptr || - std::strcmp(current_operation_, operation) != 0) + current_operation_ != operation) { l.unlock(); HPX_THROW_EXCEPTION(hpx::error::invalid_status, @@ -345,7 +357,7 @@ namespace hpx::collectives::detail { } current_operation_ = operation; } - else if (std::strcmp(current_operation_, operation) != 0) + else if (current_operation_ != operation) { l.unlock(); HPX_THROW_EXCEPTION(hpx::error::invalid_status, @@ -423,7 +435,8 @@ namespace hpx::collectives::detail { hpx::lcos::local::and_gate gate_; std::size_t const num_sites_; std::size_t on_ready_count_ = 0; - char const* current_operation_ = nullptr; + hpx::traits::communication::operation_id_type current_operation_ = + nullptr; bool needs_initialization_ = true; bool data_available_ = false; }; diff --git a/libs/full/collectives/include/hpx/collectives/exclusive_scan.hpp b/libs/full/collectives/include/hpx/collectives/exclusive_scan.hpp index 121bd9e7983c..f1b593f2045c 100644 --- a/libs/full/collectives/include/hpx/collectives/exclusive_scan.hpp +++ b/libs/full/collectives/include/hpx/collectives/exclusive_scan.hpp @@ -152,10 +152,15 @@ namespace hpx::traits { struct exclusive_scan_tag; template <> - constexpr char const* communicator_name() noexcept + struct communicator_data { - return "exclusive_scan"; - } + static constexpr char const* name() noexcept + { + return "exclusive_scan"; + } + + HPX_EXPORT static operation_id_type id() noexcept; + }; } // namespace communication /////////////////////////////////////////////////////////////////////////// @@ -169,8 +174,8 @@ namespace hpx::traits { std::size_t generation, T&& t, F&& op) { return communicator.template handle_data>( - communication::communicator_name< - communication::exclusive_scan_tag>(), + communication::communicator_data< + communication::exclusive_scan_tag>::id(), which, generation, // step function (invoked for each get) [&t](auto& data, std::size_t which) { diff --git a/libs/full/collectives/include/hpx/collectives/gather.hpp b/libs/full/collectives/include/hpx/collectives/gather.hpp index ac85cea4e128..76a7f3acbf9f 100644 --- a/libs/full/collectives/include/hpx/collectives/gather.hpp +++ b/libs/full/collectives/include/hpx/collectives/gather.hpp @@ -236,10 +236,15 @@ namespace hpx::traits { struct gather_tag; template <> - constexpr char const* communicator_name() noexcept + struct communicator_data { - return "gather"; - } + static constexpr char const* name() noexcept + { + return "gather"; + } + + HPX_EXPORT static operation_id_type id() noexcept; + }; } // namespace communication template @@ -250,7 +255,8 @@ namespace hpx::traits { std::size_t generation, T&& t) { return communicator.template handle_data>( - communication::communicator_name(), + communication::communicator_data< + communication::gather_tag>::id(), which, generation, // step function (invoked once for get) [&t](auto& data, std::size_t which) { @@ -265,7 +271,8 @@ namespace hpx::traits { std::size_t generation, T&& t) { return communicator.template handle_data>( - communication::communicator_name(), + communication::communicator_data< + communication::gather_tag>::id(), which, generation, // step function (invoked for each set) [&t](auto& data, std::size_t which) { diff --git a/libs/full/collectives/include/hpx/collectives/inclusive_scan.hpp b/libs/full/collectives/include/hpx/collectives/inclusive_scan.hpp index df3c5ce7ec87..e7387cc1299d 100644 --- a/libs/full/collectives/include/hpx/collectives/inclusive_scan.hpp +++ b/libs/full/collectives/include/hpx/collectives/inclusive_scan.hpp @@ -140,10 +140,15 @@ namespace hpx::traits { struct inclusive_scan_tag; template <> - constexpr char const* communicator_name() noexcept + struct communicator_data { - return "inclusive_scan"; - } + static constexpr char const* name() noexcept + { + return "inclusive_scan"; + } + + HPX_EXPORT static operation_id_type id() noexcept; + }; } // namespace communication /////////////////////////////////////////////////////////////////////////// @@ -157,8 +162,8 @@ namespace hpx::traits { std::size_t generation, T&& t, F&& op) { return communicator.template handle_data>( - communication::communicator_name< - communication::inclusive_scan_tag>(), + communication::communicator_data< + communication::inclusive_scan_tag>::id(), which, generation, // step function (invoked for each get) [&t](auto& data, std::size_t which) { diff --git a/libs/full/collectives/include/hpx/collectives/reduce.hpp b/libs/full/collectives/include/hpx/collectives/reduce.hpp index a0a28b8eb8d6..bf1684f31361 100644 --- a/libs/full/collectives/include/hpx/collectives/reduce.hpp +++ b/libs/full/collectives/include/hpx/collectives/reduce.hpp @@ -237,10 +237,15 @@ namespace hpx::traits { struct reduce_tag; template <> - constexpr char const* communicator_name() noexcept + struct communicator_data { - return "reduce"; - } + static constexpr char const* name() noexcept + { + return "reduce"; + } + + HPX_EXPORT static operation_id_type id() noexcept; + }; } // namespace communication /////////////////////////////////////////////////////////////////////////// @@ -253,7 +258,8 @@ namespace hpx::traits { std::size_t generation, T&& t, F&& op) { return communicator.template handle_data>( - communication::communicator_name(), + communication::communicator_data< + communication::reduce_tag>::id(), which, generation, // step function (invoked once for get) [&t](auto& data, std::size_t which) { @@ -280,7 +286,8 @@ namespace hpx::traits { std::size_t generation, T&& t) { return communicator.template handle_data>( - communication::communicator_name(), + communication::communicator_data< + communication::reduce_tag>::id(), which, generation, // step function (invoked for each set) [t = HPX_FORWARD(T, t)](auto& data, std::size_t which) mutable { diff --git a/libs/full/collectives/include/hpx/collectives/scatter.hpp b/libs/full/collectives/include/hpx/collectives/scatter.hpp index e8d3985105b9..fb2ed7609678 100644 --- a/libs/full/collectives/include/hpx/collectives/scatter.hpp +++ b/libs/full/collectives/include/hpx/collectives/scatter.hpp @@ -229,10 +229,15 @@ namespace hpx::traits { struct scatter_tag; template <> - constexpr char const* communicator_name() noexcept + struct communicator_data { - return "scatter"; - } + static constexpr char const* name() noexcept + { + return "scatter"; + } + + HPX_EXPORT static operation_id_type id() noexcept; + }; } // namespace communication template @@ -245,7 +250,8 @@ namespace hpx::traits { using data_type = typename Result::result_type; return communicator.template handle_data( - communication::communicator_name(), + communication::communicator_data< + communication::scatter_tag>::id(), which, generation, // step function (invoked once for get) nullptr, @@ -261,7 +267,8 @@ namespace hpx::traits { std::size_t generation, std::vector&& t) { return communicator.template handle_data( - communication::communicator_name(), + communication::communicator_data< + communication::scatter_tag>::id(), which, generation, // step function (invoked once for set) [&t](auto& data, std::size_t) { data = HPX_MOVE(t); }, diff --git a/libs/full/collectives/src/all_gather.cpp b/libs/full/collectives/src/all_gather.cpp new file mode 100644 index 000000000000..cdcf847e29a4 --- /dev/null +++ b/libs/full/collectives/src/all_gather.cpp @@ -0,0 +1,26 @@ +// Copyright (c) 2024 Hartmut Kaiser +// +// SPDX-License-Identifier: BSL-1.0 +// Distributed under the Boost Software License, Version 1.0. (See accompanying +// file LICENSE_1_0.txt or copy at http://www.boost.org/LICENSE_1_0.txt) + +#include + +#if !defined(HPX_COMPUTE_DEVICE_CODE) + +#include + +#include + +namespace hpx::traits::communication { + + // This is explicitly instantiated to ensure that the id is stable across + // shared libraries. + operation_id_type communicator_data::id() noexcept + { + static std::uint8_t id = 0; + return &id; + } +} // namespace hpx::traits::communication + +#endif diff --git a/libs/full/collectives/src/all_reduce.cpp b/libs/full/collectives/src/all_reduce.cpp new file mode 100644 index 000000000000..8847c05f6532 --- /dev/null +++ b/libs/full/collectives/src/all_reduce.cpp @@ -0,0 +1,26 @@ +// Copyright (c) 2024 Hartmut Kaiser +// +// SPDX-License-Identifier: BSL-1.0 +// Distributed under the Boost Software License, Version 1.0. (See accompanying +// file LICENSE_1_0.txt or copy at http://www.boost.org/LICENSE_1_0.txt) + +#include + +#if !defined(HPX_COMPUTE_DEVICE_CODE) + +#include + +#include + +namespace hpx::traits::communication { + + // This is explicitly instantiated to ensure that the id is stable across + // shared libraries. + operation_id_type communicator_data::id() noexcept + { + static std::uint8_t id = 0; + return &id; + } +} // namespace hpx::traits::communication + +#endif diff --git a/libs/full/collectives/src/all_to_all.cpp b/libs/full/collectives/src/all_to_all.cpp new file mode 100644 index 000000000000..482b968a98e3 --- /dev/null +++ b/libs/full/collectives/src/all_to_all.cpp @@ -0,0 +1,26 @@ +// Copyright (c) 2024 Hartmut Kaiser +// +// SPDX-License-Identifier: BSL-1.0 +// Distributed under the Boost Software License, Version 1.0. (See accompanying +// file LICENSE_1_0.txt or copy at http://www.boost.org/LICENSE_1_0.txt) + +#include + +#if !defined(HPX_COMPUTE_DEVICE_CODE) + +#include + +#include + +namespace hpx::traits::communication { + + // This is explicitly instantiated to ensure that the id is stable across + // shared libraries. + operation_id_type communicator_data::id() noexcept + { + static std::uint8_t id = 0; + return &id; + } +} // namespace hpx::traits::communication + +#endif diff --git a/libs/full/collectives/src/broadcast.cpp b/libs/full/collectives/src/broadcast.cpp new file mode 100644 index 000000000000..87ddffa8b85f --- /dev/null +++ b/libs/full/collectives/src/broadcast.cpp @@ -0,0 +1,26 @@ +// Copyright (c) 2024 Hartmut Kaiser +// +// SPDX-License-Identifier: BSL-1.0 +// Distributed under the Boost Software License, Version 1.0. (See accompanying +// file LICENSE_1_0.txt or copy at http://www.boost.org/LICENSE_1_0.txt) + +#include + +#if !defined(HPX_COMPUTE_DEVICE_CODE) + +#include + +#include + +namespace hpx::traits::communication { + + // This is explicitly instantiated to ensure that the id is stable across + // shared libraries. + operation_id_type communicator_data::id() noexcept + { + static std::uint8_t id = 0; + return &id; + } +} // namespace hpx::traits::communication + +#endif diff --git a/libs/full/collectives/src/exclusive_scan.cpp b/libs/full/collectives/src/exclusive_scan.cpp new file mode 100644 index 000000000000..8a8d43864bfc --- /dev/null +++ b/libs/full/collectives/src/exclusive_scan.cpp @@ -0,0 +1,26 @@ +// Copyright (c) 2024 Hartmut Kaiser +// +// SPDX-License-Identifier: BSL-1.0 +// Distributed under the Boost Software License, Version 1.0. (See accompanying +// file LICENSE_1_0.txt or copy at http://www.boost.org/LICENSE_1_0.txt) + +#include + +#if !defined(HPX_COMPUTE_DEVICE_CODE) + +#include + +#include + +namespace hpx::traits::communication { + + // This is explicitly instantiated to ensure that the id is stable across + // shared libraries. + operation_id_type communicator_data::id() noexcept + { + static std::uint8_t id = 0; + return &id; + } +} // namespace hpx::traits::communication + +#endif diff --git a/libs/full/collectives/src/gather.cpp b/libs/full/collectives/src/gather.cpp new file mode 100644 index 000000000000..badfd86c0beb --- /dev/null +++ b/libs/full/collectives/src/gather.cpp @@ -0,0 +1,26 @@ +// Copyright (c) 2024 Hartmut Kaiser +// +// SPDX-License-Identifier: BSL-1.0 +// Distributed under the Boost Software License, Version 1.0. (See accompanying +// file LICENSE_1_0.txt or copy at http://www.boost.org/LICENSE_1_0.txt) + +#include + +#if !defined(HPX_COMPUTE_DEVICE_CODE) + +#include + +#include + +namespace hpx::traits::communication { + + // This is explicitly instantiated to ensure that the id is stable across + // shared libraries. + operation_id_type communicator_data::id() noexcept + { + static std::uint8_t id = 0; + return &id; + } +} // namespace hpx::traits::communication + +#endif diff --git a/libs/full/collectives/src/inclusive_scan.cpp b/libs/full/collectives/src/inclusive_scan.cpp new file mode 100644 index 000000000000..b75e77e3b9c8 --- /dev/null +++ b/libs/full/collectives/src/inclusive_scan.cpp @@ -0,0 +1,26 @@ +// Copyright (c) 2024 Hartmut Kaiser +// +// SPDX-License-Identifier: BSL-1.0 +// Distributed under the Boost Software License, Version 1.0. (See accompanying +// file LICENSE_1_0.txt or copy at http://www.boost.org/LICENSE_1_0.txt) + +#include + +#if !defined(HPX_COMPUTE_DEVICE_CODE) + +#include + +#include + +namespace hpx::traits::communication { + + // This is explicitly instantiated to ensure that the id is stable across + // shared libraries. + operation_id_type communicator_data::id() noexcept + { + static std::uint8_t id = 0; + return &id; + } +} // namespace hpx::traits::communication + +#endif diff --git a/libs/full/collectives/src/reduce.cpp b/libs/full/collectives/src/reduce.cpp new file mode 100644 index 000000000000..444ca2a2d965 --- /dev/null +++ b/libs/full/collectives/src/reduce.cpp @@ -0,0 +1,26 @@ +// Copyright (c) 2024 Hartmut Kaiser +// +// SPDX-License-Identifier: BSL-1.0 +// Distributed under the Boost Software License, Version 1.0. (See accompanying +// file LICENSE_1_0.txt or copy at http://www.boost.org/LICENSE_1_0.txt) + +#include + +#if !defined(HPX_COMPUTE_DEVICE_CODE) + +#include + +#include + +namespace hpx::traits::communication { + + // This is explicitly instantiated to ensure that the id is stable across + // shared libraries. + operation_id_type communicator_data::id() noexcept + { + static std::uint8_t id = 0; + return &id; + } +} // namespace hpx::traits::communication + +#endif diff --git a/libs/full/collectives/src/scatter.cpp b/libs/full/collectives/src/scatter.cpp new file mode 100644 index 000000000000..4c7c9c78991c --- /dev/null +++ b/libs/full/collectives/src/scatter.cpp @@ -0,0 +1,26 @@ +// Copyright (c) 2024 Hartmut Kaiser +// +// SPDX-License-Identifier: BSL-1.0 +// Distributed under the Boost Software License, Version 1.0. (See accompanying +// file LICENSE_1_0.txt or copy at http://www.boost.org/LICENSE_1_0.txt) + +#include + +#if !defined(HPX_COMPUTE_DEVICE_CODE) + +#include + +#include + +namespace hpx::traits::communication { + + // This is explicitly instantiated to ensure that the id is stable across + // shared libraries. + operation_id_type communicator_data::id() noexcept + { + static std::uint8_t id = 0; + return &id; + } +} // namespace hpx::traits::communication + +#endif From d4baff380ee7c0f7b9650f025c50c7f5ae500b29 Mon Sep 17 00:00:00 2001 From: Hartmut Kaiser Date: Sat, 13 Jan 2024 15:38:09 -0600 Subject: [PATCH 3/9] Use scope_exit --- .../hpx/collectives/detail/communicator.hpp | 25 +++---------------- 1 file changed, 3 insertions(+), 22 deletions(-) diff --git a/libs/full/collectives/include/hpx/collectives/detail/communicator.hpp b/libs/full/collectives/include/hpx/collectives/detail/communicator.hpp index ec21d34f192f..bfc2d895ea40 100644 --- a/libs/full/collectives/include/hpx/collectives/detail/communicator.hpp +++ b/libs/full/collectives/include/hpx/collectives/detail/communicator.hpp @@ -14,6 +14,7 @@ #include #include #include +#include #include #include #include @@ -244,27 +245,6 @@ namespace hpx::collectives::detail { return fut; } - template - struct on_exit - { - explicit constexpr on_exit(F&& f_) noexcept - : f(HPX_MOVE(f_)) - { - } - - on_exit(on_exit const&) = delete; - on_exit(on_exit&&) = delete; - on_exit& operator=(on_exit const&) = delete; - on_exit& operator=(on_exit&&) = delete; - - ~on_exit() - { - f(); - } - - F f; - }; - // Step will be invoked under lock for each site that checks in (either // set or get). // @@ -320,7 +300,8 @@ namespace hpx::collectives::detail { // On exit, keep track of number of invocations of this // callback. - on_exit _([this] { ++on_ready_count_; }); + auto on_exit = hpx::experimental::scope_exit( + [this] { ++on_ready_count_; }); if constexpr (!std::is_same_v>) From d4576856333d7c95f8c2a6f1bcc17b14a60f1eff Mon Sep 17 00:00:00 2001 From: Hartmut Kaiser Date: Sun, 21 Jan 2024 13:36:41 -0600 Subject: [PATCH 4/9] Refine sequence checking, improve error messages --- .../include/hpx/collectives/all_gather.hpp | 9 +- .../include/hpx/collectives/all_reduce.hpp | 9 +- .../include/hpx/collectives/all_to_all.hpp | 9 +- .../include/hpx/collectives/broadcast.hpp | 11 +- .../hpx/collectives/detail/communicator.hpp | 118 +++++++++++------- .../hpx/collectives/exclusive_scan.hpp | 9 +- .../include/hpx/collectives/gather.hpp | 11 +- .../hpx/collectives/inclusive_scan.hpp | 9 +- .../include/hpx/collectives/reduce.hpp | 11 +- .../include/hpx/collectives/scatter.hpp | 11 +- libs/full/collectives/src/all_gather.cpp | 8 +- libs/full/collectives/src/all_reduce.cpp | 8 +- libs/full/collectives/src/all_to_all.cpp | 8 +- libs/full/collectives/src/broadcast.cpp | 8 +- .../collectives/src/create_communicator.cpp | 10 +- libs/full/collectives/src/exclusive_scan.cpp | 8 +- libs/full/collectives/src/gather.cpp | 8 +- libs/full/collectives/src/inclusive_scan.cpp | 8 +- libs/full/collectives/src/reduce.cpp | 8 +- libs/full/collectives/src/scatter.cpp | 8 +- 20 files changed, 126 insertions(+), 163 deletions(-) diff --git a/libs/full/collectives/include/hpx/collectives/all_gather.hpp b/libs/full/collectives/include/hpx/collectives/all_gather.hpp index 0e83de83217f..d9224414bc5b 100644 --- a/libs/full/collectives/include/hpx/collectives/all_gather.hpp +++ b/libs/full/collectives/include/hpx/collectives/all_gather.hpp @@ -136,12 +136,7 @@ namespace hpx::traits { template <> struct communicator_data { - static constexpr char const* name() noexcept - { - return "all_gather"; - } - - HPX_EXPORT static operation_id_type id() noexcept; + HPX_EXPORT static char const* name() noexcept; }; } // namespace communication @@ -156,7 +151,7 @@ namespace hpx::traits { { return communicator.template handle_data>( communication::communicator_data< - communication::all_gather_tag>::id(), + communication::all_gather_tag>::name(), which, generation, // step function (invoked for each get) [&t](auto& data, std::size_t which) { diff --git a/libs/full/collectives/include/hpx/collectives/all_reduce.hpp b/libs/full/collectives/include/hpx/collectives/all_reduce.hpp index 43bdc4ce0d01..1fd006794ffa 100644 --- a/libs/full/collectives/include/hpx/collectives/all_reduce.hpp +++ b/libs/full/collectives/include/hpx/collectives/all_reduce.hpp @@ -142,12 +142,7 @@ namespace hpx::traits { template <> struct communicator_data { - static constexpr char const* name() noexcept - { - return "all_reduce"; - } - - HPX_EXPORT static operation_id_type id() noexcept; + HPX_EXPORT static char const* name() noexcept; }; } // namespace communication @@ -162,7 +157,7 @@ namespace hpx::traits { { return communicator.template handle_data>( communication::communicator_data< - communication::all_reduce_tag>::id(), + communication::all_reduce_tag>::name(), which, generation, // step function (invoked for each get) [&t](auto& data, std::size_t which) { diff --git a/libs/full/collectives/include/hpx/collectives/all_to_all.hpp b/libs/full/collectives/include/hpx/collectives/all_to_all.hpp index 055f326e7be3..e4d7648542da 100644 --- a/libs/full/collectives/include/hpx/collectives/all_to_all.hpp +++ b/libs/full/collectives/include/hpx/collectives/all_to_all.hpp @@ -137,12 +137,7 @@ namespace hpx::traits { template <> struct communicator_data { - static constexpr char const* name() noexcept - { - return "all_to_all"; - } - - HPX_EXPORT static operation_id_type id() noexcept; + HPX_EXPORT static char const* name() noexcept; }; } // namespace communication @@ -157,7 +152,7 @@ namespace hpx::traits { { return communicator.template handle_data>( communication::communicator_data< - communication::all_to_all_tag>::id(), + communication::all_to_all_tag>::name(), which, generation, // step function (invoked for each get) [&t](auto& data, std::size_t which) { diff --git a/libs/full/collectives/include/hpx/collectives/broadcast.hpp b/libs/full/collectives/include/hpx/collectives/broadcast.hpp index b70fa309ca28..bd5c12ba47f5 100644 --- a/libs/full/collectives/include/hpx/collectives/broadcast.hpp +++ b/libs/full/collectives/include/hpx/collectives/broadcast.hpp @@ -219,12 +219,7 @@ namespace hpx::traits { template <> struct communicator_data { - static constexpr char const* name() noexcept - { - return "broadcast"; - } - - HPX_EXPORT static operation_id_type id() noexcept; + HPX_EXPORT static char const* name() noexcept; }; } // namespace communication @@ -239,7 +234,7 @@ namespace hpx::traits { return communicator.template handle_data( communication::communicator_data< - communication::broadcast_tag>::id(), + communication::broadcast_tag>::name(), which, generation, // no step function nullptr, @@ -257,7 +252,7 @@ namespace hpx::traits { { return communicator.template handle_data>( communication::communicator_data< - communication::broadcast_tag>::id(), + communication::broadcast_tag>::name(), which, generation, // step function (invoked once for set) [&t](auto& data, std::size_t) { data[0] = HPX_FORWARD(T, t); }, diff --git a/libs/full/collectives/include/hpx/collectives/detail/communicator.hpp b/libs/full/collectives/include/hpx/collectives/detail/communicator.hpp index bfc2d895ea40..a8821eea3c6f 100644 --- a/libs/full/collectives/include/hpx/collectives/detail/communicator.hpp +++ b/libs/full/collectives/include/hpx/collectives/detail/communicator.hpp @@ -35,8 +35,6 @@ namespace hpx::traits { namespace communication { - using operation_id_type = void const*; - // Retrieve name of the current communicator template struct communicator_data @@ -45,11 +43,6 @@ namespace hpx::traits { { return ""; } - - static constexpr operation_id_type id() noexcept - { - return nullptr; - } }; } // namespace communication } // namespace hpx::traits @@ -65,7 +58,15 @@ namespace hpx::collectives::detail { public: HPX_EXPORT communicator_server() noexcept; - HPX_EXPORT explicit communicator_server(std::size_t num_sites) noexcept; + HPX_EXPORT explicit communicator_server( + std::size_t num_sites, char const* basename) noexcept; + + communicator_server(communicator_server const&) = delete; + communicator_server(communicator_server&&) = delete; + communicator_server& operator=(communicator_server const&) = delete; + communicator_server& operator=(communicator_server&&) = delete; + + HPX_EXPORT ~communicator_server(); private: template @@ -245,18 +246,39 @@ namespace hpx::collectives::detail { return fut; } + template + bool set_operation_and_check_sequencing(Lock& l, char const* operation, + std::size_t which, std::size_t generation) + { + if (current_operation_ == nullptr) + { + if (on_ready_count_ != 0) + { + l.unlock(); + HPX_THROW_EXCEPTION(hpx::error::invalid_status, + "communicator::handle_data", + "communicator: {}: sequencing error, on_ready callback " + "was already invoked before the start of the " + "collective operation {}, which {}, generation {}.", + basename_, operation, which, generation); + } + current_operation_ = operation; + return true; + } + return false; + } + // Step will be invoked under lock for each site that checks in (either // set or get). // // Finalizer will be invoked under lock after all sites have checked in. template - auto handle_data( - hpx::traits::communication::operation_id_type operation, - std::size_t which, std::size_t generation, - [[maybe_unused]] Step&& step, Finalizer&& finalizer, + auto handle_data(char const* operation, std::size_t which, + std::size_t generation, [[maybe_unused]] Step&& step, + Finalizer&& finalizer, std::size_t num_values = static_cast(-1)) { - auto on_ready = [this, operation, which, num_values, + auto on_ready = [this, operation, which, generation, num_values, finalizer = HPX_FORWARD(Finalizer, finalizer)]( shared_future&& f) mutable { // This callback will be invoked once for each participating @@ -278,10 +300,12 @@ namespace hpx::collectives::detail { l.unlock(); HPX_THROW_EXCEPTION(hpx::error::invalid_status, "communicator::handle_data::on_ready", - "sequencing error, operation type mismatch: invoked " - "for {}, ongoing operation {}", - operation, - current_operation_ ? current_operation_ : "unknown"); + "communicator {}: sequencing error, operation type " + "mismatch: invoked for {}, ongoing operation {}, which " + "{}, generation {}.", + basename_, operation, + current_operation_ ? current_operation_ : "unknown", + which, generation); } // Verify that the number of invocations of this callback is in @@ -291,11 +315,12 @@ namespace hpx::collectives::detail { l.unlock(); HPX_THROW_EXCEPTION(hpx::error::invalid_status, "communicator::handle_data::on_ready", - "sequencing error, an excessive number of on_ready " - "callbacks have been invoked before the end of the " - "collective {} operation. Expected count {}, received " - "count {}.", - operation, on_ready_count_, num_sites_); + "communictor {}: sequencing error, an excessive " + "number of on_ready callbacks have been invoked before " + "the end of the collective operation {}, which {}, " + "generation {}. Expected count {}, received count {}.", + basename_, operation, which, generation, + on_ready_count_, num_sites_); } // On exit, keep track of number of invocations of this @@ -314,6 +339,7 @@ namespace hpx::collectives::detail { { HPX_UNUSED(this); HPX_UNUSED(which); + HPX_UNUSED(generation); HPX_UNUSED(num_values); HPX_UNUSED(finalizer); } @@ -324,33 +350,27 @@ namespace hpx::collectives::detail { // Verify that there is no overlap between different types of // operations on the same communicator. - if (current_operation_ == nullptr) - { - if (on_ready_count_ != 0) - { - l.unlock(); - HPX_THROW_EXCEPTION(hpx::error::invalid_status, - "communicator::handle_data", - "sequencing error, on_ready callback was already " - "invoked before the start of the collective {} " - "operation", - operation); - } - current_operation_ = operation; - } - else if (current_operation_ != operation) + set_operation_and_check_sequencing(l, operation, which, generation); + + auto f = get_future_and_synchronize( + generation, num_values, HPX_MOVE(on_ready), l); + + // We may have just finished a different operation, thus we have to + // possibly reset the operation type stored in this communicator. + if (current_operation_ != operation && + !set_operation_and_check_sequencing( + l, operation, which, generation)) { l.unlock(); HPX_THROW_EXCEPTION(hpx::error::invalid_status, "communicator::handle_data", - "sequencing error, operation type mismatch: invoked for " - "{}, ongoing operation {}", - operation, current_operation_); + "communicator {}: sequencing error, operation type " + "mismatch: invoked for {}, ongoing operation {}, which {}, " + "generation {}.", + basename_, operation, current_operation_, which, + generation); } - auto f = get_future_and_synchronize( - generation, num_values, HPX_MOVE(on_ready), l); - if constexpr (!std::is_same_v>) { // call provided step function for each invocation site @@ -360,7 +380,7 @@ namespace hpx::collectives::detail { // Make sure next generation is enabled only after previous // generation has finished executing. gate_.set(which, l, - [this, operation, generation]( + [this, operation, which, generation]( auto& l, auto& gate, error_code& ec) { // This callback is invoked synchronously once for each // collective operation after all data has been received and @@ -377,8 +397,10 @@ namespace hpx::collectives::detail { "communicator::handle_data", "sequencing error, not all on_ready callbacks have " "been invoked at the end of the collective {} " - "operation. Expected count {}, received count {}.", - operation, on_ready_count_, num_sites_); + "operation. Expected count {}, received count {}, " + "which {}, generation {}.", + *operation, on_ready_count_, num_sites_, which, + generation); return; } @@ -416,10 +438,10 @@ namespace hpx::collectives::detail { hpx::lcos::local::and_gate gate_; std::size_t const num_sites_; std::size_t on_ready_count_ = 0; - hpx::traits::communication::operation_id_type current_operation_ = - nullptr; + char const* current_operation_ = nullptr; bool needs_initialization_ = true; bool data_available_ = false; + char const* basename_ = nullptr; }; } // namespace hpx::collectives::detail diff --git a/libs/full/collectives/include/hpx/collectives/exclusive_scan.hpp b/libs/full/collectives/include/hpx/collectives/exclusive_scan.hpp index f1b593f2045c..929aad09f55b 100644 --- a/libs/full/collectives/include/hpx/collectives/exclusive_scan.hpp +++ b/libs/full/collectives/include/hpx/collectives/exclusive_scan.hpp @@ -154,12 +154,7 @@ namespace hpx::traits { template <> struct communicator_data { - static constexpr char const* name() noexcept - { - return "exclusive_scan"; - } - - HPX_EXPORT static operation_id_type id() noexcept; + HPX_EXPORT static char const* name() noexcept; }; } // namespace communication @@ -175,7 +170,7 @@ namespace hpx::traits { { return communicator.template handle_data>( communication::communicator_data< - communication::exclusive_scan_tag>::id(), + communication::exclusive_scan_tag>::name(), which, generation, // step function (invoked for each get) [&t](auto& data, std::size_t which) { diff --git a/libs/full/collectives/include/hpx/collectives/gather.hpp b/libs/full/collectives/include/hpx/collectives/gather.hpp index 76a7f3acbf9f..79b4f48329c8 100644 --- a/libs/full/collectives/include/hpx/collectives/gather.hpp +++ b/libs/full/collectives/include/hpx/collectives/gather.hpp @@ -238,12 +238,7 @@ namespace hpx::traits { template <> struct communicator_data { - static constexpr char const* name() noexcept - { - return "gather"; - } - - HPX_EXPORT static operation_id_type id() noexcept; + HPX_EXPORT static char const* name() noexcept; }; } // namespace communication @@ -256,7 +251,7 @@ namespace hpx::traits { { return communicator.template handle_data>( communication::communicator_data< - communication::gather_tag>::id(), + communication::gather_tag>::name(), which, generation, // step function (invoked once for get) [&t](auto& data, std::size_t which) { @@ -272,7 +267,7 @@ namespace hpx::traits { { return communicator.template handle_data>( communication::communicator_data< - communication::gather_tag>::id(), + communication::gather_tag>::name(), which, generation, // step function (invoked for each set) [&t](auto& data, std::size_t which) { diff --git a/libs/full/collectives/include/hpx/collectives/inclusive_scan.hpp b/libs/full/collectives/include/hpx/collectives/inclusive_scan.hpp index e7387cc1299d..9e1458afbd47 100644 --- a/libs/full/collectives/include/hpx/collectives/inclusive_scan.hpp +++ b/libs/full/collectives/include/hpx/collectives/inclusive_scan.hpp @@ -142,12 +142,7 @@ namespace hpx::traits { template <> struct communicator_data { - static constexpr char const* name() noexcept - { - return "inclusive_scan"; - } - - HPX_EXPORT static operation_id_type id() noexcept; + HPX_EXPORT static char const* name() noexcept; }; } // namespace communication @@ -163,7 +158,7 @@ namespace hpx::traits { { return communicator.template handle_data>( communication::communicator_data< - communication::inclusive_scan_tag>::id(), + communication::inclusive_scan_tag>::name(), which, generation, // step function (invoked for each get) [&t](auto& data, std::size_t which) { diff --git a/libs/full/collectives/include/hpx/collectives/reduce.hpp b/libs/full/collectives/include/hpx/collectives/reduce.hpp index bf1684f31361..6b1725954fab 100644 --- a/libs/full/collectives/include/hpx/collectives/reduce.hpp +++ b/libs/full/collectives/include/hpx/collectives/reduce.hpp @@ -239,12 +239,7 @@ namespace hpx::traits { template <> struct communicator_data { - static constexpr char const* name() noexcept - { - return "reduce"; - } - - HPX_EXPORT static operation_id_type id() noexcept; + HPX_EXPORT static char const* name() noexcept; }; } // namespace communication @@ -259,7 +254,7 @@ namespace hpx::traits { { return communicator.template handle_data>( communication::communicator_data< - communication::reduce_tag>::id(), + communication::reduce_tag>::name(), which, generation, // step function (invoked once for get) [&t](auto& data, std::size_t which) { @@ -287,7 +282,7 @@ namespace hpx::traits { { return communicator.template handle_data>( communication::communicator_data< - communication::reduce_tag>::id(), + communication::reduce_tag>::name(), which, generation, // step function (invoked for each set) [t = HPX_FORWARD(T, t)](auto& data, std::size_t which) mutable { diff --git a/libs/full/collectives/include/hpx/collectives/scatter.hpp b/libs/full/collectives/include/hpx/collectives/scatter.hpp index fb2ed7609678..b24f2844bac9 100644 --- a/libs/full/collectives/include/hpx/collectives/scatter.hpp +++ b/libs/full/collectives/include/hpx/collectives/scatter.hpp @@ -231,12 +231,7 @@ namespace hpx::traits { template <> struct communicator_data { - static constexpr char const* name() noexcept - { - return "scatter"; - } - - HPX_EXPORT static operation_id_type id() noexcept; + HPX_EXPORT static char const* name() noexcept; }; } // namespace communication @@ -251,7 +246,7 @@ namespace hpx::traits { return communicator.template handle_data( communication::communicator_data< - communication::scatter_tag>::id(), + communication::scatter_tag>::name(), which, generation, // step function (invoked once for get) nullptr, @@ -268,7 +263,7 @@ namespace hpx::traits { { return communicator.template handle_data( communication::communicator_data< - communication::scatter_tag>::id(), + communication::scatter_tag>::name(), which, generation, // step function (invoked once for set) [&t](auto& data, std::size_t) { data = HPX_MOVE(t); }, diff --git a/libs/full/collectives/src/all_gather.cpp b/libs/full/collectives/src/all_gather.cpp index cdcf847e29a4..b84d877d7dec 100644 --- a/libs/full/collectives/src/all_gather.cpp +++ b/libs/full/collectives/src/all_gather.cpp @@ -10,16 +10,14 @@ #include -#include - namespace hpx::traits::communication { // This is explicitly instantiated to ensure that the id is stable across // shared libraries. - operation_id_type communicator_data::id() noexcept + char const* communicator_data::name() noexcept { - static std::uint8_t id = 0; - return &id; + static char const* name = "all_gather"; + return name; } } // namespace hpx::traits::communication diff --git a/libs/full/collectives/src/all_reduce.cpp b/libs/full/collectives/src/all_reduce.cpp index 8847c05f6532..409e17a9c1a0 100644 --- a/libs/full/collectives/src/all_reduce.cpp +++ b/libs/full/collectives/src/all_reduce.cpp @@ -10,16 +10,14 @@ #include -#include - namespace hpx::traits::communication { // This is explicitly instantiated to ensure that the id is stable across // shared libraries. - operation_id_type communicator_data::id() noexcept + char const* communicator_data::name() noexcept { - static std::uint8_t id = 0; - return &id; + static char const* name = "all_reduce"; + return name; } } // namespace hpx::traits::communication diff --git a/libs/full/collectives/src/all_to_all.cpp b/libs/full/collectives/src/all_to_all.cpp index 482b968a98e3..4037cbcfadcc 100644 --- a/libs/full/collectives/src/all_to_all.cpp +++ b/libs/full/collectives/src/all_to_all.cpp @@ -10,16 +10,14 @@ #include -#include - namespace hpx::traits::communication { // This is explicitly instantiated to ensure that the id is stable across // shared libraries. - operation_id_type communicator_data::id() noexcept + char const* communicator_data::name() noexcept { - static std::uint8_t id = 0; - return &id; + static char const* name = "all_to_all"; + return name; } } // namespace hpx::traits::communication diff --git a/libs/full/collectives/src/broadcast.cpp b/libs/full/collectives/src/broadcast.cpp index 87ddffa8b85f..578692fe31de 100644 --- a/libs/full/collectives/src/broadcast.cpp +++ b/libs/full/collectives/src/broadcast.cpp @@ -10,16 +10,14 @@ #include -#include - namespace hpx::traits::communication { // This is explicitly instantiated to ensure that the id is stable across // shared libraries. - operation_id_type communicator_data::id() noexcept + char const* communicator_data::name() noexcept { - static std::uint8_t id = 0; - return &id; + static const char* name = "broadcast"; + return name; } } // namespace hpx::traits::communication diff --git a/libs/full/collectives/src/create_communicator.cpp b/libs/full/collectives/src/create_communicator.cpp index c2f97c9cb0e5..cdf70006b2ea 100644 --- a/libs/full/collectives/src/create_communicator.cpp +++ b/libs/full/collectives/src/create_communicator.cpp @@ -54,13 +54,17 @@ namespace hpx::collectives { HPX_ASSERT(false); // shouldn't ever be called } - communicator_server::communicator_server(std::size_t num_sites) noexcept + communicator_server::communicator_server( + std::size_t num_sites, char const* basename) noexcept : gate_(num_sites) , num_sites_(num_sites) + , basename_(basename) { HPX_ASSERT( num_sites != 0 && num_sites != static_cast(-1)); } + + communicator_server::~communicator_server() = default; } // namespace detail /////////////////////////////////////////////////////////////////////////// @@ -120,7 +124,7 @@ namespace hpx::collectives { if (this_site == root_site) { // create a new communicator - auto c = hpx::local_new(num_sites); + auto c = hpx::local_new(num_sites, basename); // register the communicator's id using the given basename, this // keeps the communicator alive @@ -173,7 +177,7 @@ namespace hpx::collectives { if (this_site == root_site) { // create a new communicator - auto c = hpx::local_new(num_sites); + auto c = hpx::local_new(num_sites, basename); // register the communicator's id using the given basename, this // keeps the communicator alive diff --git a/libs/full/collectives/src/exclusive_scan.cpp b/libs/full/collectives/src/exclusive_scan.cpp index 8a8d43864bfc..7eeb77ecee46 100644 --- a/libs/full/collectives/src/exclusive_scan.cpp +++ b/libs/full/collectives/src/exclusive_scan.cpp @@ -10,16 +10,14 @@ #include -#include - namespace hpx::traits::communication { // This is explicitly instantiated to ensure that the id is stable across // shared libraries. - operation_id_type communicator_data::id() noexcept + char const* communicator_data::name() noexcept { - static std::uint8_t id = 0; - return &id; + static char const* name = "exclusive_scan"; + return name; } } // namespace hpx::traits::communication diff --git a/libs/full/collectives/src/gather.cpp b/libs/full/collectives/src/gather.cpp index badfd86c0beb..9e74e2bf6c73 100644 --- a/libs/full/collectives/src/gather.cpp +++ b/libs/full/collectives/src/gather.cpp @@ -10,16 +10,14 @@ #include -#include - namespace hpx::traits::communication { // This is explicitly instantiated to ensure that the id is stable across // shared libraries. - operation_id_type communicator_data::id() noexcept + char const* communicator_data::name() noexcept { - static std::uint8_t id = 0; - return &id; + static char const* name = "gather"; + return name; } } // namespace hpx::traits::communication diff --git a/libs/full/collectives/src/inclusive_scan.cpp b/libs/full/collectives/src/inclusive_scan.cpp index b75e77e3b9c8..77e08c04e54a 100644 --- a/libs/full/collectives/src/inclusive_scan.cpp +++ b/libs/full/collectives/src/inclusive_scan.cpp @@ -10,16 +10,14 @@ #include -#include - namespace hpx::traits::communication { // This is explicitly instantiated to ensure that the id is stable across // shared libraries. - operation_id_type communicator_data::id() noexcept + char const* communicator_data::name() noexcept { - static std::uint8_t id = 0; - return &id; + static char const* name = "inclusive_scan"; + return name; } } // namespace hpx::traits::communication diff --git a/libs/full/collectives/src/reduce.cpp b/libs/full/collectives/src/reduce.cpp index 444ca2a2d965..23561defa064 100644 --- a/libs/full/collectives/src/reduce.cpp +++ b/libs/full/collectives/src/reduce.cpp @@ -10,16 +10,14 @@ #include -#include - namespace hpx::traits::communication { // This is explicitly instantiated to ensure that the id is stable across // shared libraries. - operation_id_type communicator_data::id() noexcept + char const* communicator_data::name() noexcept { - static std::uint8_t id = 0; - return &id; + static char const* name = "reduce"; + return name; } } // namespace hpx::traits::communication diff --git a/libs/full/collectives/src/scatter.cpp b/libs/full/collectives/src/scatter.cpp index 4c7c9c78991c..b8e61757395a 100644 --- a/libs/full/collectives/src/scatter.cpp +++ b/libs/full/collectives/src/scatter.cpp @@ -10,16 +10,14 @@ #include -#include - namespace hpx::traits::communication { // This is explicitly instantiated to ensure that the id is stable across // shared libraries. - operation_id_type communicator_data::id() noexcept + char const* communicator_data::name() noexcept { - static std::uint8_t id = 0; - return &id; + static char const* name = "scatter"; + return name; } } // namespace hpx::traits::communication From adf8a6864ca2e215f6326ede5a5888b58a1e6654 Mon Sep 17 00:00:00 2001 From: Hartmut Kaiser Date: Mon, 22 Jan 2024 14:13:00 -0600 Subject: [PATCH 5/9] Fixing race in collective operations - adding test --- libs/core/futures/src/future_data.cpp | 34 +- .../include/hpx/lcos_local/and_gate.hpp | 2 +- .../hpx/collectives/detail/communicator.hpp | 54 +- .../collectives/tests/unit/CMakeLists.txt | 3 +- .../tests/unit/concurrent_collectives.cpp | 814 ++++++++++++++++++ 5 files changed, 871 insertions(+), 36 deletions(-) create mode 100644 libs/full/collectives/tests/unit/concurrent_collectives.cpp diff --git a/libs/core/futures/src/future_data.cpp b/libs/core/futures/src/future_data.cpp index 28d8eda865d3..e41a4e4d5844 100644 --- a/libs/core/futures/src/future_data.cpp +++ b/libs/core/futures/src/future_data.cpp @@ -42,10 +42,13 @@ namespace hpx::lcos::detail { /////////////////////////////////////////////////////////////////////////// struct handle_continuation_recursion_count { - handle_continuation_recursion_count() noexcept - : count_(threads::get_continuation_recursion_count()) + handle_continuation_recursion_count() = default; + + std::size_t increment() { - ++count_; + HPX_ASSERT(count_ == nullptr); + count_ = &threads::get_continuation_recursion_count(); + return ++*count_; } handle_continuation_recursion_count( @@ -59,10 +62,14 @@ namespace hpx::lcos::detail { ~handle_continuation_recursion_count() { - --count_; + if (count_ != nullptr) + { + --*count_; + } } - std::size_t& count_; + private: + std::size_t* count_ = nullptr; }; /////////////////////////////////////////////////////////////////////////// @@ -252,17 +259,20 @@ namespace hpx::lcos::detail { { // We need to run the completion on a new thread if we are on a non HPX // thread. + bool const is_hpx_thread = nullptr != hpx::threads::get_self_ptr(); + bool recurse_asynchronously = false; + + handle_continuation_recursion_count cnt; + if (is_hpx_thread) + { #if defined(HPX_HAVE_THREADS_GET_STACK_POINTER) - bool recurse_asynchronously = - !this_thread::has_sufficient_stack_space(); + recurse_asynchronously = !this_thread::has_sufficient_stack_space(); #else - handle_continuation_recursion_count const cnt; - bool recurse_asynchronously = - cnt.count_ > HPX_CONTINUATION_MAX_RECURSION_DEPTH || - (hpx::threads::get_self_ptr() == nullptr); + recurse_asynchronously = + cnt.increment() > HPX_CONTINUATION_MAX_RECURSION_DEPTH; #endif + } - bool const is_hpx_thread = nullptr != hpx::threads::get_self_ptr(); if (!is_hpx_thread || !recurse_asynchronously) { // directly execute continuation on this thread diff --git a/libs/core/lcos_local/include/hpx/lcos_local/and_gate.hpp b/libs/core/lcos_local/include/hpx/lcos_local/and_gate.hpp index 8ee7ebf0a133..96fe88bdd5a5 100644 --- a/libs/core/lcos_local/include/hpx/lcos_local/and_gate.hpp +++ b/libs/core/lcos_local/include/hpx/lcos_local/and_gate.hpp @@ -408,7 +408,7 @@ namespace hpx::lcos::local { // Note: This type is not thread-safe. It has to be protected from // concurrent access by different threads by the code using instances // of this type. - struct and_gate : public base_and_gate + struct and_gate : base_and_gate { private: using base_type = base_and_gate; diff --git a/libs/full/collectives/include/hpx/collectives/detail/communicator.hpp b/libs/full/collectives/include/hpx/collectives/detail/communicator.hpp index a8821eea3c6f..844f91de9c67 100644 --- a/libs/full/collectives/include/hpx/collectives/detail/communicator.hpp +++ b/libs/full/collectives/include/hpx/collectives/detail/communicator.hpp @@ -181,7 +181,8 @@ namespace hpx::collectives::detail { }; private: - std::size_t get_num_sites(std::size_t num_values) const noexcept + [[nodiscard]] constexpr std::size_t get_num_sites( + std::size_t num_values) const noexcept { return num_values == static_cast(-1) ? num_sites_ : num_values; @@ -231,19 +232,22 @@ namespace hpx::collectives::detail { std::size_t generation, std::size_t capacity, F&& f, Lock& l) { HPX_ASSERT_OWNS_LOCK(l); - auto sf = gate_.get_shared_future(l); - - traits::detail::get_shared_state(sf)->reserve_callbacks( - get_num_sites(capacity)); - - auto fut = sf.then(hpx::launch::sync, HPX_FORWARD(F, f)); + // Wait for the requested generation to be processed. gate_.synchronize(generation == static_cast(-1) ? gate_.generation(l) : generation, l); - return fut; + // Get future from gate only after synchronization as otherwise we + // may get a future returned that does not belong to the requested + // generation. + auto sf = gate_.get_shared_future(l); + + traits::detail::get_shared_state(sf)->reserve_callbacks( + get_num_sites(capacity)); + + return sf.then(hpx::launch::sync, HPX_FORWARD(F, f)); } template @@ -262,9 +266,16 @@ namespace hpx::collectives::detail { "collective operation {}, which {}, generation {}.", basename_, operation, which, generation); } - current_operation_ = operation; + + if (generation == static_cast(-1) || + generation == gate_.generation(l)) + { + current_operation_ = operation; + } + return true; } + return false; } @@ -284,6 +295,11 @@ namespace hpx::collectives::detail { // This callback will be invoked once for each participating // site after all sites have checked in. + // On exit, keep track of number of invocations of this + // callback. + auto on_exit = hpx::experimental::scope_exit( + [this] { ++on_ready_count_; }); + f.get(); // propagate any exceptions // It does not matter whether the lock will be acquired here. It @@ -291,6 +307,7 @@ namespace hpx::collectives::detail { // re-acquired here (if `on_ready` happens to run on a new // thread asynchronously). std::unique_lock l(mtx_, std::try_to_lock); + [[maybe_unused]] util::ignore_while_checking il(&l); // Verify that there is no overlap between different types of // operations on the same communicator. @@ -315,7 +332,7 @@ namespace hpx::collectives::detail { l.unlock(); HPX_THROW_EXCEPTION(hpx::error::invalid_status, "communicator::handle_data::on_ready", - "communictor {}: sequencing error, an excessive " + "communicator {}: sequencing error, an excessive " "number of on_ready callbacks have been invoked before " "the end of the collective operation {}, which {}, " "generation {}. Expected count {}, received count {}.", @@ -323,11 +340,6 @@ namespace hpx::collectives::detail { on_ready_count_, num_sites_); } - // On exit, keep track of number of invocations of this - // callback. - auto on_exit = hpx::experimental::scope_exit( - [this] { ++on_ready_count_; }); - if constexpr (!std::is_same_v>) { @@ -338,8 +350,6 @@ namespace hpx::collectives::detail { else { HPX_UNUSED(this); - HPX_UNUSED(which); - HPX_UNUSED(generation); HPX_UNUSED(num_values); HPX_UNUSED(finalizer); } @@ -373,7 +383,7 @@ namespace hpx::collectives::detail { if constexpr (!std::is_same_v>) { - // call provided step function for each invocation site + // Call provided step function for each invocation site. HPX_FORWARD(Step, step)(access_data(num_values), which); } @@ -399,7 +409,7 @@ namespace hpx::collectives::detail { "been invoked at the end of the collective {} " "operation. Expected count {}, received count {}, " "which {}, generation {}.", - *operation, on_ready_count_, num_sites_, which, + operation, on_ready_count_, num_sites_, which, generation); return; } @@ -416,7 +426,7 @@ namespace hpx::collectives::detail { return f; } - // protect against vector idiosyncrasies + // Protect against vector idiosyncrasies. template static constexpr decltype(auto) handle_bool(Data&& data) noexcept { @@ -433,15 +443,15 @@ namespace hpx::collectives::detail { template friend struct hpx::traits::communication_operation; - mutex_type mtx_; hpx::unique_any_nonser data_; hpx::lcos::local::and_gate gate_; std::size_t const num_sites_; std::size_t on_ready_count_ = 0; char const* current_operation_ = nullptr; + char const* basename_ = nullptr; + mutex_type mtx_; bool needs_initialization_ = true; bool data_available_ = false; - char const* basename_ = nullptr; }; } // namespace hpx::collectives::detail diff --git a/libs/full/collectives/tests/unit/CMakeLists.txt b/libs/full/collectives/tests/unit/CMakeLists.txt index fbe375a17734..6e75b80c4909 100644 --- a/libs/full/collectives/tests/unit/CMakeLists.txt +++ b/libs/full/collectives/tests/unit/CMakeLists.txt @@ -1,4 +1,4 @@ -# Copyright (c) 2019-2023 Hartmut Kaiser +# Copyright (c) 2019-2024 Hartmut Kaiser # # SPDX-License-Identifier: BSL-1.0 # Distributed under the Boost Software License, Version 1.0. (See accompanying @@ -23,6 +23,7 @@ if(HPX_WITH_NETWORKING) set(tests ${tests} broadcast_direct + concurrent_collectives exclusive_scan_ gather inclusive_scan_ diff --git a/libs/full/collectives/tests/unit/concurrent_collectives.cpp b/libs/full/collectives/tests/unit/concurrent_collectives.cpp new file mode 100644 index 000000000000..7e242d743b42 --- /dev/null +++ b/libs/full/collectives/tests/unit/concurrent_collectives.cpp @@ -0,0 +1,814 @@ +// Copyright (c) 2019-2024 Hartmut Kaiser +// +// SPDX-License-Identifier: BSL-1.0 +// Distributed under the Boost Software License, Version 1.0. (See accompanying +// file LICENSE_1_0.txt or copy at http://www.boost.org/LICENSE_1_0.txt) + +#include + +#if !defined(HPX_COMPUTE_DEVICE_CODE) +#include +#include +#include +#include + +#include +#include +#include +#include +#include +#include + +using namespace hpx::collectives; + +constexpr char const* concurrent_basename = "/test/concurrent_communicator/"; +#if defined(HPX_DEBUG) +constexpr int ITERATIONS = 100; +#else +constexpr int ITERATIONS = 1000; +#endif +constexpr std::uint32_t num_sites = 10; + +std::atomic generation(0); + +double test_all_gather( + communicator const& comm, std::uint32_t num_localities, std::uint32_t here) +{ + hpx::chrono::high_resolution_timer const t; + + for (int i = 0; i != ITERATIONS; ++i) + { + std::size_t const gen = ++generation; + + hpx::future> overall_result = + all_gather(comm, here + i, generation_arg(gen)); + + std::vector r = overall_result.get(); + HPX_TEST_EQ(r.size(), num_localities); + + for (std::size_t j = 0; j != r.size(); ++j) + { + HPX_TEST_EQ(r[j], j + i); + } + } + + return t.elapsed(); +} + +double test_all_reduce( + communicator const& comm, std::uint32_t num_localities, std::uint32_t here) +{ + hpx::chrono::high_resolution_timer const t; + + for (int i = 0; i != ITERATIONS; ++i) + { + std::size_t const gen = ++generation; + + hpx::future overall_result = all_reduce( + comm, here + i, std::plus{}, generation_arg(gen)); + + std::uint32_t sum = 0; + for (std::uint32_t j = 0; j != num_localities; ++j) + { + sum += j + i; + } + HPX_TEST_EQ(sum, overall_result.get()); + } + + return t.elapsed(); +} + +double test_all_to_all( + communicator const& comm, std::uint32_t num_localities, std::uint32_t here) +{ + hpx::chrono::high_resolution_timer const t; + + for (int i = 0; i != ITERATIONS; ++i) + { + std::size_t const gen = ++generation; + + std::vector values(num_localities); + std::fill(values.begin(), values.end(), here + i); + + hpx::future> overall_result = + all_to_all(comm, std::move(values), generation_arg(gen)); + + std::vector r = overall_result.get(); + + HPX_TEST_EQ(r.size(), num_localities); + + for (std::size_t j = 0; j != r.size(); ++j) + { + HPX_TEST_EQ(r[j], j + i); + } + } + + return t.elapsed(); +} + +double test_broadcast(communicator const& comm, std::uint32_t here) +{ + hpx::chrono::high_resolution_timer const t; + + for (std::uint32_t i = 0; i != ITERATIONS; ++i) + { + std::size_t const gen = ++generation; + + if (here == 0) + { + hpx::future result = + broadcast_to(comm, i + 42, generation_arg(gen)); + + HPX_TEST_EQ(i + 42, result.get()); + } + else + { + hpx::future result = + hpx::collectives::broadcast_from( + comm, generation_arg(gen)); + + HPX_TEST_EQ(i + 42, result.get()); + } + } + + return t.elapsed(); +} + +double test_exclusive_scan(communicator const& comm, std::uint32_t here) +{ + hpx::chrono::high_resolution_timer const t; + + for (int i = 0; i != ITERATIONS; ++i) + { + std::size_t const gen = ++generation; + + hpx::future overall_result = + exclusive_scan(comm, here + i, std::plus<>{}, generation_arg(gen)); + + std::uint32_t sum = i; + for (std::uint32_t j = 0; j < here; ++j) + { + sum += j + i; + } + HPX_TEST_EQ(sum, overall_result.get()); + } + + return t.elapsed(); +} + +double test_gather(communicator const& comm, std::uint32_t here) +{ + hpx::chrono::high_resolution_timer const t; + + for (std::uint32_t i = 0; i != ITERATIONS; ++i) + { + std::size_t const gen = ++generation; + + if (here == 0) + { + hpx::future> overall_result = + gather_here(comm, 42 + i, generation_arg(gen)); + + std::vector sol = overall_result.get(); + for (std::size_t j = 0; j != sol.size(); ++j) + { + HPX_TEST(j + 42 + i == sol[j]); + } + } + else + { + hpx::future overall_result = + gather_there(comm, here + 42 + i, generation_arg(gen)); + overall_result.get(); + } + } + + return t.elapsed(); +} + +double test_inclusive_scan(communicator const& comm, std::uint32_t here) +{ + hpx::chrono::high_resolution_timer const t; + + for (std::uint32_t i = 0; i != ITERATIONS; ++i) + { + std::size_t const gen = ++generation; + + hpx::future overall_result = inclusive_scan( + comm, here + i, std::plus{}, generation_arg(gen)); + + std::uint32_t sum = 0; + for (std::uint32_t j = 0; j != here + 1; ++j) + { + sum += j + i; + } + HPX_TEST_EQ(sum, overall_result.get()); + } + + return t.elapsed(); +} + +double test_reduce( + communicator const& comm, std::uint32_t num_localities, std::uint32_t here) +{ + hpx::chrono::high_resolution_timer const t; + + for (std::uint32_t i = 0; i != ITERATIONS; ++i) + { + std::size_t const gen = ++generation; + + std::uint32_t value = here + i; + if (here == 0) + { + hpx::future overall_result = reduce_here( + comm, std::move(value), std::plus<>{}, generation_arg(gen)); + + std::uint32_t sum = 0; + for (std::uint32_t j = 0; j != num_localities; ++j) + { + sum += j + i; + } + HPX_TEST_EQ(sum, overall_result.get()); + } + else + { + hpx::future overall_result = + reduce_there(comm, std::move(value), generation_arg(gen)); + overall_result.get(); + } + } + + return t.elapsed(); +} + +double test_scatter( + communicator const& comm, std::uint32_t num_localities, std::uint32_t here) +{ + hpx::chrono::high_resolution_timer const t; + + for (std::uint32_t i = 0; i != ITERATIONS; ++i) + { + std::size_t const gen = ++generation; + + if (here == 0) + { + std::vector data(num_localities); + std::iota(data.begin(), data.end(), 42 + i); + + hpx::future result = + scatter_to(comm, std::move(data), generation_arg(gen)); + + HPX_TEST_EQ(i + 42 + here, result.get()); + } + else + { + hpx::future result = + scatter_from(comm, generation_arg(gen)); + + HPX_TEST_EQ(i + 42 + here, result.get()); + } + } + + return t.elapsed(); +} + +//////////////////////////////////////////////////////////////////////////////// +double test_local_all_gather(std::vector const& comms) +{ + double elapsed = 0.; + + for (std::uint32_t i = 0; i != 10 * ITERATIONS; ++i) + { + std::vector> sites; + sites.reserve(num_sites); + + auto const gen = ++generation; + + // launch num_sites threads to represent different sites + for (std::uint32_t site = 0; site != num_sites; ++site) + { + sites.push_back(hpx::async([&, site, i] { + hpx::chrono::high_resolution_timer const t; + + auto const value = site; + + hpx::future> overall_result = + all_gather(comms[site], value + i, this_site_arg(site), + generation_arg(gen)); + + std::vector const r = overall_result.get(); + HPX_TEST_EQ(r.size(), num_sites); + + for (std::size_t j = 0; j != r.size(); ++j) + { + HPX_TEST_EQ(r[j], j + i); + } + + if (site == 0) + { + elapsed += t.elapsed(); + } + })); + } + + hpx::wait_all(std::move(sites)); + } + + return elapsed; +} + +double test_local_all_reduce(std::vector const& comms) +{ + double elapsed = 0.; + + for (std::uint32_t i = 0; i != 10 * ITERATIONS; ++i) + { + std::vector> sites; + sites.reserve(num_sites); + + auto const gen = ++generation; + + // launch num_sites threads to represent different sites + for (std::uint32_t site = 0; site != num_sites; ++site) + { + sites.push_back(hpx::async([&, site] { + hpx::chrono::high_resolution_timer const t; + + // test functionality based on immediate local result value + auto value = site; + + hpx::future result = + all_reduce(comms[site], value, std::plus<>{}, + this_site_arg(site), generation_arg(gen)); + + std::uint32_t sum = 0; + for (std::uint32_t j = 0; j != num_sites; ++j) + { + sum += j; + } + + HPX_TEST_EQ(sum, result.get()); + + if (site == 0) + { + elapsed += t.elapsed(); + } + })); + } + + hpx::wait_all(std::move(sites)); + } + + return elapsed; +} + +double test_local_all_to_all(std::vector const& comms) +{ + double elapsed = 0.; + + for (std::uint32_t i = 0; i != 10 * ITERATIONS; ++i) + { + std::vector> sites; + sites.reserve(num_sites); + + auto const gen = ++generation; + + // launch num_sites threads to represent different sites + for (std::uint32_t site = 0; site != num_sites; ++site) + { + sites.push_back(hpx::async([&, site]() { + hpx::chrono::high_resolution_timer const t; + + // test functionality based on immediate local result value + auto value = site; + + hpx::future> overall_result = + all_gather(comms[site], value, this_site_arg(value), + generation_arg(gen)); + + std::vector const r = overall_result.get(); + HPX_TEST_EQ(r.size(), num_sites); + + for (std::size_t j = 0; j != r.size(); ++j) + { + HPX_TEST_EQ(r[j], j); + } + + if (site == 0) + { + elapsed += t.elapsed(); + } + })); + } + + hpx::wait_all(std::move(sites)); + } + + return elapsed; +} + +double test_local_broadcast(std::vector const& comms) +{ + double elapsed = 0.; + + for (std::uint32_t i = 0; i != 10 * ITERATIONS; ++i) + { + std::vector> sites; + sites.reserve(num_sites); + + auto const gen = ++generation; + + // launch num_sites threads to represent different sites + for (std::uint32_t site = 0; site != num_sites; ++site) + { + sites.push_back(hpx::async([&, site]() { + hpx::chrono::high_resolution_timer const t; + + // test functionality based on immediate local result value + if (site == 0) + { + hpx::future result = + broadcast_to(comms[site], 42 + i, this_site_arg(site), + generation_arg(gen)); + + HPX_TEST_EQ(42 + i, result.get()); + } + else + { + hpx::future result = + hpx::collectives::broadcast_from( + comms[site], this_site_arg(site), + generation_arg(gen)); + + HPX_TEST_EQ(42 + i, result.get()); + } + + if (site == 0) + { + elapsed += t.elapsed(); + } + })); + } + + hpx::wait_all(std::move(sites)); + } + + return elapsed; +} + +double test_local_exclusive_scan(std::vector const& comms) +{ + double elapsed = 0.; + + for (std::uint32_t i = 0; i != 10 * ITERATIONS; ++i) + { + std::vector> sites; + sites.reserve(num_sites); + + auto const gen = ++generation; + + // launch num_sites threads to represent different sites + for (std::uint32_t site = 0; site != num_sites; ++site) + { + sites.push_back(hpx::async([&, site]() { + hpx::chrono::high_resolution_timer const t; + + hpx::future overall_result = + exclusive_scan(comms[site], site + i, std::plus<>{}, + this_site_arg(site), generation_arg(gen)); + + auto const result = overall_result.get(); + + std::uint32_t sum = i; + for (std::uint32_t j = 0; j != site; ++j) + { + sum += j + i; + } + HPX_TEST_EQ(sum, result); + + if (site == 0) + { + elapsed += t.elapsed(); + } + })); + } + + hpx::wait_all(std::move(sites)); + } + + return elapsed; +} + +double test_local_gather(std::vector const& comms) +{ + double elapsed = 0.; + + for (std::uint32_t i = 0; i != 10 * ITERATIONS; ++i) + { + std::vector> sites; + sites.reserve(num_sites); + + auto const gen = ++generation; + + // launch num_sites threads to represent different sites + for (std::uint32_t site = 0; site != num_sites; ++site) + { + sites.push_back(hpx::async([&, site]() { + hpx::chrono::high_resolution_timer const t; + + if (site == 0) + { + hpx::future> overall_result = + gather_here(comms[site], 42 + i, generation_arg(gen), + this_site_arg(site)); + + std::vector const sol = overall_result.get(); + for (std::size_t j = 0; j != sol.size(); ++j) + { + HPX_TEST(j + 42 + i == sol[j]); + } + } + else + { + hpx::future overall_result = + gather_there(comms[site], site + 42 + i, + generation_arg(gen), this_site_arg(site)); + overall_result.get(); + } + + if (site == 0) + { + elapsed += t.elapsed(); + } + })); + } + + hpx::wait_all(std::move(sites)); + } + + return elapsed; +} + +double test_local_inclusive_scan(std::vector const& comms) +{ + double elapsed = 0.; + + for (std::uint32_t i = 0; i != 10 * ITERATIONS; ++i) + { + std::vector> sites; + sites.reserve(num_sites); + + auto const gen = ++generation; + + // launch num_sites threads to represent different sites + for (std::uint32_t site = 0; site != num_sites; ++site) + { + sites.push_back(hpx::async([&, site]() { + hpx::chrono::high_resolution_timer const t; + + hpx::future overall_result = + inclusive_scan(comms[site], site + i, std::plus<>{}, + this_site_arg(site), generation_arg(gen)); + + auto const result = overall_result.get(); + + std::uint32_t sum = 0; + for (std::uint32_t j = 0; j != site + 1; ++j) + { + sum += j + i; + } + HPX_TEST_EQ(sum, result); + + if (site == 0) + { + elapsed += t.elapsed(); + } + })); + } + + hpx::wait_all(std::move(sites)); + } + + return elapsed; +} + +double test_local_reduce(std::vector const& comms) +{ + double elapsed = 0.; + + for (std::uint32_t i = 0; i != 10 * ITERATIONS; ++i) + { + std::vector> sites; + sites.reserve(num_sites); + + auto const gen = ++generation; + + // launch num_sites threads to represent different sites + for (std::uint32_t site = 0; site != num_sites; ++site) + { + sites.push_back(hpx::async([&, site]() { + hpx::chrono::high_resolution_timer const t; + + // test functionality based on immediate local result value + auto value = site + i; + if (site == 0) + { + hpx::future overall_result = reduce_here( + comms[site], std::move(value), std::plus<>{}, + generation_arg(gen), this_site_arg(site)); + + std::uint32_t sum = 0; + for (std::uint32_t j = 0; j != num_sites; ++j) + { + sum += j + i; + } + HPX_TEST_EQ(sum, overall_result.get()); + } + else + { + hpx::future overall_result = + reduce_there(comms[site], std::move(value), + generation_arg(gen), this_site_arg(site)); + overall_result.get(); + } + + if (site == 0) + { + elapsed += t.elapsed(); + } + })); + } + + hpx::wait_all(std::move(sites)); + } + + return elapsed; +} + +double test_local_scatter(std::vector const& comms) +{ + double elapsed = 0.; + + for (std::uint32_t i = 0; i != 10 * ITERATIONS; ++i) + { + std::vector> sites; + sites.reserve(num_sites); + + auto const gen = ++generation; + + // launch num_sites threads to represent different sites + for (std::uint32_t site = 0; site != num_sites; ++site) + { + sites.push_back(hpx::async([&, site] { + hpx::chrono::high_resolution_timer const t; + + if (site == 0) + { + std::vector data(num_sites); + std::iota(data.begin(), data.end(), 42 + i); + + hpx::future result = + scatter_to(comms[site], std::move(data), + generation_arg(gen), this_site_arg(site)); + + HPX_TEST_EQ(i + 42 + site, result.get()); + } + else + { + hpx::future result = + scatter_from(comms[site], + generation_arg(gen), this_site_arg(site)); + + HPX_TEST_EQ(i + 42 + site, result.get()); + } + + if (site == 0) + { + elapsed += t.elapsed(); + } + })); + } + + hpx::wait_all(std::move(sites)); + } + + return elapsed; +} + +//////////////////////////////////////////////////////////////////////////////// +int hpx_main() +{ + std::uint32_t const here = hpx::get_locality_id(); + +#if defined(HPX_HAVE_NETWORKING) + if (hpx::get_num_localities(hpx::launch::sync) > 1) + { + std::uint32_t const num_localities = + hpx::get_num_localities(hpx::launch::sync); + HPX_TEST_LTE(static_cast(2), num_localities); + + auto const concurrent_comm = create_communicator(concurrent_basename, + num_sites_arg(num_localities), this_site_arg(here)); + + auto f1 = + hpx::async(&test_all_gather, concurrent_comm, num_localities, here); + auto f2 = + hpx::async(&test_all_reduce, concurrent_comm, num_localities, here); + auto f3 = + hpx::async(&test_all_to_all, concurrent_comm, num_localities, here); + auto f4 = hpx::async(&test_broadcast, concurrent_comm, here); + auto f5 = hpx::async(&test_exclusive_scan, concurrent_comm, here); + auto f6 = hpx::async(&test_gather, concurrent_comm, here); + auto f7 = hpx::async(&test_inclusive_scan, concurrent_comm, here); + auto f8 = + hpx::async(&test_reduce, concurrent_comm, num_localities, here); + auto f9 = + hpx::async(&test_scatter, concurrent_comm, num_localities, here); + + hpx::wait_all(f1, f2, f3, f4, f5, f6, f7, f8, f9); + + if (here == 0) + { + std::cout << "remote all_gather timing: " + << f1.get() / ITERATIONS << " [s]\n"; + std::cout << "remote all_reduce timing: " + << f2.get() / ITERATIONS << " [s]\n"; + std::cout << "remote all_to_all timing: " + << f3.get() / ITERATIONS << " [s]\n"; + std::cout << "remote broadcast timing: " + << f4.get() / ITERATIONS << " [s]\n"; + std::cout << "remote exclusive_scan timing: " + << f5.get() / ITERATIONS << " [s]\n"; + std::cout << "remote gather timing: " + << f6.get() / ITERATIONS << " [s]\n"; + std::cout << "remote inclusive_scan timing: " + << f7.get() / ITERATIONS << " [s]\n"; + std::cout << "remote reduce timing: " + << f8.get() / ITERATIONS << " [s]\n"; + std::cout << "remote scatter timing: " + << f9.get() / ITERATIONS << " [s]\n"; + } + } +#endif + + if (here == 0) + { + generation = 0; + + std::vector concurrent_comms; + concurrent_comms.reserve(num_sites); + + for (std::uint32_t site = 0; site != num_sites; ++site) + { + concurrent_comms.push_back( + create_local_communicator(concurrent_basename, + num_sites_arg(num_sites), this_site_arg(site))); + } + + auto f1 = hpx::async(&test_local_all_gather, concurrent_comms); + auto f2 = hpx::async(&test_local_all_reduce, concurrent_comms); + auto f3 = hpx::async(&test_local_all_to_all, concurrent_comms); + auto f4 = hpx::async(&test_local_broadcast, concurrent_comms); + auto f5 = hpx::async(&test_local_exclusive_scan, concurrent_comms); + auto f6 = hpx::async(&test_local_gather, concurrent_comms); + auto f7 = hpx::async(&test_local_inclusive_scan, concurrent_comms); + auto f8 = hpx::async(&test_local_reduce, concurrent_comms); + auto f9 = hpx::async(&test_local_scatter, concurrent_comms); + + hpx::wait_all(f1, f2, f3, f4, f5, f6, f7, f8, f9); + + std::cout << "local all_gather timing: " + << f1.get() / (10 * ITERATIONS) << " [s]\n"; + std::cout << "local all_reduce timing: " + << f2.get() / (10 * ITERATIONS) << " [s]\n"; + std::cout << "local all_to_all timing: " + << f3.get() / (10 * ITERATIONS) << " [s]\n"; + std::cout << "local broadcast timing: " + << f4.get() / (10 * ITERATIONS) << " [s]\n"; + std::cout << "local exclusive_scan timing: " + << f5.get() / (10 * ITERATIONS) << " [s]\n"; + std::cout << "local gather timing: " + << f6.get() / (10 * ITERATIONS) << " [s]\n"; + std::cout << "local inclusive_scan timing: " + << f7.get() / (10 * ITERATIONS) << " [s]\n"; + std::cout << "local reduce timing: " + << f8.get() / (10 * ITERATIONS) << " [s]\n"; + std::cout << "local scatter timing: " + << f9.get() / (10 * ITERATIONS) << " [s]\n"; + } + + return hpx::finalize(); +} + +int main(int argc, char* argv[]) +{ + std::vector const cfg = {"hpx.run_hpx_main!=1"}; + + hpx::init_params init_args; + init_args.cfg = cfg; + + HPX_TEST_EQ(hpx::init(argc, argv, init_args), 0); + return hpx::util::report_errors(); +} + +#endif From d63651699a8fdb28d10b273d35fda26e8be25812 Mon Sep 17 00:00:00 2001 From: Hartmut Kaiser Date: Wed, 10 Apr 2024 14:01:34 -0500 Subject: [PATCH 6/9] Fixing test --- .../tests/unit/concurrent_collectives.cpp | 276 +++++++++++++----- 1 file changed, 200 insertions(+), 76 deletions(-) diff --git a/libs/full/collectives/tests/unit/concurrent_collectives.cpp b/libs/full/collectives/tests/unit/concurrent_collectives.cpp index 7e242d743b42..bf0552f88c7a 100644 --- a/libs/full/collectives/tests/unit/concurrent_collectives.cpp +++ b/libs/full/collectives/tests/unit/concurrent_collectives.cpp @@ -9,12 +9,16 @@ #if !defined(HPX_COMPUTE_DEVICE_CODE) #include #include +#include +#include #include +#include #include #include #include #include +#include #include #include #include @@ -29,17 +33,111 @@ constexpr int ITERATIONS = 1000; #endif constexpr std::uint32_t num_sites = 10; -std::atomic generation(0); +//////////////////////////////////////////////////////////////////////////////// +// Each generation of the communicator has to consistently represent a +// particular operation across all localities. For this reason, this test first +// generates generations to be used for the distributed tests and exposes those +// to all participating localities. +struct generations +{ + constexpr static char const* operations[] = {"all_gather", "all_reduce", + "all_to_all", "broadcast", "exclusive_scan", "gather", "inclusive_scan", + "reduce", "scatter"}; + + generations() = default; + + void init(std::mt19937& gen, std::size_t iterations) + { + std::uniform_int_distribution dist( + 0, std::size(operations) - 1); + + for (std::size_t i = 0; i != iterations * std::size(operations); ++i) + { + data[operations[dist(gen)]].generations.push_back(i + 1); + } + + was_initialized = true; + } + + bool get_next_generation(char const* operation, std::size_t& generation) + { + if (auto it = data.find(operation); it != data.end()) + { + if (it->second.current < it->second.generations.size()) + { + generation = it->second.generations[it->second.current++]; + return true; + } + return false; // no more generations available + } + return false; + } + + std::size_t get_iterations(char const* operation) const + { + if (auto it = data.find(operation); it != data.end()) + { + return it->second.generations.size(); + } + return 0; + } + + bool initialized() const + { + return was_initialized; + } + +private: + friend class hpx::serialization::access; + + template + void load(Archive& ar, unsigned) + { + ar & data; + for (auto& [k, v] : data) + { + v.current = 0; + } + } + + template + void save(Archive& ar, unsigned) const + { + ar & data; + } + + HPX_SERIALIZATION_SPLIT_MEMBER(); + + bool was_initialized = false; + struct generation_data + { + std::size_t current = 0; + std::vector generations; + }; + std::map data; +}; + +generations distributed; +generations local; + +generations get_generations(bool local_generations) +{ + generations const& result = local_generations ? local : distributed; + hpx::util::yield_while([&] { return !result.initialized(); }); + return result; +} +HPX_PLAIN_ACTION(get_generations); + +//////////////////////////////////////////////////////////////////////////////// double test_all_gather( communicator const& comm, std::uint32_t num_localities, std::uint32_t here) { hpx::chrono::high_resolution_timer const t; - for (int i = 0; i != ITERATIONS; ++i) + std::size_t gen = 0; + for (int i = 0; distributed.get_next_generation("all_gather", gen); ++i) { - std::size_t const gen = ++generation; - hpx::future> overall_result = all_gather(comm, here + i, generation_arg(gen)); @@ -60,10 +158,9 @@ double test_all_reduce( { hpx::chrono::high_resolution_timer const t; - for (int i = 0; i != ITERATIONS; ++i) + std::size_t gen = 0; + for (int i = 0; distributed.get_next_generation("all_reduce", gen); ++i) { - std::size_t const gen = ++generation; - hpx::future overall_result = all_reduce( comm, here + i, std::plus{}, generation_arg(gen)); @@ -83,10 +180,9 @@ double test_all_to_all( { hpx::chrono::high_resolution_timer const t; - for (int i = 0; i != ITERATIONS; ++i) + std::size_t gen = 0; + for (int i = 0; distributed.get_next_generation("all_to_all", gen); ++i) { - std::size_t const gen = ++generation; - std::vector values(num_localities); std::fill(values.begin(), values.end(), here + i); @@ -110,10 +206,10 @@ double test_broadcast(communicator const& comm, std::uint32_t here) { hpx::chrono::high_resolution_timer const t; - for (std::uint32_t i = 0; i != ITERATIONS; ++i) + std::size_t gen = 0; + for (std::uint32_t i = 0; distributed.get_next_generation("broadcast", gen); + ++i) { - std::size_t const gen = ++generation; - if (here == 0) { hpx::future result = @@ -138,10 +234,9 @@ double test_exclusive_scan(communicator const& comm, std::uint32_t here) { hpx::chrono::high_resolution_timer const t; - for (int i = 0; i != ITERATIONS; ++i) + std::size_t gen = 0; + for (int i = 0; distributed.get_next_generation("exclusive_scan", gen); ++i) { - std::size_t const gen = ++generation; - hpx::future overall_result = exclusive_scan(comm, here + i, std::plus<>{}, generation_arg(gen)); @@ -160,10 +255,10 @@ double test_gather(communicator const& comm, std::uint32_t here) { hpx::chrono::high_resolution_timer const t; - for (std::uint32_t i = 0; i != ITERATIONS; ++i) + std::size_t gen = 0; + for (std::uint32_t i = 0; distributed.get_next_generation("gather", gen); + ++i) { - std::size_t const gen = ++generation; - if (here == 0) { hpx::future> overall_result = @@ -190,10 +285,10 @@ double test_inclusive_scan(communicator const& comm, std::uint32_t here) { hpx::chrono::high_resolution_timer const t; - for (std::uint32_t i = 0; i != ITERATIONS; ++i) + std::size_t gen = 0; + for (std::uint32_t i = 0; + distributed.get_next_generation("inclusive_scan", gen); ++i) { - std::size_t const gen = ++generation; - hpx::future overall_result = inclusive_scan( comm, here + i, std::plus{}, generation_arg(gen)); @@ -213,10 +308,9 @@ double test_reduce( { hpx::chrono::high_resolution_timer const t; - for (std::uint32_t i = 0; i != ITERATIONS; ++i) + std::size_t gen = 0; + for (int i = 0; distributed.get_next_generation("reduce", gen); ++i) { - std::size_t const gen = ++generation; - std::uint32_t value = here + i; if (here == 0) { @@ -246,10 +340,9 @@ double test_scatter( { hpx::chrono::high_resolution_timer const t; - for (std::uint32_t i = 0; i != ITERATIONS; ++i) + std::size_t gen = 0; + for (int i = 0; distributed.get_next_generation("scatter", gen); ++i) { - std::size_t const gen = ++generation; - if (here == 0) { std::vector data(num_localities); @@ -277,13 +370,12 @@ double test_local_all_gather(std::vector const& comms) { double elapsed = 0.; - for (std::uint32_t i = 0; i != 10 * ITERATIONS; ++i) + std::size_t gen = 0; + for (std::uint32_t i = 0; local.get_next_generation("all_gather", gen); ++i) { std::vector> sites; sites.reserve(num_sites); - auto const gen = ++generation; - // launch num_sites threads to represent different sites for (std::uint32_t site = 0; site != num_sites; ++site) { @@ -321,13 +413,12 @@ double test_local_all_reduce(std::vector const& comms) { double elapsed = 0.; - for (std::uint32_t i = 0; i != 10 * ITERATIONS; ++i) + std::size_t gen = 0; + for (std::uint32_t i = 0; local.get_next_generation("all_reduce", gen); ++i) { std::vector> sites; sites.reserve(num_sites); - auto const gen = ++generation; - // launch num_sites threads to represent different sites for (std::uint32_t site = 0; site != num_sites; ++site) { @@ -366,13 +457,12 @@ double test_local_all_to_all(std::vector const& comms) { double elapsed = 0.; - for (std::uint32_t i = 0; i != 10 * ITERATIONS; ++i) + std::size_t gen = 0; + for (std::uint32_t i = 0; local.get_next_generation("all_to_all", gen); ++i) { std::vector> sites; sites.reserve(num_sites); - auto const gen = ++generation; - // launch num_sites threads to represent different sites for (std::uint32_t site = 0; site != num_sites; ++site) { @@ -411,13 +501,12 @@ double test_local_broadcast(std::vector const& comms) { double elapsed = 0.; - for (std::uint32_t i = 0; i != 10 * ITERATIONS; ++i) + std::size_t gen = 0; + for (std::uint32_t i = 0; local.get_next_generation("broadcast", gen); ++i) { std::vector> sites; sites.reserve(num_sites); - auto const gen = ++generation; - // launch num_sites threads to represent different sites for (std::uint32_t site = 0; site != num_sites; ++site) { @@ -460,13 +549,13 @@ double test_local_exclusive_scan(std::vector const& comms) { double elapsed = 0.; - for (std::uint32_t i = 0; i != 10 * ITERATIONS; ++i) + std::size_t gen = 0; + for (std::uint32_t i = 0; local.get_next_generation("exclusive_scan", gen); + ++i) { std::vector> sites; sites.reserve(num_sites); - auto const gen = ++generation; - // launch num_sites threads to represent different sites for (std::uint32_t site = 0; site != num_sites; ++site) { @@ -503,13 +592,12 @@ double test_local_gather(std::vector const& comms) { double elapsed = 0.; - for (std::uint32_t i = 0; i != 10 * ITERATIONS; ++i) + std::size_t gen = 0; + for (std::uint32_t i = 0; local.get_next_generation("gather", gen); ++i) { std::vector> sites; sites.reserve(num_sites); - auto const gen = ++generation; - // launch num_sites threads to represent different sites for (std::uint32_t site = 0; site != num_sites; ++site) { @@ -553,13 +641,13 @@ double test_local_inclusive_scan(std::vector const& comms) { double elapsed = 0.; - for (std::uint32_t i = 0; i != 10 * ITERATIONS; ++i) + std::size_t gen = 0; + for (std::uint32_t i = 0; local.get_next_generation("inclusive_scan", gen); + ++i) { std::vector> sites; sites.reserve(num_sites); - auto const gen = ++generation; - // launch num_sites threads to represent different sites for (std::uint32_t site = 0; site != num_sites; ++site) { @@ -596,13 +684,12 @@ double test_local_reduce(std::vector const& comms) { double elapsed = 0.; - for (std::uint32_t i = 0; i != 10 * ITERATIONS; ++i) + std::size_t gen = 0; + for (std::uint32_t i = 0; local.get_next_generation("reduce", gen); ++i) { std::vector> sites; sites.reserve(num_sites); - auto const gen = ++generation; - // launch num_sites threads to represent different sites for (std::uint32_t site = 0; site != num_sites; ++site) { @@ -649,13 +736,12 @@ double test_local_scatter(std::vector const& comms) { double elapsed = 0.; - for (std::uint32_t i = 0; i != 10 * ITERATIONS; ++i) + std::size_t gen = 0; + for (std::uint32_t i = 0; local.get_next_generation("scatter", gen); ++i) { std::vector> sites; sites.reserve(num_sites); - auto const gen = ++generation; - // launch num_sites threads to represent different sites for (std::uint32_t site = 0; site != num_sites; ++site) { @@ -696,10 +782,30 @@ double test_local_scatter(std::vector const& comms) } //////////////////////////////////////////////////////////////////////////////// -int hpx_main() +int hpx_main(hpx::program_options::variables_map& vm) { std::uint32_t const here = hpx::get_locality_id(); + unsigned int seed = std::random_device{}(); + if (vm.count("seed")) + seed = vm["seed"].as(); + + std::cout << "using seed: " << seed << std::endl; + std::mt19937 gen(seed); + + if (here == 0) + { + distributed.init(gen, ITERATIONS); + local.init(gen, 10 * ITERATIONS); + } + else + { + auto console = hpx::agas::get_console_locality(); + distributed = + hpx::async(get_generations_action(), console, false).get(); + local = hpx::async(get_generations_action(), console, true).get(); + } + #if defined(HPX_HAVE_NETWORKING) if (hpx::get_num_localities(hpx::launch::sync) > 1) { @@ -730,31 +836,38 @@ int hpx_main() if (here == 0) { std::cout << "remote all_gather timing: " - << f1.get() / ITERATIONS << " [s]\n"; + << f1.get() / distributed.get_iterations("all_gather") + << " [s]\n"; std::cout << "remote all_reduce timing: " - << f2.get() / ITERATIONS << " [s]\n"; + << f2.get() / distributed.get_iterations("all_reduce") + << " [s]\n"; std::cout << "remote all_to_all timing: " - << f3.get() / ITERATIONS << " [s]\n"; + << f3.get() / distributed.get_iterations("all_to_all") + << " [s]\n"; std::cout << "remote broadcast timing: " - << f4.get() / ITERATIONS << " [s]\n"; + << f4.get() / distributed.get_iterations("broadcast") + << " [s]\n"; std::cout << "remote exclusive_scan timing: " - << f5.get() / ITERATIONS << " [s]\n"; + << f5.get() / distributed.get_iterations("exclusive_scan") + << " [s]\n"; std::cout << "remote gather timing: " - << f6.get() / ITERATIONS << " [s]\n"; + << f6.get() / distributed.get_iterations("gather") + << " [s]\n"; std::cout << "remote inclusive_scan timing: " - << f7.get() / ITERATIONS << " [s]\n"; + << f7.get() / distributed.get_iterations("inclusive_scan") + << " [s]\n"; std::cout << "remote reduce timing: " - << f8.get() / ITERATIONS << " [s]\n"; + << f8.get() / distributed.get_iterations("reduce") + << " [s]\n"; std::cout << "remote scatter timing: " - << f9.get() / ITERATIONS << " [s]\n"; + << f9.get() / distributed.get_iterations("scatter") + << " [s]\n"; } } #endif if (here == 0) { - generation = 0; - std::vector concurrent_comms; concurrent_comms.reserve(num_sites); @@ -778,23 +891,25 @@ int hpx_main() hpx::wait_all(f1, f2, f3, f4, f5, f6, f7, f8, f9); std::cout << "local all_gather timing: " - << f1.get() / (10 * ITERATIONS) << " [s]\n"; + << f1.get() / local.get_iterations("all_gather") << " [s]\n"; std::cout << "local all_reduce timing: " - << f2.get() / (10 * ITERATIONS) << " [s]\n"; + << f2.get() / local.get_iterations("all_reduce") << " [s]\n"; std::cout << "local all_to_all timing: " - << f3.get() / (10 * ITERATIONS) << " [s]\n"; + << f3.get() / local.get_iterations("all_to_all") << " [s]\n"; std::cout << "local broadcast timing: " - << f4.get() / (10 * ITERATIONS) << " [s]\n"; + << f4.get() / local.get_iterations("broadcast") << " [s]\n"; std::cout << "local exclusive_scan timing: " - << f5.get() / (10 * ITERATIONS) << " [s]\n"; + << f5.get() / local.get_iterations("exclusive_scan") + << " [s]\n"; std::cout << "local gather timing: " - << f6.get() / (10 * ITERATIONS) << " [s]\n"; + << f6.get() / local.get_iterations("gather") << " [s]\n"; std::cout << "local inclusive_scan timing: " - << f7.get() / (10 * ITERATIONS) << " [s]\n"; + << f7.get() / local.get_iterations("inclusive_scan") + << " [s]\n"; std::cout << "local reduce timing: " - << f8.get() / (10 * ITERATIONS) << " [s]\n"; + << f8.get() / local.get_iterations("reduce") << " [s]\n"; std::cout << "local scatter timing: " - << f9.get() / (10 * ITERATIONS) << " [s]\n"; + << f9.get() / local.get_iterations("scatter") << " [s]\n"; } return hpx::finalize(); @@ -802,9 +917,18 @@ int hpx_main() int main(int argc, char* argv[]) { + // add command line option which controls the random number generator seed + using namespace hpx::program_options; + options_description desc_commandline( + "Usage: " HPX_APPLICATION_STRING " [options]"); + + desc_commandline.add_options()("seed,s", value(), + "the random number generator seed to use for this run"); + std::vector const cfg = {"hpx.run_hpx_main!=1"}; hpx::init_params init_args; + init_args.desc_cmdline = desc_commandline; init_args.cfg = cfg; HPX_TEST_EQ(hpx::init(argc, argv, init_args), 0); From fdd1bce80bf975e34ad7fc6371be4a5e5665d103 Mon Sep 17 00:00:00 2001 From: Hartmut Kaiser Date: Wed, 20 Mar 2024 14:31:12 -0500 Subject: [PATCH 7/9] Ignoring locks to avoid exceptions --- .circleci/config.yml | 3 +- .github/workflows/linux_debug.yml | 3 + .github/workflows/linux_debug_fetch_hwloc.yml | 3 + .github/workflows/macos_debug.yml | 3 + .github/workflows/macos_debug_fetch_hwloc.yml | 3 + .github/workflows/windows_clang_debug.yml | 4 +- .github/workflows/windows_debug_vs2019.yml | 3 + .github/workflows/windows_debug_vs2022.yml | 3 + .../windows_debug_vs2022_fetch_hwloc.yml | 4 + .jenkins/lsu/env-common.sh | 3 + CMakeLists.txt | 8 +- .../hpx/components/iostreams/ostream.hpp | 8 +- libs/core/assertion/CMakeLists.txt | 2 +- libs/core/futures/CMakeLists.txt | 1 + .../hpx/futures/detail/future_data.hpp | 9 +- .../io_service/src/io_service_thread_pool.cpp | 4 +- libs/core/lcos_local/CMakeLists.txt | 1 + .../include/hpx/lcos_local/and_gate.hpp | 2 +- .../include/hpx/lcos_local/channel.hpp | 4 +- .../lcos_local/detail/preprocess_future.hpp | 28 ++-- .../include/hpx/lcos_local/trigger.hpp | 4 +- libs/core/lock_registration/CMakeLists.txt | 17 ++- .../detail/register_locks.hpp | 125 +++++++++++++++--- .../lock_registration/src/register_locks.cpp | 121 ++++++++++------- libs/core/runtime_local/CMakeLists.txt | 1 + .../core/runtime_local/src/interval_timer.cpp | 2 +- libs/core/schedulers/CMakeLists.txt | 1 + .../include/hpx/schedulers/thread_queue.hpp | 2 +- libs/core/synchronization/CMakeLists.txt | 3 +- .../include/hpx/synchronization/barrier.hpp | 4 +- .../synchronization/condition_variable.hpp | 4 +- .../hpx/synchronization/shared_mutex.hpp | 5 +- .../src/detail/condition_variable.cpp | 4 +- .../src/detail/counting_semaphore.cpp | 4 +- .../src/detail/sliding_semaphore.cpp | 4 +- libs/core/synchronization/src/mutex.cpp | 1 + libs/core/thread_support/CMakeLists.txt | 12 +- libs/core/type_support/CMakeLists.txt | 4 +- .../hpx/type_support}/assert_owns_lock.hpp | 3 +- libs/full/agas/src/addressing_service.cpp | 2 +- .../src/server/primary_namespace_server.cpp | 4 +- libs/full/collectives/CMakeLists.txt | 2 + .../detail/communication_set_node.hpp | 4 +- .../hpx/collectives/detail/communicator.hpp | 7 +- .../tests/unit/concurrent_collectives.cpp | 11 +- .../include/hpx/parcelset/parcelport_impl.hpp | 7 - 46 files changed, 306 insertions(+), 151 deletions(-) rename libs/core/{thread_support/include/hpx/thread_support => type_support/include/hpx/type_support}/assert_owns_lock.hpp (95%) diff --git a/.circleci/config.yml b/.circleci/config.yml index 014765077dda..c42efad5aaa4 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -1,5 +1,6 @@ # Copyright (c) 2017-2018 Thomas Heller # Copyright (c) 2015 Martin Stumpf +# Copyright (c) 2022-2024 Hartmut Kaiser # # SPDX-License-Identifier: BSL-1.0 # Distributed under the Boost Software License, Version 1.0. (See accompanying @@ -777,7 +778,7 @@ jobs: # is enabled, and other machines may fail similarly. # # Having to reconfigure here forces everything to be rebuilt, thus - # we disable it all together. + # we disable it altogether. # # cmake \ # -DHPX_WITH_DATAPAR_VC=Off . diff --git a/.github/workflows/linux_debug.yml b/.github/workflows/linux_debug.yml index c6c1685f0250..551e6d7215b2 100644 --- a/.github/workflows/linux_debug.yml +++ b/.github/workflows/linux_debug.yml @@ -1,4 +1,5 @@ # Copyright (c) 2020 ETH Zurich +# Copyright (c) 2024 The STE||AR Group # # SPDX-License-Identifier: BSL-1.0 # Distributed under the Boost Software License, Version 1.0. (See accompanying @@ -28,6 +29,8 @@ jobs: -DHPX_WITH_EXAMPLES=ON \ -DHPX_WITH_TESTS=ON \ -DHPX_WITH_TESTS_MAX_THREADS_PER_LOCALITY=2 \ + -DHPX_WITH_VERIFY_LOCKS=ON \ + -DHPX_WITH_VERIFY_LOCKS_BACKTRACE=ON \ -DHPX_WITH_CHECK_MODULE_DEPENDENCIES=On - name: Build shell: bash diff --git a/.github/workflows/linux_debug_fetch_hwloc.yml b/.github/workflows/linux_debug_fetch_hwloc.yml index 0ec32cda5c0d..df887921e4ad 100644 --- a/.github/workflows/linux_debug_fetch_hwloc.yml +++ b/.github/workflows/linux_debug_fetch_hwloc.yml @@ -1,4 +1,5 @@ # Copyright (c) 2024 Vedant Nimje +# Copyright (c) 2024 The STE||AR Group # # SPDX-License-Identifier: BSL-1.0 # Distributed under the Boost Software License, Version 1.0. (See accompanying @@ -32,6 +33,8 @@ jobs: -DHPX_WITH_EXAMPLES=ON \ -DHPX_WITH_TESTS=ON \ -DHPX_WITH_TESTS_MAX_THREADS_PER_LOCALITY=2 \ + -DHPX_WITH_VERIFY_LOCKS=ON \ + -DHPX_WITH_VERIFY_LOCKS_BACKTRACE=ON \ -DHPX_WITH_CHECK_MODULE_DEPENDENCIES=On - name: Build diff --git a/.github/workflows/macos_debug.yml b/.github/workflows/macos_debug.yml index 3ddb09f960f0..de39c7c871f5 100644 --- a/.github/workflows/macos_debug.yml +++ b/.github/workflows/macos_debug.yml @@ -1,4 +1,5 @@ # Copyright (c) 2020 Mikael Simberg +# Copyright (c) 2024 The STE||AR Group # # SPDX-License-Identifier: BSL-1.0 # Distributed under the Boost Software License, Version 1.0. (See accompanying @@ -38,6 +39,8 @@ jobs: -DHPX_WITH_EXAMPLES=ON \ -DHPX_WITH_TESTS=ON \ -DHPX_WITH_TESTS_MAX_THREADS_PER_LOCALITY=3 \ + -DHPX_WITH_VERIFY_LOCKS=ON \ + -DHPX_WITH_VERIFY_LOCKS_BACKTRACE=ON \ -DHPX_WITH_CHECK_MODULE_DEPENDENCIES=ON - name: Build shell: bash diff --git a/.github/workflows/macos_debug_fetch_hwloc.yml b/.github/workflows/macos_debug_fetch_hwloc.yml index e8ad7ab94dca..628baf14827a 100644 --- a/.github/workflows/macos_debug_fetch_hwloc.yml +++ b/.github/workflows/macos_debug_fetch_hwloc.yml @@ -1,4 +1,5 @@ # Copyright (c) 2024 Vedant Nimje +# Copyright (c) 2024 The STE||AR Group # # SPDX-License-Identifier: BSL-1.0 # Distributed under the Boost Software License, Version 1.0. (See accompanying @@ -39,6 +40,8 @@ jobs: -DHPX_WITH_EXAMPLES=ON \ -DHPX_WITH_TESTS=ON \ -DHPX_WITH_TESTS_MAX_THREADS_PER_LOCALITY=3 \ + -DHPX_WITH_VERIFY_LOCKS=ON \ + -DHPX_WITH_VERIFY_LOCKS_BACKTRACE=ON \ -DHPX_WITH_CHECK_MODULE_DEPENDENCIES=ON - name: Build shell: bash diff --git a/.github/workflows/windows_clang_debug.yml b/.github/workflows/windows_clang_debug.yml index e888de86cb0f..2da4c6b4a9e6 100644 --- a/.github/workflows/windows_clang_debug.yml +++ b/.github/workflows/windows_clang_debug.yml @@ -1,5 +1,5 @@ # Copyright (c) 2020 Mikael Simberg -# Copyright (c) 2022 Hartmut Kaiser +# Copyright (c) 2022-2024 Hartmut Kaiser # # SPDX-License-Identifier: BSL-1.0 # Distributed under the Boost Software License, Version 1.0. (See accompanying @@ -37,6 +37,8 @@ jobs: -DHPX_WITH_DEPRECATION_WARNINGS=OFF \ -DHPX_WITH_TESTS_MAX_THREADS_PER_LOCALITY=2 \ -DHPX_COROUTINES_WITH_SWAP_CONTEXT_EMULATION=ON \ + -DHPX_WITH_VERIFY_LOCKS=ON \ + -DHPX_WITH_VERIFY_LOCKS_BACKTRACE=ON \ -DHPX_WITH_CHECK_MODULE_DEPENDENCIES=On \ - name: Build shell: bash diff --git a/.github/workflows/windows_debug_vs2019.yml b/.github/workflows/windows_debug_vs2019.yml index 48a50dfd105d..4ef381b46f08 100644 --- a/.github/workflows/windows_debug_vs2019.yml +++ b/.github/workflows/windows_debug_vs2019.yml @@ -1,4 +1,5 @@ # Copyright (c) 2020 Mikael Simberg +# Copyright (c) 2024 The STE||AR Group # # SPDX-License-Identifier: BSL-1.0 # Distributed under the Boost Software License, Version 1.0. (See accompanying @@ -36,6 +37,8 @@ jobs: -DHPX_WITH_DEPRECATION_WARNINGS=OFF \ -DHPX_WITH_TESTS_MAX_THREADS_PER_LOCALITY=2 \ -DHPX_COROUTINES_WITH_SWAP_CONTEXT_EMULATION=ON \ + -DHPX_WITH_VERIFY_LOCKS=ON \ + -DHPX_WITH_VERIFY_LOCKS_BACKTRACE=ON \ -DHPX_WITH_CHECK_MODULE_DEPENDENCIES=On - name: Build shell: bash diff --git a/.github/workflows/windows_debug_vs2022.yml b/.github/workflows/windows_debug_vs2022.yml index d9dead6fc17c..74d23bfc4973 100644 --- a/.github/workflows/windows_debug_vs2022.yml +++ b/.github/workflows/windows_debug_vs2022.yml @@ -1,4 +1,5 @@ # Copyright (c) 2020 Mikael Simberg +# Copyright (c) 2024 The STE||AR Group # # SPDX-License-Identifier: BSL-1.0 # Distributed under the Boost Software License, Version 1.0. (See accompanying @@ -36,6 +37,8 @@ jobs: -DHPX_WITH_DEPRECATION_WARNINGS=OFF \ -DHPX_WITH_TESTS_MAX_THREADS_PER_LOCALITY=2 \ -DHPX_COROUTINES_WITH_SWAP_CONTEXT_EMULATION=ON \ + -DHPX_WITH_VERIFY_LOCKS=ON \ + -DHPX_WITH_VERIFY_LOCKS_BACKTRACE=ON \ -DHPX_WITH_CHECK_MODULE_DEPENDENCIES=On - name: Build shell: bash diff --git a/.github/workflows/windows_debug_vs2022_fetch_hwloc.yml b/.github/workflows/windows_debug_vs2022_fetch_hwloc.yml index b4e70a76850b..5054c2468412 100644 --- a/.github/workflows/windows_debug_vs2022_fetch_hwloc.yml +++ b/.github/workflows/windows_debug_vs2022_fetch_hwloc.yml @@ -1,4 +1,5 @@ # Copyright (c) 2024 Vedant Nimje +# Copyright (c) 2024 The STE||AR Group # # SPDX-License-Identifier: BSL-1.0 # Distributed under the Boost Software License, Version 1.0. (See accompanying @@ -37,6 +38,9 @@ jobs: -DHPX_WITH_DEPRECATION_WARNINGS=OFF \ -DHPX_WITH_TESTS_MAX_THREADS_PER_LOCALITY=2 \ -DHPX_COROUTINES_WITH_SWAP_CONTEXT_EMULATION=ON \ + -DHPX_COROUTINES_WITH_SWAP_CONTEXT_EMULATION=ON \ + -DHPX_WITH_VERIFY_LOCKS=ON \ + -DHPX_WITH_VERIFY_LOCKS_BACKTRACE=ON \ -DHPX_WITH_CHECK_MODULE_DEPENDENCIES=On - name: Build shell: bash diff --git a/.jenkins/lsu/env-common.sh b/.jenkins/lsu/env-common.sh index f0bd7e5dd00b..15e4b4e91f3f 100644 --- a/.jenkins/lsu/env-common.sh +++ b/.jenkins/lsu/env-common.sh @@ -1,4 +1,5 @@ # Copyright (c) 2021 ETH Zurich +# Copyright (c) 2024 The STE||AR Group # # SPDX-License-Identifier: BSL-1.0 # Distributed under the Boost Software License, Version 1.0. (See accompanying @@ -9,6 +10,8 @@ configure_extra_options+=" -DHPX_WITH_CHECK_MODULE_DEPENDENCIES=ON" if [ "${build_type}" = "Debug" ]; then configure_extra_options+=" -DHPX_WITH_PARCELPORT_COUNTERS=ON" configure_extra_options+=" -DLCI_DEBUG=ON" + configure_extra_options+=" -DHPX_WITH_VERIFY_LOCKS=ON" +# configure_extra_options+=" -DHPX_WITH_VERIFY_LOCKS_BACKTRACE=ON" fi ctest_extra_args+=" --verbose " diff --git a/CMakeLists.txt b/CMakeLists.txt index febddff9ad22..e24967c30db5 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -1392,15 +1392,9 @@ hpx_option( OFF CATEGORY "Debugging" ) -set(HPX_WITH_VERIFY_LOCKS_DEFAULT OFF) -if(${CMAKE_BUILD_TYPE} STREQUAL "Debug") - set(HPX_WITH_VERIFY_LOCKS_DEFAULT ON) -endif() - hpx_option( HPX_WITH_VERIFY_LOCKS BOOL - "Enable lock verification code (default: OFF, enabled in debug builds)" - ${HPX_WITH_VERIFY_LOCKS_DEFAULT} + "Enable lock verification code (default: OFF, enabled in debug builds)" OFF CATEGORY "Debugging" ADVANCED ) diff --git a/components/iostreams/include/hpx/components/iostreams/ostream.hpp b/components/iostreams/include/hpx/components/iostreams/ostream.hpp index 7d1889d326eb..3918eaa8e7ec 100644 --- a/components/iostreams/include/hpx/components/iostreams/ostream.hpp +++ b/components/iostreams/include/hpx/components/iostreams/ostream.hpp @@ -269,14 +269,13 @@ namespace hpx { namespace iostreams { // Create the next buffer, returns the previous buffer buffer next = this->detail::buffer::init_locked(); - // Unlock the mutex before we cleanup. + // Unlock the mutex before we clean up. l.unlock(); // since mtx_ is recursive and apply will do an AGAS lookup, // we need to ignore the lock here in case we are called // recursively - hpx::util::ignore_while_checking il(&l); - HPX_UNUSED(il); + [[maybe_unused]] hpx::util::ignore_while_checking il(&l); // Perform the write operation, then destroy the old buffer and // stream. @@ -367,8 +366,7 @@ namespace hpx { namespace iostreams { ostream& operator<<(std_stream_type& (*manip_fun)(std_stream_type&) ) { std::unique_lock l(*mtx_); - util::ignore_while_checking ignore(&l); - HPX_UNUSED(ignore); + [[maybe_unused]] util::ignore_while_checking ignore(&l); return streaming_operator_lazy(manip_fun); } diff --git a/libs/core/assertion/CMakeLists.txt b/libs/core/assertion/CMakeLists.txt index 51574fa2fb7c..3a281e484a2d 100644 --- a/libs/core/assertion/CMakeLists.txt +++ b/libs/core/assertion/CMakeLists.txt @@ -1,4 +1,4 @@ -# Copyright (c) 2019 The STE||AR-Group +# Copyright (c) 2019-2024 The STE||AR-Group # # SPDX-License-Identifier: BSL-1.0 # Distributed under the Boost Software License, Version 1.0. (See accompanying diff --git a/libs/core/futures/CMakeLists.txt b/libs/core/futures/CMakeLists.txt index a7b48127e601..3387538bd92b 100644 --- a/libs/core/futures/CMakeLists.txt +++ b/libs/core/futures/CMakeLists.txt @@ -72,6 +72,7 @@ add_hpx_module( hpx_errors hpx_functional hpx_logging + hpx_lock_registration hpx_memory hpx_serialization hpx_synchronization diff --git a/libs/core/futures/include/hpx/futures/detail/future_data.hpp b/libs/core/futures/include/hpx/futures/detail/future_data.hpp index c0be2e72a328..2f108651a6d1 100644 --- a/libs/core/futures/include/hpx/futures/detail/future_data.hpp +++ b/libs/core/futures/include/hpx/futures/detail/future_data.hpp @@ -1,4 +1,4 @@ -// Copyright (c) 2007-2023 Hartmut Kaiser +// Copyright (c) 2007-2024 Hartmut Kaiser // // SPDX-License-Identifier: BSL-1.0 // Distributed under the Boost Software License, Version 1.0. (See accompanying @@ -17,12 +17,13 @@ #include #include #include +#include #include #include #include -#include #include #include +#include #include #include @@ -467,6 +468,7 @@ namespace hpx::lcos::detail { // At this point the lock needs to be acquired to safely access the // registered continuations std::unique_lock l(mtx_); + [[maybe_unused]] util::ignore_while_checking il(&l); // handle all threads waiting for the future to become ready auto on_completed = HPX_MOVE(on_completed_); @@ -514,6 +516,7 @@ namespace hpx::lcos::detail { // Note: cv.notify_one() above 'consumes' the lock 'l' and leaves // it unlocked when returning. HPX_ASSERT_DOESNT_OWN_LOCK(l); + il.reset_owns_registration(); // invoke the callback (continuation) function if (!on_completed.empty()) @@ -544,6 +547,7 @@ namespace hpx::lcos::detail { // At this point the lock needs to be acquired to safely access the // registered continuations std::unique_lock l(mtx_); + [[maybe_unused]] util::ignore_while_checking il(&l); // handle all threads waiting for the future to become ready auto on_completed = HPX_MOVE(on_completed_); @@ -591,6 +595,7 @@ namespace hpx::lcos::detail { // Note: cv.notify_one() above 'consumes' the lock 'l' and leaves // it unlocked when returning. HPX_ASSERT_DOESNT_OWN_LOCK(l); + il.reset_owns_registration(); // invoke the callback (continuation) function if (!on_completed.empty()) diff --git a/libs/core/io_service/src/io_service_thread_pool.cpp b/libs/core/io_service/src/io_service_thread_pool.cpp index ec05ba5775a4..ee0772aa685e 100644 --- a/libs/core/io_service/src/io_service_thread_pool.cpp +++ b/libs/core/io_service/src/io_service_thread_pool.cpp @@ -78,10 +78,10 @@ namespace hpx::threads::detail { /////////////////////////////////////////////////////////////////////////// bool io_service_thread_pool::run( - std::unique_lock& l, std::size_t num_threads) + [[maybe_unused]] std::unique_lock& l, + std::size_t num_threads) { HPX_ASSERT(l.owns_lock()); - HPX_UNUSED(l); util::barrier startup(1); return threads_->run(num_threads, false, &startup); } diff --git a/libs/core/lcos_local/CMakeLists.txt b/libs/core/lcos_local/CMakeLists.txt index f9ad5b60cb50..d4978905a229 100644 --- a/libs/core/lcos_local/CMakeLists.txt +++ b/libs/core/lcos_local/CMakeLists.txt @@ -56,5 +56,6 @@ add_hpx_module( hpx_pack_traversal hpx_errors hpx_memory + hpx_type_support CMAKE_SUBDIRS examples tests ) diff --git a/libs/core/lcos_local/include/hpx/lcos_local/and_gate.hpp b/libs/core/lcos_local/include/hpx/lcos_local/and_gate.hpp index 96fe88bdd5a5..bf5134ae1a8f 100644 --- a/libs/core/lcos_local/include/hpx/lcos_local/and_gate.hpp +++ b/libs/core/lcos_local/include/hpx/lcos_local/and_gate.hpp @@ -18,7 +18,7 @@ #include #include #include -#include +#include #include #include diff --git a/libs/core/lcos_local/include/hpx/lcos_local/channel.hpp b/libs/core/lcos_local/include/hpx/lcos_local/channel.hpp index a92c7a685223..1c0ba4c93a7e 100644 --- a/libs/core/lcos_local/include/hpx/lcos_local/channel.hpp +++ b/libs/core/lcos_local/include/hpx/lcos_local/channel.hpp @@ -1,4 +1,4 @@ -// Copyright (c) 2016-2022 Hartmut Kaiser +// Copyright (c) 2016-2024 Hartmut Kaiser // // SPDX-License-Identifier: BSL-1.0 // Distributed under the Boost Software License, Version 1.0. (See accompanying @@ -18,9 +18,9 @@ #include #include #include -#include #include #include +#include #include #include diff --git a/libs/core/lcos_local/include/hpx/lcos_local/detail/preprocess_future.hpp b/libs/core/lcos_local/include/hpx/lcos_local/detail/preprocess_future.hpp index 9fa015d211a3..dbe789a20939 100644 --- a/libs/core/lcos_local/include/hpx/lcos_local/detail/preprocess_future.hpp +++ b/libs/core/lcos_local/include/hpx/lcos_local/detail/preprocess_future.hpp @@ -70,8 +70,8 @@ namespace hpx::serialization::detail { void trigger() { - // hpx::lcos::local::promise<>::set_value() might need to acquire - // a lock, as such, we check the our triggering condition inside a + // hpx::lcos::local::promise<>::set_value() might need to acquire a + // lock, as such, we check our triggering condition inside a // critical section and trigger the promise outside of it. bool set_value = false; @@ -195,17 +195,15 @@ namespace hpx::serialization::detail { }; } // namespace hpx::serialization::detail -namespace hpx::util { - - // This is explicitly instantiated to ensure that the id is stable across - // shared libraries. - template <> - struct extra_data_helper +// This is explicitly instantiated to ensure that the id is stable across +// shared libraries. +template <> +struct hpx::util::extra_data_helper< + hpx::serialization::detail::preprocess_futures> +{ + HPX_CORE_EXPORT static hpx::util::extra_data_id_type id() noexcept; + static constexpr void reset( + hpx::serialization::detail::preprocess_futures*) noexcept { - HPX_CORE_EXPORT static extra_data_id_type id() noexcept; - static constexpr void reset( - serialization::detail::preprocess_futures*) noexcept - { - } - }; -} // namespace hpx::util + } +}; diff --git a/libs/core/lcos_local/include/hpx/lcos_local/trigger.hpp b/libs/core/lcos_local/include/hpx/lcos_local/trigger.hpp index f9272e2a930f..72d7ed8f91a1 100644 --- a/libs/core/lcos_local/include/hpx/lcos_local/trigger.hpp +++ b/libs/core/lcos_local/include/hpx/lcos_local/trigger.hpp @@ -1,4 +1,4 @@ -// Copyright (c) 2007-2022 Hartmut Kaiser +// Copyright (c) 2007-2024 Hartmut Kaiser // // SPDX-License-Identifier: BSL-1.0 // Distributed under the Boost Software License, Version 1.0. (See accompanying @@ -15,8 +15,8 @@ #include #include #include -#include #include +#include #include #include diff --git a/libs/core/lock_registration/CMakeLists.txt b/libs/core/lock_registration/CMakeLists.txt index a45f99381d73..ec7a46a2e45c 100644 --- a/libs/core/lock_registration/CMakeLists.txt +++ b/libs/core/lock_registration/CMakeLists.txt @@ -1,4 +1,4 @@ -# Copyright (c) 2019-2021 The STE||AR-Group +# Copyright (c) 2019-2024 The STE||AR-Group # # SPDX-License-Identifier: BSL-1.0 # Distributed under the Boost Software License, Version 1.0. (See accompanying @@ -9,13 +9,24 @@ list(APPEND CMAKE_MODULE_PATH "${CMAKE_CURRENT_SOURCE_DIR}/cmake") set(lock_registration_headers hpx/lock_registration/detail/register_locks.hpp) set(lock_registration_sources register_locks.cpp) +if(HPX_WITH_VERIFY_LOCKS_BACKTRACE) + set(additional_dependencies hpx_debugging) +endif() + include(HPX_AddModule) add_hpx_module( core lock_registration GLOBAL_HEADER_GEN ON + ADD_TO_GLOBAL_HEADER "hpx/lock_registration/detail/register_locks.hpp" SOURCES ${lock_registration_sources} HEADERS ${lock_registration_headers} - DEPENDENCIES hpx_assertion hpx_concepts hpx_config hpx_errors hpx_functional - hpx_type_support + DEPENDENCIES + hpx_assertion + hpx_concepts + hpx_config + hpx_errors + hpx_functional + hpx_type_support + ${additional_dependencies} CMAKE_SUBDIRS examples tests ) diff --git a/libs/core/lock_registration/include/hpx/lock_registration/detail/register_locks.hpp b/libs/core/lock_registration/include/hpx/lock_registration/detail/register_locks.hpp index e043eb33711c..e2c84259a9ca 100644 --- a/libs/core/lock_registration/include/hpx/lock_registration/detail/register_locks.hpp +++ b/libs/core/lock_registration/include/hpx/lock_registration/detail/register_locks.hpp @@ -1,4 +1,4 @@ -// Copyright (c) 2007-2022 Hartmut Kaiser +// Copyright (c) 2007-2024 Hartmut Kaiser // Copyright (c) 2014 Thomas Heller // // SPDX-License-Identifier: BSL-1.0 @@ -10,7 +10,7 @@ #include #include #include -#include +#include #include #include @@ -22,6 +22,8 @@ #include #endif +#include + /////////////////////////////////////////////////////////////////////////////// namespace hpx::util { @@ -41,6 +43,11 @@ namespace hpx::util { explicit lock_data(std::size_t trace_depth); lock_data(register_lock_data* data, std::size_t trace_depth); + lock_data(lock_data const&) = delete; + lock_data(lock_data&&) = delete; + lock_data& operator=(lock_data const&) = delete; + lock_data& operator=(lock_data&&) = delete; + ~lock_data(); bool ignore_; @@ -61,6 +68,11 @@ namespace hpx::util { { } + held_locks_data(held_locks_data const&) = delete; + held_locks_data(held_locks_data&&) = delete; + held_locks_data& operator=(held_locks_data const&) = delete; + held_locks_data& operator=(held_locks_data&&) = delete; + held_locks_map map_; bool enabled_; bool ignore_all_locks_; @@ -75,10 +87,10 @@ namespace hpx::util { HPX_CORE_EXPORT void enable_lock_detection() noexcept; HPX_CORE_EXPORT void disable_lock_detection() noexcept; HPX_CORE_EXPORT void trace_depth_lock_detection(std::size_t value) noexcept; - HPX_CORE_EXPORT void ignore_lock(void const* lock) noexcept; - HPX_CORE_EXPORT void reset_ignored(void const* lock) noexcept; - HPX_CORE_EXPORT void ignore_all_locks() noexcept; - HPX_CORE_EXPORT void reset_ignored_all() noexcept; + HPX_CORE_EXPORT bool ignore_lock(void const* lock) noexcept; + HPX_CORE_EXPORT bool reset_ignored(void const* lock) noexcept; + HPX_CORE_EXPORT bool ignore_all_locks() noexcept; + HPX_CORE_EXPORT bool reset_ignored_all() noexcept; using registered_locks_error_handler_type = hpx::function; @@ -104,14 +116,27 @@ namespace hpx::util { struct ignore_all_while_checking { ignore_all_while_checking() noexcept + : owns_registration_(ignore_all_locks()) { - ignore_all_locks(); } + ignore_all_while_checking(ignore_all_while_checking const&) = delete; + ignore_all_while_checking(ignore_all_while_checking&&) = delete; + ignore_all_while_checking& operator=( + ignore_all_while_checking const&) = delete; + ignore_all_while_checking& operator=( + ignore_all_while_checking&&) = delete; + ~ignore_all_while_checking() noexcept { - reset_ignored_all(); + if (owns_registration_) + { + reset_ignored_all(); + } } + + private: + bool owns_registration_; }; namespace detail { @@ -120,21 +145,75 @@ namespace hpx::util { } template >> + typename Enable = std::enable_if_t && + detail::has_owns_lock_v>> struct ignore_while_checking + { + explicit ignore_while_checking(Lock const* lock) noexcept + : mtx_(lock->owns_lock() ? lock->mutex() : nullptr) + , owns_registration_(false) + { + if (mtx_ != nullptr) + { + owns_registration_ = ignore_lock(mtx_); + } + } + + ignore_while_checking(ignore_while_checking const&) = delete; + ignore_while_checking(ignore_while_checking&&) = delete; + ignore_while_checking& operator=(ignore_while_checking const&) = delete; + ignore_while_checking& operator=(ignore_while_checking&&) = delete; + + ~ignore_while_checking() + { + if (mtx_ != nullptr && owns_registration_) + { + reset_ignored(mtx_); + } + } + + void reset_owns_registration() noexcept + { + owns_registration_ = false; + } + + private: + void const* mtx_; + bool owns_registration_; + }; + + template + struct ignore_while_checking && + !detail::has_owns_lock_v>> { explicit ignore_while_checking(Lock const* lock) noexcept : mtx_(lock->mutex()) + , owns_registration_(ignore_lock(mtx_)) { - ignore_lock(mtx_); } + ignore_while_checking(ignore_while_checking const&) = delete; + ignore_while_checking(ignore_while_checking&&) = delete; + ignore_while_checking& operator=(ignore_while_checking const&) = delete; + ignore_while_checking& operator=(ignore_while_checking&&) = delete; + ~ignore_while_checking() { - reset_ignored(mtx_); + if (owns_registration_) + { + reset_ignored(mtx_); + } + } + + void reset_owns_registration() noexcept + { + owns_registration_ = false; } + private: void const* mtx_; + bool owns_registration_; }; // The following functions are used to store the held locks information @@ -157,6 +236,8 @@ namespace hpx::util { explicit constexpr ignore_while_checking(Lock const* /*lock*/) noexcept { } + + constexpr void reset_owns_registration() noexcept {} }; struct ignore_all_while_checking @@ -181,11 +262,23 @@ namespace hpx::util { std::size_t /*value*/) noexcept { } - constexpr inline void ignore_lock(void const* /*lock*/) noexcept {} - constexpr inline void reset_ignored(void const* /*lock*/) noexcept {} + constexpr inline bool ignore_lock(void const* /*lock*/) noexcept + { + return true; + } + constexpr inline bool reset_ignored(void const* /*lock*/) noexcept + { + return true; + } - constexpr inline void ignore_all_locks() noexcept {} - constexpr inline void reset_ignored_all() noexcept {} + constexpr inline bool ignore_all_locks() noexcept + { + return true; + } + constexpr inline bool reset_ignored_all() noexcept + { + return true; + } struct held_locks_data { @@ -200,3 +293,5 @@ namespace hpx::util { #endif } // namespace hpx::util + +#include diff --git a/libs/core/lock_registration/src/register_locks.cpp b/libs/core/lock_registration/src/register_locks.cpp index fe76203f8dea..5106165347ed 100644 --- a/libs/core/lock_registration/src/register_locks.cpp +++ b/libs/core/lock_registration/src/register_locks.cpp @@ -12,10 +12,13 @@ #include #include #include +#ifdef HPX_HAVE_VERIFY_LOCKS_BACKTRACE +#include +#endif #include #include -#include +#include #include /////////////////////////////////////////////////////////////////////////////// @@ -38,7 +41,7 @@ namespace hpx::util { : ignore_(false) , user_data_(data) #ifdef HPX_HAVE_VERIFY_LOCKS_BACKTRACE - , backtrace_(hpx::detail::trace(trace_depth)) + , backtrace_(hpx::util::trace(trace_depth)) #endif { } @@ -51,20 +54,26 @@ namespace hpx::util { struct held_locks_data_ptr { held_locks_data_ptr() - : data_(new held_locks_data) + : data_(std::make_unique()) { } void reinitialize() { - data_.reset(new held_locks_data); + data_ = std::make_unique(); } // note: this invalidates the stored pointer - this is intentional - std::unique_ptr release() noexcept + std::unique_ptr release() && noexcept { HPX_ASSERT(!!data_); - return HPX_MOVE(data_); + + std::unique_ptr result; + + using std::swap; + swap(result, data_); + + return result; } void set(std::unique_ptr&& data) noexcept @@ -81,7 +90,7 @@ namespace hpx::util { static held_locks_data_ptr& get_held_locks() { - static thread_local held_locks_data_ptr held_locks; + thread_local held_locks_data_ptr held_locks; if (!held_locks.data_) { held_locks.reinitialize(); @@ -112,9 +121,15 @@ namespace hpx::util { return !get_held_locks().data_->ignore_all_locks_; } - static void set_ignore_all_locks(bool enable) + static bool set_ignore_all_locks(bool enable) { - get_held_locks().data_->ignore_all_locks_ = enable; + bool& val= get_held_locks().data_->ignore_all_locks_; + if (val != enable) + { + val = enable; + return true; + } + return false; } }; @@ -126,7 +141,7 @@ namespace hpx::util { // retrieve the current thread_local data about held locks std::unique_ptr get_held_locks_data() { - return detail::register_locks::get_held_locks().release(); + return HPX_MOVE(detail::register_locks::get_held_locks()).release(); } // set the current thread_local data about held locks @@ -151,7 +166,10 @@ namespace hpx::util { detail::register_locks::lock_detection_trace_depth_ = value; } - static registered_locks_error_handler_type registered_locks_error_handler; + namespace { + + registered_locks_error_handler_type registered_locks_error_handler; + } void set_registered_locks_error_handler( registered_locks_error_handler_type f) noexcept @@ -159,7 +177,10 @@ namespace hpx::util { registered_locks_error_handler = HPX_MOVE(f); } - static register_locks_predicate_type register_locks_predicate; + namespace { + + register_locks_predicate_type register_locks_predicate; + } void set_register_locks_predicate(register_locks_predicate_type f) noexcept { @@ -180,23 +201,21 @@ namespace hpx::util { register_locks::held_locks_map& held_locks = register_locks::get_lock_map(); - register_locks::held_locks_map::iterator it = - held_locks.find(lock); - if (it != held_locks.end()) + if (held_locks.find(lock) != held_locks.end()) return false; // this lock is already registered std::pair p; if (!data) { - p = held_locks.insert(std::make_pair(lock, - detail::lock_data( - register_locks::lock_detection_trace_depth_))); + p = held_locks.emplace( + lock, register_locks::lock_detection_trace_depth_); } else { - p = held_locks.insert(std::make_pair(lock, - detail::lock_data(data, - register_locks::lock_detection_trace_depth_))); + p = held_locks.emplace(std::piecewise_construct, + std::forward_as_tuple(lock), + std::forward_as_tuple( + data, register_locks::lock_detection_trace_depth_)); } return p.second; } @@ -221,9 +240,7 @@ namespace hpx::util { register_locks::held_locks_map& held_locks = register_locks::get_lock_map(); - register_locks::held_locks_map::iterator it = - held_locks.find(lock); - if (it == held_locks.end()) + if (held_locks.find(lock) == held_locks.end()) return false; // this lock is not registered held_locks.erase(lock); @@ -242,10 +259,8 @@ namespace hpx::util { inline bool some_locks_are_not_ignored( register_locks::held_locks_map const& held_locks) noexcept { - using iterator = register_locks::held_locks_map::const_iterator; - - iterator end = held_locks.end(); - for (iterator it = held_locks.begin(); it != end; ++it) + auto const end = held_locks.end(); + for (auto it = held_locks.begin(); it != end; ++it) { //lock_data const& data = *(*it).second; if (!it->second.ignore_) @@ -266,20 +281,19 @@ namespace hpx::util { if (enabled && register_locks::lock_detection_enabled_ && (!register_locks_predicate || register_locks_predicate())) { - register_locks::held_locks_map& held_locks = - register_locks::get_lock_map(); - // we create a log message if there are still registered locks for // this OS-thread - if (!held_locks.empty()) + if (register_locks::held_locks_map const& held_locks = + register_locks::get_lock_map(); + !held_locks.empty()) { // Temporarily disable verifying locks in case verify_no_locks // gets called recursively. - auto old_value = detail::register_locks::get_lock_enabled(); + auto old_value = register_locks::get_lock_enabled(); - detail::register_locks::set_lock_enabled(false); + register_locks::set_lock_enabled(false); auto on_exit = hpx::experimental::scope_exit([old_value] { - detail::register_locks::set_lock_enabled(old_value); + register_locks::set_lock_enabled(old_value); }); if (detail::some_locks_are_not_ignored(held_locks)) @@ -324,7 +338,7 @@ namespace hpx::util { namespace detail { - void set_ignore_status(void const* lock, bool status) + bool set_ignore_status(void const* lock, bool status) { if (register_locks::lock_detection_enabled_ && (!register_locks_predicate || register_locks_predicate())) @@ -332,64 +346,69 @@ namespace hpx::util { register_locks::held_locks_map& held_locks = register_locks::get_lock_map(); - register_locks::held_locks_map::iterator it = - held_locks.find(lock); + auto const it = held_locks.find(lock); if (it == held_locks.end()) { // this can happen if the lock was registered to be ignored // on a different OS thread - // HPX_THROW_EXCEPTION( - // hpx::error::invalid_status, "set_ignore_status", - // "The given lock has not been registered."); - return; + return false; } - it->second.ignore_ = status; + if (it->second.ignore_ != status) + { + it->second.ignore_ = status; + return true; + } } + return false; } } // namespace detail - void ignore_lock(void const* lock) noexcept + bool ignore_lock(void const* lock) noexcept { try { - detail::set_ignore_status(lock, true); + return detail::set_ignore_status(lock, true); } catch (...) { + return false; } } - void reset_ignored(void const* lock) noexcept + bool reset_ignored(void const* lock) noexcept { try { - detail::set_ignore_status(lock, false); + return detail::set_ignore_status(lock, false); } catch (...) { + return false; } } - void ignore_all_locks() noexcept + bool ignore_all_locks() noexcept { try { - detail::register_locks::set_ignore_all_locks(true); + return detail::register_locks::set_ignore_all_locks(true); } catch (...) { + return false; } } - void reset_ignored_all() noexcept + bool reset_ignored_all() noexcept { try { - detail::register_locks::set_ignore_all_locks(false); + return detail::register_locks::set_ignore_all_locks(false); } catch (...) { + return false; } } } // namespace hpx::util diff --git a/libs/core/runtime_local/CMakeLists.txt b/libs/core/runtime_local/CMakeLists.txt index 85865b6bd8db..31b45764a2be 100644 --- a/libs/core/runtime_local/CMakeLists.txt +++ b/libs/core/runtime_local/CMakeLists.txt @@ -109,5 +109,6 @@ add_hpx_module( hpx_threadmanager hpx_timing hpx_topology + hpx_type_support CMAKE_SUBDIRS examples tests ) diff --git a/libs/core/runtime_local/src/interval_timer.cpp b/libs/core/runtime_local/src/interval_timer.cpp index dc904e559e99..692873e1bb99 100644 --- a/libs/core/runtime_local/src/interval_timer.cpp +++ b/libs/core/runtime_local/src/interval_timer.cpp @@ -11,9 +11,9 @@ #include #include #include -#include #include #include +#include #include #include diff --git a/libs/core/schedulers/CMakeLists.txt b/libs/core/schedulers/CMakeLists.txt index 0476a76a8893..12f5fdea6f60 100644 --- a/libs/core/schedulers/CMakeLists.txt +++ b/libs/core/schedulers/CMakeLists.txt @@ -63,5 +63,6 @@ add_hpx_module( hpx_logging hpx_synchronization hpx_threading_base + hpx_type_support CMAKE_SUBDIRS examples tests ) diff --git a/libs/core/schedulers/include/hpx/schedulers/thread_queue.hpp b/libs/core/schedulers/include/hpx/schedulers/thread_queue.hpp index 57e4521109eb..f0092f4998a9 100644 --- a/libs/core/schedulers/include/hpx/schedulers/thread_queue.hpp +++ b/libs/core/schedulers/include/hpx/schedulers/thread_queue.hpp @@ -15,13 +15,13 @@ #include #include #include -#include #include #include #include #include #include #include +#include #if defined(HPX_HAVE_THREAD_MINIMAL_DEADLOCK_DETECTION) #include diff --git a/libs/core/synchronization/CMakeLists.txt b/libs/core/synchronization/CMakeLists.txt index fe5434333db6..741798a9a997 100644 --- a/libs/core/synchronization/CMakeLists.txt +++ b/libs/core/synchronization/CMakeLists.txt @@ -82,7 +82,8 @@ add_hpx_module( hpx_memory hpx_threading_base hpx_thread_support - hpx_topology hpx_timing + hpx_topology + hpx_type_support CMAKE_SUBDIRS examples tests ) diff --git a/libs/core/synchronization/include/hpx/synchronization/barrier.hpp b/libs/core/synchronization/include/hpx/synchronization/barrier.hpp index 52f29227cf35..15693424816c 100644 --- a/libs/core/synchronization/include/hpx/synchronization/barrier.hpp +++ b/libs/core/synchronization/include/hpx/synchronization/barrier.hpp @@ -1,4 +1,4 @@ -// Copyright (c) 2007-2023 Hartmut Kaiser +// Copyright (c) 2007-2024 Hartmut Kaiser // Copyright (c) 2016 Thomas Heller // // SPDX-License-Identifier: BSL-1.0 @@ -17,8 +17,8 @@ #include #include #include -#include #include +#include #include #include diff --git a/libs/core/synchronization/include/hpx/synchronization/condition_variable.hpp b/libs/core/synchronization/include/hpx/synchronization/condition_variable.hpp index 5c142ce03596..90699e91d9ca 100644 --- a/libs/core/synchronization/include/hpx/synchronization/condition_variable.hpp +++ b/libs/core/synchronization/include/hpx/synchronization/condition_variable.hpp @@ -1,4 +1,4 @@ -// Copyright (c) 2007-2023 Hartmut Kaiser +// Copyright (c) 2007-2024 Hartmut Kaiser // Copyright (c) 2022 Bhumit Attarde // Copyright (c) 2013 Agustin Berge // @@ -21,9 +21,9 @@ #include #include #include -#include #include #include +#include #include #include diff --git a/libs/core/synchronization/include/hpx/synchronization/shared_mutex.hpp b/libs/core/synchronization/include/hpx/synchronization/shared_mutex.hpp index b713dcacbc10..35c14cdaf2ca 100644 --- a/libs/core/synchronization/include/hpx/synchronization/shared_mutex.hpp +++ b/libs/core/synchronization/include/hpx/synchronization/shared_mutex.hpp @@ -1,6 +1,6 @@ // (C) Copyright 2006-2008 Anthony Williams // (C) Copyright 2011 Bryce Lelbach -// (C) Copyright 2022-2023 Hartmut Kaiser +// (C) Copyright 2022-2024 Hartmut Kaiser // // SPDX-License-Identifier: BSL-1.0 // Distributed under the Boost Software License, Version 1.0. (See accompanying @@ -18,7 +18,7 @@ #include #include #include -#include +#include #include #include @@ -71,6 +71,7 @@ namespace hpx::detail { [[maybe_unused]] util::ignore_while_checking il(&lk); exclusive_cond.notify_one_no_unlock(lk); shared_cond.notify_all(HPX_MOVE(lk)); + il.reset_owns_registration(); } bool set_state(shared_state& s1, shared_state& s) noexcept diff --git a/libs/core/synchronization/src/detail/condition_variable.cpp b/libs/core/synchronization/src/detail/condition_variable.cpp index a51a0543914f..56d2469d949a 100644 --- a/libs/core/synchronization/src/detail/condition_variable.cpp +++ b/libs/core/synchronization/src/detail/condition_variable.cpp @@ -1,4 +1,4 @@ -// Copyright (c) 2007-2023 Hartmut Kaiser +// Copyright (c) 2007-2024 Hartmut Kaiser // Copyright (c) 2013-2015 Agustin Berge // // SPDX-License-Identifier: BSL-1.0 @@ -14,10 +14,10 @@ #include #include #include -#include #include #include #include +#include #include #include diff --git a/libs/core/synchronization/src/detail/counting_semaphore.cpp b/libs/core/synchronization/src/detail/counting_semaphore.cpp index 6248663e9fb3..7eeb415c2788 100644 --- a/libs/core/synchronization/src/detail/counting_semaphore.cpp +++ b/libs/core/synchronization/src/detail/counting_semaphore.cpp @@ -1,4 +1,4 @@ -// Copyright (c) 2007-2022 Hartmut Kaiser +// Copyright (c) 2007-2024 Hartmut Kaiser // Copyright (c) 2011 Bryce Lelbach // // SPDX-License-Identifier: BSL-1.0 @@ -10,7 +10,7 @@ #include #include #include -#include +#include #include #include diff --git a/libs/core/synchronization/src/detail/sliding_semaphore.cpp b/libs/core/synchronization/src/detail/sliding_semaphore.cpp index ef9a5b6c2fa4..394a61b5da3f 100644 --- a/libs/core/synchronization/src/detail/sliding_semaphore.cpp +++ b/libs/core/synchronization/src/detail/sliding_semaphore.cpp @@ -1,4 +1,4 @@ -// Copyright (c) 2016-2023 Hartmut Kaiser +// Copyright (c) 2016-2024 Hartmut Kaiser // // SPDX-License-Identifier: BSL-1.0 // Distributed under the Boost Software License, Version 1.0. (See accompanying @@ -8,7 +8,7 @@ #include #include #include -#include +#include #include #include diff --git a/libs/core/synchronization/src/mutex.cpp b/libs/core/synchronization/src/mutex.cpp index 581fc4172706..dd238f8eb1c7 100644 --- a/libs/core/synchronization/src/mutex.cpp +++ b/libs/core/synchronization/src/mutex.cpp @@ -128,6 +128,7 @@ namespace hpx { #if defined(HPX_MSVC) #pragma warning(pop) #endif + il.reset_owns_registration(); } } diff --git a/libs/core/thread_support/CMakeLists.txt b/libs/core/thread_support/CMakeLists.txt index 03511da64195..fb994c4f634d 100644 --- a/libs/core/thread_support/CMakeLists.txt +++ b/libs/core/thread_support/CMakeLists.txt @@ -1,22 +1,18 @@ -# Copyright (c) 2019 The STE||AR-Group +# Copyright (c) 2019-2024 The STE||AR-Group # # SPDX-License-Identifier: BSL-1.0 # Distributed under the Boost Software License, Version 1.0. (See accompanying # file LICENSE_1_0.txt or copy at http://www.boost.org/LICENSE_1_0.txt) set(thread_support_headers - hpx/thread_support/assert_owns_lock.hpp - hpx/thread_support/atomic_count.hpp - hpx/thread_support/set_thread_name.hpp - hpx/thread_support/spinlock.hpp - hpx/thread_support/thread_specific_ptr.hpp + hpx/thread_support/atomic_count.hpp hpx/thread_support/set_thread_name.hpp + hpx/thread_support/spinlock.hpp hpx/thread_support/thread_specific_ptr.hpp hpx/thread_support/unlock_guard.hpp ) # cmake-format: off set(thread_support_compat_headers hpx/thread_support.hpp => hpx/modules/thread_support.hpp - hpx/util/assert_owns_lock.hpp => hpx/modules/thread_support.hpp hpx/util/atomic_count.hpp => hpx/modules/thread_support.hpp hpx/util/set_thread_name.hpp => hpx/modules/thread_support.hpp hpx/util/thread_specific_ptr.hpp => hpx/modules/thread_support.hpp @@ -33,6 +29,6 @@ add_hpx_module( SOURCES ${thread_support_sources} HEADERS ${thread_support_headers} COMPAT_HEADERS ${thread_support_compat_headers} - MODULE_DEPENDENCIES hpx_assertion hpx_config hpx_concepts hpx_type_support + MODULE_DEPENDENCIES hpx_assertion hpx_config hpx_concepts CMAKE_SUBDIRS examples tests ) diff --git a/libs/core/type_support/CMakeLists.txt b/libs/core/type_support/CMakeLists.txt index d3c9ef8c3e3a..5aacf92de760 100644 --- a/libs/core/type_support/CMakeLists.txt +++ b/libs/core/type_support/CMakeLists.txt @@ -1,4 +1,4 @@ -# Copyright (c) 2019-2023 The STE||AR-Group +# Copyright (c) 2019-2024 The STE||AR-Group # # SPDX-License-Identifier: BSL-1.0 # Distributed under the Boost Software License, Version 1.0. (See accompanying @@ -7,6 +7,7 @@ set(type_support_headers hpx/type_support/detail/with_result_of.hpp hpx/type_support/detail/wrap_int.hpp + hpx/type_support/assert_owns_lock.hpp hpx/type_support/bit_cast.hpp hpx/type_support/construct_at.hpp hpx/type_support/decay.hpp @@ -42,6 +43,7 @@ endif() set(type_support_compat_headers hpx/type_support.hpp => hpx/modules/type_support.hpp hpx/util/always_void.hpp => hpx/modules/type_support.hpp + hpx/util/assert_owns_lock.hpp => hpx/modules/type_support.hpp hpx/util/decay.hpp => hpx/modules/type_support.hpp hpx/util/detected.hpp => hpx/modules/type_support.hpp hpx/util/identity.hpp => hpx/modules/type_support.hpp diff --git a/libs/core/thread_support/include/hpx/thread_support/assert_owns_lock.hpp b/libs/core/type_support/include/hpx/type_support/assert_owns_lock.hpp similarity index 95% rename from libs/core/thread_support/include/hpx/thread_support/assert_owns_lock.hpp rename to libs/core/type_support/include/hpx/type_support/assert_owns_lock.hpp index 98f3756a2ac6..5c5f85f76c22 100644 --- a/libs/core/thread_support/include/hpx/thread_support/assert_owns_lock.hpp +++ b/libs/core/type_support/include/hpx/type_support/assert_owns_lock.hpp @@ -1,5 +1,5 @@ // Copyright (c) 2013 Agustin Berge -// Copyright (c) 2022 Hartmut Kaiser +// Copyright (c) 2022-2024 Hartmut Kaiser // // SPDX-License-Identifier: BSL-1.0 // Distributed under the Boost Software License, Version 1.0. (See accompanying @@ -10,7 +10,6 @@ #include #include #include -#include #include diff --git a/libs/full/agas/src/addressing_service.cpp b/libs/full/agas/src/addressing_service.cpp index bea6e6525b28..dd44cf57b7b1 100644 --- a/libs/full/agas/src/addressing_service.cpp +++ b/libs/full/agas/src/addressing_service.cpp @@ -33,8 +33,8 @@ #include #include #include -#include #include +#include #include #include diff --git a/libs/full/agas_base/src/server/primary_namespace_server.cpp b/libs/full/agas_base/src/server/primary_namespace_server.cpp index baa22de5ddad..e81cb0e84fbe 100644 --- a/libs/full/agas_base/src/server/primary_namespace_server.cpp +++ b/libs/full/agas_base/src/server/primary_namespace_server.cpp @@ -1,5 +1,5 @@ // Copyright (c) 2011 Bryce Adelstein-Lelbach -// Copyright (c) 2012-2023 Hartmut Kaiser +// Copyright (c) 2012-2024 Hartmut Kaiser // Copyright (c) 2016 Thomas Heller // // SPDX-License-Identifier: BSL-1.0 @@ -16,8 +16,8 @@ #include #include #include -#include #include +#include #include #include diff --git a/libs/full/collectives/CMakeLists.txt b/libs/full/collectives/CMakeLists.txt index 0c5910f9043d..c9921579badb 100644 --- a/libs/full/collectives/CMakeLists.txt +++ b/libs/full/collectives/CMakeLists.txt @@ -22,9 +22,11 @@ set(collectives_headers hpx/collectives/communication_set.hpp hpx/collectives/channel_communicator.hpp hpx/collectives/create_communicator.hpp + hpx/collectives/detail/barrier_node.hpp hpx/collectives/detail/channel_communicator.hpp hpx/collectives/detail/communication_set_node.hpp hpx/collectives/detail/communicator.hpp + hpx/collectives/detail/latch.hpp hpx/collectives/exclusive_scan.hpp hpx/collectives/fold.hpp hpx/collectives/gather.hpp diff --git a/libs/full/collectives/include/hpx/collectives/detail/communication_set_node.hpp b/libs/full/collectives/include/hpx/collectives/detail/communication_set_node.hpp index b343a5743858..e19b6b112361 100644 --- a/libs/full/collectives/include/hpx/collectives/detail/communication_set_node.hpp +++ b/libs/full/collectives/include/hpx/collectives/detail/communication_set_node.hpp @@ -1,4 +1,4 @@ -// Copyright (c) 2020 Hartmut Kaiser +// Copyright (c) 2020-2024 Hartmut Kaiser // // SPDX-License-Identifier: BSL-1.0 // Distributed under the Boost Software License, Version 1.0. (See accompanying @@ -17,7 +17,7 @@ #include #include #include -#include +#include #include #include diff --git a/libs/full/collectives/include/hpx/collectives/detail/communicator.hpp b/libs/full/collectives/include/hpx/collectives/detail/communicator.hpp index 844f91de9c67..2104d20fc2e3 100644 --- a/libs/full/collectives/include/hpx/collectives/detail/communicator.hpp +++ b/libs/full/collectives/include/hpx/collectives/detail/communicator.hpp @@ -16,9 +16,10 @@ #include #include #include +#include #include #include -#include +#include #include #include @@ -307,7 +308,7 @@ namespace hpx::collectives::detail { // re-acquired here (if `on_ready` happens to run on a new // thread asynchronously). std::unique_lock l(mtx_, std::try_to_lock); - [[maybe_unused]] util::ignore_while_checking il(&l); + //[[maybe_unused]] util::ignore_while_checking il(&l); // Verify that there is no overlap between different types of // operations on the same communicator. @@ -356,7 +357,7 @@ namespace hpx::collectives::detail { }; std::unique_lock l(mtx_); - [[maybe_unused]] util::ignore_while_checking il(&l); + [[maybe_unused]] util::ignore_all_while_checking il; // Verify that there is no overlap between different types of // operations on the same communicator. diff --git a/libs/full/collectives/tests/unit/concurrent_collectives.cpp b/libs/full/collectives/tests/unit/concurrent_collectives.cpp index bf0552f88c7a..7d7a3eeb8881 100644 --- a/libs/full/collectives/tests/unit/concurrent_collectives.cpp +++ b/libs/full/collectives/tests/unit/concurrent_collectives.cpp @@ -93,7 +93,10 @@ struct generations template void load(Archive& ar, unsigned) { + // clang-format off ar & data; + // clang-format on + for (auto& [k, v] : data) { v.current = 0; @@ -103,7 +106,9 @@ struct generations template void save(Archive& ar, unsigned) const { + // clang-format off ar & data; + // clang-format on } HPX_SERIALIZATION_SPLIT_MEMBER(); @@ -414,7 +419,8 @@ double test_local_all_reduce(std::vector const& comms) double elapsed = 0.; std::size_t gen = 0; - for (std::uint32_t i = 0; local.get_next_generation("all_reduce", gen); ++i) + for ([[maybe_unused]] std::uint32_t i = 0; + local.get_next_generation("all_reduce", gen); ++i) { std::vector> sites; sites.reserve(num_sites); @@ -458,7 +464,8 @@ double test_local_all_to_all(std::vector const& comms) double elapsed = 0.; std::size_t gen = 0; - for (std::uint32_t i = 0; local.get_next_generation("all_to_all", gen); ++i) + for ([[maybe_unused]] std::uint32_t i = 0; + local.get_next_generation("all_to_all", gen); ++i) { std::vector> sites; sites.reserve(num_sites); diff --git a/libs/full/parcelset/include/hpx/parcelset/parcelport_impl.hpp b/libs/full/parcelset/include/hpx/parcelset/parcelport_impl.hpp index 62b64d596241..8527e0e77509 100644 --- a/libs/full/parcelset/include/hpx/parcelset/parcelport_impl.hpp +++ b/libs/full/parcelset/include/hpx/parcelset/parcelport_impl.hpp @@ -315,7 +315,6 @@ namespace hpx::parcelset { { enqueue_parcels( dest, HPX_MOVE(parcels), HPX_MOVE(handlers)); - get_connection_and_send_parcels(dest); } }); @@ -712,9 +711,6 @@ namespace hpx::parcelset { std::unique_lock const l(mtx_); - // We ignore the lock here. It might happen that while enqueuing, - // we need to acquire a lock. This should not cause any problems - // (famous last words) [[maybe_unused]] util::ignore_while_checking il(&l); mapped_type& e = pending_parcels_[locality_id]; @@ -736,9 +732,6 @@ namespace hpx::parcelset { std::unique_lock const l(mtx_); - // We ignore the lock here. It might happen that while enqueuing, - // we need to acquire a lock. This should not cause any problems - // (famous last words) [[maybe_unused]] util::ignore_while_checking il(&l); HPX_ASSERT(parcels.size() == handlers.size()); From e0416f7428351bf9589a83a264fd31e58ae538d8 Mon Sep 17 00:00:00 2001 From: Hartmut Kaiser Date: Fri, 12 Apr 2024 08:52:36 -0500 Subject: [PATCH 8/9] Use MacOS 14 CI --- libs/core/lock_registration/src/register_locks.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libs/core/lock_registration/src/register_locks.cpp b/libs/core/lock_registration/src/register_locks.cpp index 5106165347ed..03a6e3a0ffcf 100644 --- a/libs/core/lock_registration/src/register_locks.cpp +++ b/libs/core/lock_registration/src/register_locks.cpp @@ -123,7 +123,7 @@ namespace hpx::util { static bool set_ignore_all_locks(bool enable) { - bool& val= get_held_locks().data_->ignore_all_locks_; + bool& val = get_held_locks().data_->ignore_all_locks_; if (val != enable) { val = enable; From ca8e62897f26d4e6c91c1f410c40d776a75fd642 Mon Sep 17 00:00:00 2001 From: Hartmut Kaiser Date: Sun, 14 Apr 2024 10:56:31 -0500 Subject: [PATCH 9/9] Split algorithm tests on CircleCI --- .circleci/config.yml | 62 +++++++++++++++++++------ .circleci/tests.unit1.algorithms | 76 ++++++++++++++++++++++++++++++ .circleci/tests.unit2.algorithms | 79 ++++++++++++++++++++++++++++++++ 3 files changed, 202 insertions(+), 15 deletions(-) create mode 100644 .circleci/tests.unit1.algorithms create mode 100644 .circleci/tests.unit2.algorithms diff --git a/.circleci/config.yml b/.circleci/config.yml index c42efad5aaa4..312201627ae5 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -490,18 +490,16 @@ jobs: paths: - ./build - tests.unit.algorithms: + tests.unit1.algorithms: <<: *defaults steps: - attach_workspace: at: /hpx - run: - name: Building Unit Tests (Algorithms) + name: Building Unit Tests (Algorithms, 1) command: | - ninja -j2 -k 0 \ - tests.unit.modules.algorithms.algorithms \ - tests.unit.modules.algorithms.block -# tests.unit.modules.algorithms.datapar_algorithms + ninja -j1 -k 0 \ + `grep -v -e ^# -e ^$ /hpx/source/.circleci/tests.unit1.algorithms` - run: name: Running Unit Tests when: always @@ -513,9 +511,7 @@ jobs: --no-compress-output \ --output-on-failure \ --tests-regex \ - "tests.unit.modules.algorithms.algorithms|\ - tests.unit.modules.algorithms.block" -# "|tests.unit.modules.algorithms.datapar_algorithms" + `grep -v -e ^# -e ^$ /hpx/source/.circleci/tests.unit1.algorithms | sed ':b;N;$!bb;s/\n/|/g'` - run: <<: *convert_xml - run: @@ -523,9 +519,42 @@ jobs: - run: <<: *move_debug_log - store_test_results: - path: tests.unit.algorithms + path: tests.unit1.algorithms - store_artifacts: - path: tests.unit.algorithms + path: tests.unit1.algorithms + + tests.unit2.algorithms: + <<: *defaults + steps: + - attach_workspace: + at: /hpx + - run: + name: Building Unit Tests (Algorithms, 2) + command: | + ninja -j1 -k 0 \ + `grep -v -e ^# -e ^$ /hpx/source/.circleci/tests.unit2.algorithms` + - run: + name: Running Unit Tests + when: always + command: | + ulimit -c unlimited + ctest \ + --timeout 120 \ + -T test \ + --no-compress-output \ + --output-on-failure \ + --tests-regex \ + `grep -v -e ^# -e ^$ /hpx/source/.circleci/tests.unit2.algorithms | sed ':b;N;$!bb;s/\n/|/g'` + - run: + <<: *convert_xml + - run: + <<: *move_core_dump + - run: + <<: *move_debug_log + - store_test_results: + path: tests.unit2.algorithms + - store_artifacts: + path: tests.unit2.algorithms tests.unit.container_algorithms: <<: *defaults @@ -595,7 +624,7 @@ jobs: - attach_workspace: at: /hpx - run: - name: Building Unit Tests + name: Building Unit Tests (1) command: | ninja -j2 -k 0 `grep -v -e ^# -e ^$ /hpx/source/.circleci/tests.unit1.targets` - run: @@ -626,7 +655,7 @@ jobs: - attach_workspace: at: /hpx - run: - name: Building Unit Tests + name: Building Unit Tests (2) command: | ninja -j2 -k 0 `grep -v -e ^# -e ^$ /hpx/source/.circleci/tests.unit2.targets` - run: @@ -912,7 +941,9 @@ workflows: <<: *gh_pages_filter - tests.examples: <<: *core_dependency - - tests.unit.algorithms: + - tests.unit1.algorithms: + <<: *core_dependency + - tests.unit2.algorithms: <<: *core_dependency - tests.unit.container_algorithms: <<: *core_dependency @@ -970,7 +1001,8 @@ workflows: requires: - core - tests.examples - - tests.unit.algorithms + - tests.unit1.algorithms + - tests.unit2.algorithms - tests.unit.container_algorithms - tests.unit.segmented_algorithms - tests.unit1 diff --git a/.circleci/tests.unit1.algorithms b/.circleci/tests.unit1.algorithms new file mode 100644 index 000000000000..d35e591525da --- /dev/null +++ b/.circleci/tests.unit1.algorithms @@ -0,0 +1,76 @@ +# Copyright (c) 2021-2024 Hartmut Kaiser +# +# SPDX-License-Identifier: BSL-1.0 +# Distributed under the Boost Software License, Version 1.0. (See accompanying +# file LICENSE_1_0.txt or copy at http://www.boost.org/LICENSE_1_0.txt) + +tests.unit.modules.algorithms.algorithms.adjacentdifference +tests.unit.modules.algorithms.algorithms.adjacentdifference_sender +tests.unit.modules.algorithms.algorithms.adjacentfind +tests.unit.modules.algorithms.algorithms.adjacentfind_binary +tests.unit.modules.algorithms.algorithms.all_of +tests.unit.modules.algorithms.algorithms.any_of +tests.unit.modules.algorithms.algorithms.copy +tests.unit.modules.algorithms.algorithms.copyif_random +tests.unit.modules.algorithms.algorithms.copyif_forward +tests.unit.modules.algorithms.algorithms.copyif_exception +tests.unit.modules.algorithms.algorithms.copyif_bad_alloc +tests.unit.modules.algorithms.algorithms.copyn +tests.unit.modules.algorithms.algorithms.count +tests.unit.modules.algorithms.algorithms.countif +tests.unit.modules.algorithms.algorithms.destroy +tests.unit.modules.algorithms.algorithms.destroyn +tests.unit.modules.algorithms.algorithms.ends_with +tests.unit.modules.algorithms.algorithms.equal +tests.unit.modules.algorithms.algorithms.equal_binary +tests.unit.modules.algorithms.algorithms.exclusive_scan +tests.unit.modules.algorithms.algorithms.exclusive_scan2 +tests.unit.modules.algorithms.algorithms.exclusive_scan_exception +tests.unit.modules.algorithms.algorithms.exclusive_scan_bad_alloc +tests.unit.modules.algorithms.algorithms.exclusive_scan_validate +tests.unit.modules.algorithms.algorithms.fill +tests.unit.modules.algorithms.algorithms.filln +tests.unit.modules.algorithms.algorithms.find +tests.unit.modules.algorithms.algorithms.find_sender +tests.unit.modules.algorithms.algorithms.findend +tests.unit.modules.algorithms.algorithms.findfirstof +tests.unit.modules.algorithms.algorithms.findfirstof_binary +tests.unit.modules.algorithms.algorithms.findif +tests.unit.modules.algorithms.algorithms.findifnot +tests.unit.modules.algorithms.algorithms.foreach +tests.unit.modules.algorithms.algorithms.foreach_executors +tests.unit.modules.algorithms.algorithms.foreach_prefetching +tests.unit.modules.algorithms.algorithms.foreach_sender +tests.unit.modules.algorithms.algorithms.foreach_scheduler +tests.unit.modules.algorithms.algorithms.foreachn +tests.unit.modules.algorithms.algorithms.foreachn_exception +tests.unit.modules.algorithms.algorithms.foreachn_bad_alloc +tests.unit.modules.algorithms.algorithms.for_loop +tests.unit.modules.algorithms.algorithms.for_loop_exception +tests.unit.modules.algorithms.algorithms.for_loop_induction +tests.unit.modules.algorithms.algorithms.for_loop_induction_async +tests.unit.modules.algorithms.algorithms.for_loop_n +tests.unit.modules.algorithms.algorithms.for_loop_n_strided +tests.unit.modules.algorithms.algorithms.for_loop_reduction +tests.unit.modules.algorithms.algorithms.for_loop_reduction_async +tests.unit.modules.algorithms.algorithms.for_loop_sender +tests.unit.modules.algorithms.algorithms.for_loop_strided +tests.unit.modules.algorithms.algorithms.generate +tests.unit.modules.algorithms.algorithms.generaten +tests.unit.modules.algorithms.algorithms.is_heap +tests.unit.modules.algorithms.algorithms.is_heap_until +tests.unit.modules.algorithms.algorithms.includes +tests.unit.modules.algorithms.algorithms.inclusive_scan +tests.unit.modules.algorithms.algorithms.inclusive_scan_exception +tests.unit.modules.algorithms.algorithms.inplace_merge +tests.unit.modules.algorithms.algorithms.is_partitioned +tests.unit.modules.algorithms.algorithms.is_sorted +tests.unit.modules.algorithms.algorithms.is_sorted_until +tests.unit.modules.algorithms.algorithms.lexicographical_compare +tests.unit.modules.algorithms.algorithms.make_heap +tests.unit.modules.algorithms.algorithms.max_element +tests.unit.modules.algorithms.algorithms.merge +tests.unit.modules.algorithms.algorithms.min_element +tests.unit.modules.algorithms.algorithms.minmax_element +tests.unit.modules.algorithms.algorithms.mismatch +tests.unit.modules.algorithms.algorithms.mismatch_binary diff --git a/.circleci/tests.unit2.algorithms b/.circleci/tests.unit2.algorithms new file mode 100644 index 000000000000..37f5d6020d6d --- /dev/null +++ b/.circleci/tests.unit2.algorithms @@ -0,0 +1,79 @@ +# Copyright (c) 2021-2024 Hartmut Kaiser +# +# SPDX-License-Identifier: BSL-1.0 +# Distributed under the Boost Software License, Version 1.0. (See accompanying +# file LICENSE_1_0.txt or copy at http://www.boost.org/LICENSE_1_0.txt) + +tests.unit.modules.algorithms.algorithms.move +tests.unit.modules.algorithms.algorithms.nth_element +tests.unit.modules.algorithms.algorithms.none_of +tests.unit.modules.algorithms.algorithms.parallel_sort +tests.unit.modules.algorithms.algorithms.partial_sort +tests.unit.modules.algorithms.algorithms.partial_sort_copy +tests.unit.modules.algorithms.algorithms.partition +tests.unit.modules.algorithms.algorithms.partition_copy +tests.unit.modules.algorithms.algorithms.reduce_ +tests.unit.modules.algorithms.algorithms.reduce_by_key +tests.unit.modules.algorithms.algorithms.remove +tests.unit.modules.algorithms.algorithms.remove +tests.unit.modules.algorithms.algorithms.remove1 +tests.unit.modules.algorithms.algorithms.remove2 +tests.unit.modules.algorithms.algorithms.remove_if +tests.unit.modules.algorithms.algorithms.remove_if1 +tests.unit.modules.algorithms.algorithms.remove_copy +tests.unit.modules.algorithms.algorithms.remove_copy_if +tests.unit.modules.algorithms.algorithms.replace +tests.unit.modules.algorithms.algorithms.replace_if +tests.unit.modules.algorithms.algorithms.replace_copy +tests.unit.modules.algorithms.algorithms.replace_copy_if +tests.unit.modules.algorithms.algorithms.reverse +tests.unit.modules.algorithms.algorithms.reverse_copy +tests.unit.modules.algorithms.algorithms.reverse_sender +tests.unit.modules.algorithms.algorithms.rotate +tests.unit.modules.algorithms.algorithms.rotate_copy +tests.unit.modules.algorithms.algorithms.rotate_sender +tests.unit.modules.algorithms.algorithms.search +tests.unit.modules.algorithms.algorithms.searchn +tests.unit.modules.algorithms.algorithms.set_difference +tests.unit.modules.algorithms.algorithms.set_intersection +tests.unit.modules.algorithms.algorithms.set_symmetric_difference +tests.unit.modules.algorithms.algorithms.set_union +tests.unit.modules.algorithms.algorithms.shift_left +tests.unit.modules.algorithms.algorithms.shift_right +tests.unit.modules.algorithms.algorithms.sort +tests.unit.modules.algorithms.algorithms.sort_by_key +tests.unit.modules.algorithms.algorithms.sort_exceptions +tests.unit.modules.algorithms.algorithms.stable_partition +tests.unit.modules.algorithms.algorithms.stable_sort +tests.unit.modules.algorithms.algorithms.stable_sort_exceptions +tests.unit.modules.algorithms.algorithms.starts_with +tests.unit.modules.algorithms.algorithms.swapranges +tests.unit.modules.algorithms.algorithms.transform +tests.unit.modules.algorithms.algorithms.transform_binary +tests.unit.modules.algorithms.algorithms.transform_binary2 +tests.unit.modules.algorithms.algorithms.transform_exclusive_scan +tests.unit.modules.algorithms.algorithms.transform_inclusive_scan +tests.unit.modules.algorithms.algorithms.transform_reduce +tests.unit.modules.algorithms.algorithms.transform_reduce_binary +tests.unit.modules.algorithms.algorithms.transform_reduce_binary_exception +tests.unit.modules.algorithms.algorithms.transform_reduce_binary_bad_alloc +tests.unit.modules.algorithms.algorithms.uninitialized_copy +tests.unit.modules.algorithms.algorithms.uninitialized_copyn +tests.unit.modules.algorithms.algorithms.uninitialized_default_construct +tests.unit.modules.algorithms.algorithms.uninitialized_default_constructn +tests.unit.modules.algorithms.algorithms.uninitialized_fill +tests.unit.modules.algorithms.algorithms.uninitialized_filln +tests.unit.modules.algorithms.algorithms.uninitialized_move +tests.unit.modules.algorithms.algorithms.uninitialized_moven +tests.unit.modules.algorithms.algorithms.uninitialized_relocate_backward +tests.unit.modules.algorithms.algorithms.uninitialized_relocate +tests.unit.modules.algorithms.algorithms.uninitialized_relocaten +tests.unit.modules.algorithms.algorithms.uninitialized_value_construct +tests.unit.modules.algorithms.algorithms.uninitialized_value_constructn +tests.unit.modules.algorithms.algorithms.unique +tests.unit.modules.algorithms.algorithms.unique_copy +tests.unit.modules.algorithms.block.spmd_block +tests.unit.modules.algorithms.block.task_block +tests.unit.modules.algorithms.block.task_block_executor +tests.unit.modules.algorithms.block.task_block_par +tests.unit.modules.algorithms.block.task_group