diff --git a/doc/unordered/changes.adoc b/doc/unordered/changes.adoc index 550a7068..8e317b07 100644 --- a/doc/unordered/changes.adoc +++ b/doc/unordered/changes.adoc @@ -8,6 +8,9 @@ == Release 1.87.0 +* Made visitation exclusive-locked within certain +`boost::concurrent_flat_set` operations to allow for safe mutable modification of elements +({github-pr-url}/265[PR#265^]). * In Visual Studio Natvis, supported any container with an allocator that uses fancy pointers. This applies to any fancy pointer type, as long as the proper Natvis customization point "Intrinsic" functions are written for the fancy pointer type. == Release 1.86.0 diff --git a/doc/unordered/concurrent_flat_map.adoc b/doc/unordered/concurrent_flat_map.adoc index 50c840a9..9b11ec12 100644 --- a/doc/unordered/concurrent_flat_map.adoc +++ b/doc/unordered/concurrent_flat_map.adoc @@ -389,9 +389,13 @@ prior blocking operations on `x` synchronize with *op*. So, blocking operations An operation is said to be _blocking on rehashing of_ ``__x__`` if it blocks on `x` only when an internal rehashing is issued. -Access or modification of an element of a `boost::concurrent_flat_map` passed by reference to a -user-provided visitation function do not introduce data races when the visitation function -is executed internally by the `boost::concurrent_flat_map`. +When executed internally by a `boost::concurrent_flat_map`, the following operations by a +user-provided visitation function on the element passed do not introduce data races: + +* Read access to the element. +* Non-mutable modification of the element. +* Mutable modification of the element (if the container operation executing the visitation function is not const +and its name does not contain `cvisit`.) Any `boost::concurrent_flat_map operation` that inserts or modifies an element `e` synchronizes with the internal invocation of a visitation function on `e`. diff --git a/doc/unordered/concurrent_flat_set.adoc b/doc/unordered/concurrent_flat_set.adoc index c52725cc..83327a8f 100644 --- a/doc/unordered/concurrent_flat_set.adoc +++ b/doc/unordered/concurrent_flat_set.adoc @@ -98,25 +98,35 @@ namespace boost { // visitation + template size_t xref:#concurrent_flat_set_cvisit[visit](const key_type& k, F f); template size_t xref:#concurrent_flat_set_cvisit[visit](const key_type& k, F f) const; template size_t xref:#concurrent_flat_set_cvisit[cvisit](const key_type& k, F f) const; + template size_t xref:#concurrent_flat_set_cvisit[visit](const K& k, F f); template size_t xref:#concurrent_flat_set_cvisit[visit](const K& k, F f) const; template size_t xref:#concurrent_flat_set_cvisit[cvisit](const K& k, F f) const; + template + size_t xref:concurrent_flat_set_bulk_visit[visit](FwdIterator first, FwdIterator last, F f); template size_t xref:concurrent_flat_set_bulk_visit[visit](FwdIterator first, FwdIterator last, F f) const; template size_t xref:concurrent_flat_set_bulk_visit[cvisit](FwdIterator first, FwdIterator last, F f) const; + template size_t xref:#concurrent_flat_set_cvisit_all[visit_all](F f); template size_t xref:#concurrent_flat_set_cvisit_all[visit_all](F f) const; template size_t xref:#concurrent_flat_set_cvisit_all[cvisit_all](F f) const; + template + void xref:#concurrent_flat_set_parallel_cvisit_all[visit_all](ExecutionPolicy&& policy, F f); template void xref:#concurrent_flat_set_parallel_cvisit_all[visit_all](ExecutionPolicy&& policy, F f) const; template void xref:#concurrent_flat_set_parallel_cvisit_all[cvisit_all](ExecutionPolicy&& policy, F f) const; + template bool xref:#concurrent_flat_set_cvisit_while[visit_while](F f); template bool xref:#concurrent_flat_set_cvisit_while[visit_while](F f) const; template bool xref:#concurrent_flat_set_cvisit_while[cvisit_while](F f) const; + template + bool xref:#concurrent_flat_set_parallel_cvisit_while[visit_while](ExecutionPolicy&& policy, F f); template bool xref:#concurrent_flat_set_parallel_cvisit_while[visit_while](ExecutionPolicy&& policy, F f) const; template @@ -340,9 +350,13 @@ prior blocking operations on `x` synchronize with *op*. So, blocking operations An operation is said to be _blocking on rehashing of_ ``__x__`` if it blocks on `x` only when an internal rehashing is issued. -Access or modification of an element of a `boost::concurrent_flat_set` passed by reference to a -user-provided visitation function do not introduce data races when the visitation function -is executed internally by the `boost::concurrent_flat_set`. +When executed internally by a `boost::concurrent_flat_set`, the following operations by a +user-provided visitation function on the element passed do not introduce data races: + +* Read access to the element. +* Non-mutable modification of the element. +* Mutable modification of the element (if the container operation executing the visitation function is not const +and its name does not contain `cvisit`.) Any `boost::concurrent_flat_set operation` that inserts or modifies an element `e` synchronizes with the internal invocation of a visitation function on `e`. @@ -708,8 +722,10 @@ Concurrency:;; Blocking on `*this`. ==== [c]visit ```c++ +template size_t visit(const key_type& k, F f); template size_t visit(const key_type& k, F f) const; template size_t cvisit(const key_type& k, F f) const; +template size_t visit(const K& k, F f); template size_t visit(const K& k, F f) const; template size_t cvisit(const K& k, F f) const; ``` @@ -725,6 +741,8 @@ Notes:;; The `template` overloads only participate in overload ==== Bulk visit ```c++ +template + size_t visit(FwdIterator first, FwdIterator last, F f); template size_t visit(FwdIterator first, FwdIterator last, F f) const; template @@ -758,6 +776,7 @@ Returns:;; The number of elements visited. ==== [c]visit_all ```c++ +template size_t visit_all(F f); template size_t visit_all(F f) const; template size_t cvisit_all(F f) const; ``` @@ -772,6 +791,7 @@ Returns:;; The number of elements visited. ==== Parallel [c]visit_all ```c++ +template void visit_all(ExecutionPolicy&& policy, F f); template void visit_all(ExecutionPolicy&& policy, F f) const; template void cvisit_all(ExecutionPolicy&& policy, F f) const; ``` @@ -792,6 +812,7 @@ Unsequenced execution policies are not allowed. ==== [c]visit_while ```c++ +template bool visit_while(F f); template bool visit_while(F f) const; template bool cvisit_while(F f) const; ``` @@ -807,6 +828,7 @@ Returns:;; `false` iff `f` ever returns `false`. ==== Parallel [c]visit_while ```c++ +template bool visit_while(ExecutionPolicy&& policy, F f); template bool visit_while(ExecutionPolicy&& policy, F f) const; template bool cvisit_while(ExecutionPolicy&& policy, F f) const; ``` diff --git a/include/boost/unordered/concurrent_flat_set.hpp b/include/boost/unordered/concurrent_flat_set.hpp index d0665f71..7977f91c 100644 --- a/include/boost/unordered/concurrent_flat_set.hpp +++ b/include/boost/unordered/concurrent_flat_set.hpp @@ -227,6 +227,13 @@ namespace boost { return size() == 0; } + template + BOOST_FORCEINLINE size_type visit(key_type const& k, F f) + { + BOOST_UNORDERED_STATIC_ASSERT_CONST_INVOCABLE(F) + return table_.visit(k, f); + } + template BOOST_FORCEINLINE size_type visit(key_type const& k, F f) const { @@ -241,6 +248,15 @@ namespace boost { return table_.visit(k, f); } + template + BOOST_FORCEINLINE typename std::enable_if< + detail::are_transparent::value, size_type>::type + visit(K&& k, F f) + { + BOOST_UNORDERED_STATIC_ASSERT_CONST_INVOCABLE(F) + return table_.visit(std::forward(k), f); + } + template BOOST_FORCEINLINE typename std::enable_if< detail::are_transparent::value, size_type>::type @@ -259,6 +275,15 @@ namespace boost { return table_.visit(std::forward(k), f); } + template + BOOST_FORCEINLINE + size_t visit(FwdIterator first, FwdIterator last, F f) + { + BOOST_UNORDERED_STATIC_ASSERT_BULK_VISIT_ITERATOR(FwdIterator) + BOOST_UNORDERED_STATIC_ASSERT_CONST_INVOCABLE(F) + return table_.visit(first, last, f); + } + template BOOST_FORCEINLINE size_t visit(FwdIterator first, FwdIterator last, F f) const @@ -277,6 +302,12 @@ namespace boost { return table_.visit(first, last, f); } + template size_type visit_all(F f) + { + BOOST_UNORDERED_STATIC_ASSERT_CONST_INVOCABLE(F) + return table_.visit_all(f); + } + template size_type visit_all(F f) const { BOOST_UNORDERED_STATIC_ASSERT_CONST_INVOCABLE(F) @@ -290,6 +321,16 @@ namespace boost { } #if defined(BOOST_UNORDERED_PARALLEL_ALGORITHMS) + template + typename std::enable_if::value, + void>::type + visit_all(ExecPolicy&& p, F f) + { + BOOST_UNORDERED_STATIC_ASSERT_CONST_INVOCABLE(F) + BOOST_UNORDERED_STATIC_ASSERT_EXEC_POLICY(ExecPolicy) + table_.visit_all(p, f); + } + template typename std::enable_if::value, void>::type @@ -311,6 +352,12 @@ namespace boost { } #endif + template bool visit_while(F f) + { + BOOST_UNORDERED_STATIC_ASSERT_CONST_INVOCABLE(F) + return table_.visit_while(f); + } + template bool visit_while(F f) const { BOOST_UNORDERED_STATIC_ASSERT_CONST_INVOCABLE(F) @@ -324,6 +371,16 @@ namespace boost { } #if defined(BOOST_UNORDERED_PARALLEL_ALGORITHMS) + template + typename std::enable_if::value, + bool>::type + visit_while(ExecPolicy&& p, F f) + { + BOOST_UNORDERED_STATIC_ASSERT_CONST_INVOCABLE(F) + BOOST_UNORDERED_STATIC_ASSERT_EXEC_POLICY(ExecPolicy) + return table_.visit_while(p, f); + } + template typename std::enable_if::value, bool>::type @@ -384,14 +441,14 @@ namespace boost { BOOST_FORCEINLINE bool insert_or_visit(value_type const& obj, F f) { BOOST_UNORDERED_STATIC_ASSERT_CONST_INVOCABLE(F) - return table_.insert_or_cvisit(obj, f); + return table_.insert_or_visit(obj, f); } template BOOST_FORCEINLINE bool insert_or_visit(value_type&& obj, F f) { BOOST_UNORDERED_STATIC_ASSERT_CONST_INVOCABLE(F) - return table_.insert_or_cvisit(std::move(obj), f); + return table_.insert_or_visit(std::move(obj), f); } template @@ -401,7 +458,7 @@ namespace boost { insert_or_visit(K&& k, F f) { BOOST_UNORDERED_STATIC_ASSERT_CONST_INVOCABLE(F) - return table_.try_emplace_or_cvisit(std::forward(k), f); + return table_.try_emplace_or_visit(std::forward(k), f); } template @@ -409,7 +466,7 @@ namespace boost { { BOOST_UNORDERED_STATIC_ASSERT_CONST_INVOCABLE(F) for (; first != last; ++first) { - table_.emplace_or_cvisit(*first, f); + table_.emplace_or_visit(*first, f); } } @@ -417,7 +474,7 @@ namespace boost { void insert_or_visit(std::initializer_list ilist, F f) { BOOST_UNORDERED_STATIC_ASSERT_CONST_INVOCABLE(F) - this->insert_or_cvisit(ilist.begin(), ilist.end(), f); + this->insert_or_visit(ilist.begin(), ilist.end(), f); } template @@ -469,7 +526,7 @@ namespace boost { BOOST_FORCEINLINE bool emplace_or_visit(Arg&& arg, Args&&... args) { BOOST_UNORDERED_STATIC_ASSERT_LAST_ARG_CONST_INVOCABLE(Arg, Args...) - return table_.emplace_or_cvisit( + return table_.emplace_or_visit( std::forward(arg), std::forward(args)...); } diff --git a/test/cfoa/visit_tests.cpp b/test/cfoa/visit_tests.cpp index 61452ef7..02bcea32 100644 --- a/test/cfoa/visit_tests.cpp +++ b/test/cfoa/visit_tests.cpp @@ -1,5 +1,5 @@ // Copyright (C) 2023 Christian Mazakas -// Copyright (C) 2023 Joaquin M Lopez Munoz +// Copyright (C) 2023-2024 Joaquin M Lopez Munoz // 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) @@ -18,6 +18,7 @@ #include #include +#include #include #include @@ -27,7 +28,10 @@ #include #include +#include #include +#include +#include #include namespace { @@ -971,6 +975,147 @@ namespace { boost::unordered::concurrent_flat_set* transp_set; + struct mutable_pair + { + mutable_pair(int first_ = 0, int second_ = 0): + first{first_}, second{second_} {} + + int first = 0; + mutable int second = 0; + }; + + struct null_mutable_pair + { + operator mutable_pair() const { return {0,0}; } + }; + + struct mutable_pair_hash + { + using is_transparent = void; + + std::size_t operator()(const mutable_pair& x) const + { + return boost::hash{}(x.first); + } + + std::size_t operator()(const null_mutable_pair&) const + { + return boost::hash{}(0); + } + }; + + struct mutable_pair_equal_to + { + using is_transparent = void; + + bool operator()(const mutable_pair& x, const mutable_pair& y) const + { + return x.first == y.first; + } + + bool operator()(const null_mutable_pair&, const mutable_pair& y) const + { + return 0 == y.first; + } + + bool operator()(const mutable_pair& x, const null_mutable_pair&) const + { + return x.first == 0; + } + }; + + template + void exclusive_access_for(F f) + { + std::atomic_int num_started{0}; + std::atomic_int in_visit{0}; + boost::compat::latch finish{1}; + + auto bound_f = [&] { + ++num_started; + f([&] (const mutable_pair& x) { + ++in_visit; + ++x.second; + finish.wait(); + --in_visit; + return true; + }); + }; + + std::thread t1{bound_f}, t2{bound_f}; + while(num_started != 2) { + std::this_thread::sleep_for(std::chrono::milliseconds(100)); + } + BOOST_TEST(in_visit <= 1); + finish.count_down(); + t1.join(); + t2.join(); + } + + template + void exclusive_access_set_visit(X*) + { + using visit_function = std::function; + using returning_visit_function = std::function; + X x; + x.insert({0, 0}); + + exclusive_access_for([&](visit_function f) { + x.visit({0, 0}, f); + }); + exclusive_access_for([&](visit_function f) { + x.visit(null_mutable_pair{}, f); + }); + + exclusive_access_for([&](visit_function f) { + mutable_pair a[] = {{0, 0}}; + x.visit(std::begin(a), std::end(a), f); + }); + + exclusive_access_for([&](visit_function f) { + x.visit_all(f); + }); + exclusive_access_for([&](returning_visit_function f) { + x.visit_while(f); + }); + + #if defined(BOOST_UNORDERED_PARALLEL_ALGORITHMS) + exclusive_access_for([&](visit_function f) { + x.visit_all(std::execution::par, f); + }); + exclusive_access_for([&](returning_visit_function f) { + x.visit_while(std::execution::par, f); + }); + #endif + + exclusive_access_for([&](visit_function f) { + const mutable_pair p; + x.insert_or_visit(p, f); + }); + exclusive_access_for([&](visit_function f) { + x.insert_or_visit({0,0}, f); + }); + exclusive_access_for([&](visit_function f) { + x.insert_or_visit(null_mutable_pair{}, f); + }); + + exclusive_access_for([&](visit_function f) { + mutable_pair a[] = {{0, 0}}; + x.insert_or_visit(std::begin(a), std::end(a), f); + }); + exclusive_access_for([&](visit_function f) { + std::initializer_list il = {{0, 0}}; + x.insert_or_visit(il, f); + }); + + exclusive_access_for([&](visit_function f) { + x.emplace_or_visit(0, 0, f); + }); + } + + boost::concurrent_flat_set< + mutable_pair, mutable_pair_hash, mutable_pair_equal_to>* mutable_set; + } // namespace using test::default_generator; @@ -1024,6 +1169,13 @@ UNORDERED_TEST( ((sequential)) ) +// https://github.com/boostorg/unordered/issues/260 + +UNORDERED_TEST( + exclusive_access_set_visit, + ((mutable_set)) +) + // clang-format on RUN_TESTS()