From 67c5cdb3a69f0b92d2779880ce9aa1d46e54cf7b Mon Sep 17 00:00:00 2001 From: Braden Ganetsky Date: Sun, 31 Dec 2023 11:56:14 -0600 Subject: [PATCH] Optimize `emplace()` for exactly a `value_type` or `init_type` argument (#227) * Add structs to count special member functions * Add failing emplace tests, that will pass after making the optimization * Optimize emplace() to not allocate when we already have a value_type or init_type * Fix newly failing cfoa tests --- .../unordered/detail/foa/concurrent_table.hpp | 9 + include/boost/unordered/detail/foa/table.hpp | 11 ++ .../boost/unordered/detail/type_traits.hpp | 17 ++ test/Jamfile.v2 | 2 + test/cfoa/assign_tests.cpp | 34 ++-- test/cfoa/constructor_tests.cpp | 27 +-- test/cfoa/emplace_tests.cpp | 70 ++++++- test/cfoa/exception_helpers.hpp | 2 +- test/helpers/count.hpp | 108 ++++++++++- test/unordered/emplace_smf_tests.cpp | 175 ++++++++++++++++++ 10 files changed, 418 insertions(+), 37 deletions(-) create mode 100644 test/unordered/emplace_smf_tests.cpp diff --git a/include/boost/unordered/detail/foa/concurrent_table.hpp b/include/boost/unordered/detail/foa/concurrent_table.hpp index e93784a6..5c6de985 100644 --- a/include/boost/unordered/detail/foa/concurrent_table.hpp +++ b/include/boost/unordered/detail/foa/concurrent_table.hpp @@ -28,6 +28,7 @@ #include #include #include +#include #include #include #include @@ -682,6 +683,14 @@ public: return construct_and_emplace(std::forward(args)...); } + /* Optimization for value_type and init_type, to avoid constructing twice */ + template + BOOST_FORCEINLINE auto emplace(Value&& x)->typename std::enable_if< + detail::is_similar_to_any::value,bool>::type + { + return emplace_impl(std::forward(x)); + } + BOOST_FORCEINLINE bool insert(const init_type& x){return emplace_impl(x);} diff --git a/include/boost/unordered/detail/foa/table.hpp b/include/boost/unordered/detail/foa/table.hpp index 7e14bc75..4d248c4a 100644 --- a/include/boost/unordered/detail/foa/table.hpp +++ b/include/boost/unordered/detail/foa/table.hpp @@ -18,6 +18,7 @@ #include #include #include +#include #include #include #include @@ -405,6 +406,16 @@ public: return emplace_impl(type_policy::move(x.value())); } + /* Optimization for value_type and init_type, to avoid constructing twice */ + template + BOOST_FORCEINLINE typename std::enable_if< + detail::is_similar_to_any::value, + std::pair >::type + emplace(T&& x) + { + return emplace_impl(std::forward(x)); + } + template BOOST_FORCEINLINE std::pair try_emplace( Key&& x,Args&&... args) diff --git a/include/boost/unordered/detail/type_traits.hpp b/include/boost/unordered/detail/type_traits.hpp index dc7ebafe..0ae5656f 100644 --- a/include/boost/unordered/detail/type_traits.hpp +++ b/include/boost/unordered/detail/type_traits.hpp @@ -147,6 +147,23 @@ namespace boost { !std::is_convertible::value; }; + template + using remove_cvref_t = + typename std::remove_cv::type>::type; + + template + using is_similar = std::is_same, remove_cvref_t >; + + template struct is_similar_to_any : std::false_type + { + }; + template + struct is_similar_to_any + : std::conditional::value, is_similar, + is_similar_to_any >::type + { + }; + #if BOOST_UNORDERED_TEMPLATE_DEDUCTION_GUIDES // https://eel.is/c++draft/container.requirements#container.alloc.reqmts-34 // https://eel.is/c++draft/container.requirements#unord.req.general-243 diff --git a/test/Jamfile.v2 b/test/Jamfile.v2 index 09d62122..f1ab8d81 100644 --- a/test/Jamfile.v2 +++ b/test/Jamfile.v2 @@ -85,6 +85,7 @@ local FCA_TESTS = contains_tests copy_tests deduction_tests + emplace_smf_tests emplace_tests equality_tests equivalent_keys_tests @@ -201,6 +202,7 @@ local FOA_TESTS = assign_tests insert_tests insert_hint_tests + emplace_smf_tests emplace_tests erase_tests merge_tests diff --git a/test/cfoa/assign_tests.cpp b/test/cfoa/assign_tests.cpp index 276c47b2..f201e9da 100644 --- a/test/cfoa/assign_tests.cpp +++ b/test/cfoa/assign_tests.cpp @@ -968,11 +968,9 @@ namespace { BOOST_TEST_EQ(x.key_eq(), key_equal(2)); BOOST_TEST_EQ( - raii::copy_constructor, value_type_cardinality * values.size()); - BOOST_TEST_EQ( - raii::destructor, value_type_cardinality * values.size()); - BOOST_TEST_EQ( - raii::move_constructor, value_type_cardinality * reference_cont.size()); + raii::copy_constructor, value_type_cardinality * reference_cont.size()); + BOOST_TEST_EQ(raii::destructor, 0u); + BOOST_TEST_EQ(raii::move_constructor, 0u); BOOST_TEST_EQ(raii::copy_assignment, 0u); BOOST_TEST_EQ(raii::move_assignment, 0u); } @@ -998,11 +996,9 @@ namespace { BOOST_TEST_EQ(flat.key_eq(), key_equal(2)); BOOST_TEST_EQ( - raii::copy_constructor, value_type_cardinality * values.size()); - BOOST_TEST_EQ( - raii::destructor, value_type_cardinality * values.size()); - BOOST_TEST_EQ( - raii::move_constructor, value_type_cardinality * reference_cont.size()); + raii::copy_constructor, value_type_cardinality * reference_cont.size()); + BOOST_TEST_EQ(raii::destructor, 0u); + BOOST_TEST_EQ(raii::move_constructor, 0u); BOOST_TEST_EQ(raii::copy_assignment, 0u); BOOST_TEST_EQ(raii::move_assignment, 0u); } @@ -1030,14 +1026,11 @@ namespace { BOOST_TEST_EQ(x.key_eq(), key_equal(2)); BOOST_TEST_EQ( - raii::copy_constructor, value_type_cardinality * values.size()); + raii::copy_constructor, value_type_cardinality * reference_cont.size()); BOOST_TEST_EQ( - raii::destructor, - value_type_cardinality * values.size() + - value_type_cardinality * reference_cont.size()); + raii::destructor, value_type_cardinality * reference_cont.size()); BOOST_TEST_EQ( - raii::move_constructor, - 2 * value_type_cardinality * reference_cont.size()); + raii::move_constructor, value_type_cardinality * reference_cont.size()); BOOST_TEST_EQ(raii::copy_assignment, 0u); BOOST_TEST_EQ(raii::move_assignment, 0u); } @@ -1063,14 +1056,11 @@ namespace { BOOST_TEST_EQ(flat.key_eq(), key_equal(2)); BOOST_TEST_EQ( - raii::copy_constructor, value_type_cardinality * values.size()); + raii::copy_constructor, value_type_cardinality * reference_cont.size()); BOOST_TEST_EQ( - raii::destructor, - value_type_cardinality * values.size() + - value_type_cardinality * reference_cont.size()); + raii::destructor, value_type_cardinality * reference_cont.size()); BOOST_TEST_EQ( - raii::move_constructor, - 2 * value_type_cardinality * reference_cont.size()); + raii::move_constructor, value_type_cardinality * reference_cont.size()); BOOST_TEST_EQ(raii::copy_assignment, 0u); BOOST_TEST_EQ(raii::move_assignment, 0u); } diff --git a/test/cfoa/constructor_tests.cpp b/test/cfoa/constructor_tests.cpp index 4b3f927d..aa2d0caf 100644 --- a/test/cfoa/constructor_tests.cpp +++ b/test/cfoa/constructor_tests.cpp @@ -712,9 +712,9 @@ namespace { BOOST_TEST_EQ(raii::default_constructor, 0u); BOOST_TEST_EQ( - raii::copy_constructor, value_type_cardinality * init_list.size()); + raii::copy_constructor, value_type_cardinality * init_list.size() / 2u); BOOST_TEST_EQ( - raii::move_constructor, value_type_cardinality * 11u); + raii::move_constructor, 0u); } check_raii_counts(); @@ -730,9 +730,9 @@ namespace { BOOST_TEST_EQ(raii::default_constructor, 0u); BOOST_TEST_EQ( - raii::copy_constructor, value_type_cardinality * init_list.size()); + raii::copy_constructor, value_type_cardinality * init_list.size() / 2u); BOOST_TEST_EQ( - raii::move_constructor, value_type_cardinality * 11u); + raii::move_constructor, 0u); } check_raii_counts(); @@ -748,9 +748,9 @@ namespace { BOOST_TEST_EQ(raii::default_constructor, 0u); BOOST_TEST_EQ( - raii::copy_constructor, value_type_cardinality * init_list.size()); + raii::copy_constructor, value_type_cardinality * init_list.size() / 2u); BOOST_TEST_EQ( - raii::move_constructor, value_type_cardinality * 11u); + raii::move_constructor, 0u); } check_raii_counts(); @@ -766,9 +766,9 @@ namespace { BOOST_TEST_EQ(raii::default_constructor, 0u); BOOST_TEST_EQ( - raii::copy_constructor, value_type_cardinality * init_list.size()); + raii::copy_constructor, value_type_cardinality * init_list.size() / 2u); BOOST_TEST_EQ( - raii::move_constructor, value_type_cardinality * 11u); + raii::move_constructor, 0u); } check_raii_counts(); } @@ -867,6 +867,9 @@ namespace { template void flat_constructor(X*, GF gen_factory, test::random_generator rg) { + using value_type = typename X::value_type; + static constexpr auto value_type_cardinality = + value_cardinality::value; using allocator_type = typename X::allocator_type; auto gen = gen_factory.template get(); @@ -886,8 +889,8 @@ namespace { auto const old_cc = +raii::copy_constructor; BOOST_TEST_EQ(old_dc, 0u); - BOOST_TEST_GT(old_mc, 0u); - BOOST_TEST_GT(old_cc, 0u); + BOOST_TEST_EQ(old_mc, 0u); + BOOST_TEST_EQ(old_cc, value_type_cardinality * flat.size()); X x(std::move(flat)); @@ -931,8 +934,8 @@ namespace { auto const old_cc = +raii::copy_constructor; BOOST_TEST_EQ(old_dc, 0u); - BOOST_TEST_GT(old_mc, 0u); - BOOST_TEST_GT(old_cc, 0u); + BOOST_TEST_EQ(old_mc, 0u); + BOOST_TEST_EQ(old_cc, 2u * value_type_cardinality * x.size()); flat_container flat(std::move(x)); diff --git a/test/cfoa/emplace_tests.cpp b/test/cfoa/emplace_tests.cpp index ec2e1bc0..f7105e6c 100644 --- a/test/cfoa/emplace_tests.cpp +++ b/test/cfoa/emplace_tests.cpp @@ -170,6 +170,74 @@ namespace { } } lvalue_emplace_or_visit; + struct copy_emplacer_type + { + template void operator()(std::vector& values, X& x) + { + static constexpr auto value_type_cardinality = + value_cardinality::value; + + std::atomic num_inserts{0}; + thread_runner(values, [&x, &num_inserts](boost::span s) { + for (auto const& r : s) { + bool b = x.emplace(r); + if (b) { + ++num_inserts; + } + } + }); + BOOST_TEST_EQ(num_inserts, x.size()); + BOOST_TEST_EQ(raii::default_constructor, 0u); + + BOOST_TEST_EQ(raii::copy_constructor, value_type_cardinality * x.size()); + BOOST_TEST_GT(raii::move_constructor, 0u); + + BOOST_TEST_EQ(raii::copy_assignment, 0u); + BOOST_TEST_EQ(raii::move_assignment, 0u); + } + } copy_emplacer; + + struct move_emplacer_type + { + template void operator()(std::vector& values, X& x) + { + static constexpr auto value_type_cardinality = + value_cardinality::value; + + std::atomic num_inserts{0}; + thread_runner(values, [&x, &num_inserts](boost::span s) { + for (auto& r : s) { + bool b = x.emplace(std::move(r)); + if (b) { + ++num_inserts; + } + } + }); + BOOST_TEST_EQ(num_inserts, x.size()); + BOOST_TEST_EQ(raii::default_constructor, 0u); + +#if defined(BOOST_MSVC) +#pragma warning(push) +#pragma warning(disable : 4127) // conditional expression is constant +#endif + if (std::is_same::value && + !std::is_same::value) { // map value_type can only be + // copied, no move + BOOST_TEST_EQ(raii::copy_constructor, x.size()); + } else { + BOOST_TEST_EQ(raii::copy_constructor, 0u); + } +#if defined(BOOST_MSVC) +#pragma warning(pop) // C4127 +#endif + BOOST_TEST_GT(raii::move_constructor, value_type_cardinality * x.size()); + + BOOST_TEST_EQ(raii::copy_assignment, 0u); + BOOST_TEST_EQ(raii::move_assignment, 0u); + } + } move_emplacer; + template void emplace(X*, GF gen_factory, F emplacer, test::random_generator rg) { @@ -220,7 +288,7 @@ UNORDERED_TEST( ((map)(set)) ((value_type_generator_factory)(init_type_generator_factory)) ((lvalue_emplacer)(norehash_lvalue_emplacer) - (lvalue_emplace_or_cvisit)(lvalue_emplace_or_visit)) + (lvalue_emplace_or_cvisit)(lvalue_emplace_or_visit)(copy_emplacer)(move_emplacer)) ((default_generator)(sequential)(limited_range))) // clang-format on diff --git a/test/cfoa/exception_helpers.hpp b/test/cfoa/exception_helpers.hpp index 5afe1c24..f2434082 100644 --- a/test/cfoa/exception_helpers.hpp +++ b/test/cfoa/exception_helpers.hpp @@ -33,7 +33,7 @@ static std::size_t const num_threads = std::atomic_bool should_throw{false}; -constexpr std::uint32_t throw_threshold = 2500; +constexpr std::uint32_t throw_threshold = 2300; constexpr std::uint32_t alloc_throw_threshold = 10; void enable_exceptions() { should_throw = true; } diff --git a/test/helpers/count.hpp b/test/helpers/count.hpp index 90372dca..5a86ae9c 100644 --- a/test/helpers/count.hpp +++ b/test/helpers/count.hpp @@ -7,6 +7,7 @@ #define BOOST_UNORDERED_TEST_HELPERS_COUNT_HEAD #include +#include namespace test { struct object_count @@ -84,6 +85,111 @@ namespace test { return global_object_count.constructions - constructions_; } }; -} + + struct smf_count + { + int default_constructions = 0; + int copy_constructions = 0; + int move_constructions = 0; + int copy_assignments = 0; + int move_assignments = 0; + int destructions = 0; + +#if (BOOST_CXX_VERSION < 201402L) || (defined(_MSC_VER) && _MSC_VER < 1910) + smf_count() = default; + + smf_count(int default_constructions_, int copy_constructions_, + int move_constructions_, int copy_assignments_, int move_assignments_, + int destructions_) + : default_constructions(default_constructions_), + copy_constructions(copy_constructions_), + move_constructions(move_constructions_), + copy_assignments(copy_assignments_), + move_assignments(move_assignments_), destructions(destructions_) + { + } +#endif + + void reset() { *this = smf_count(); } + + void default_construct() { ++default_constructions; } + void copy_construct() { ++copy_constructions; } + void move_construct() { ++move_constructions; } + void copy_assign() { ++copy_assignments; } + void move_assign() { ++move_assignments; } + void destruct() { ++destructions; } + + friend bool operator==(smf_count const& lhs, smf_count const& rhs) + { + return lhs.default_constructions == rhs.default_constructions && + lhs.copy_constructions == rhs.copy_constructions && + lhs.move_constructions == rhs.move_constructions && + lhs.copy_assignments == rhs.copy_assignments && + lhs.move_assignments == rhs.move_assignments && + lhs.destructions == rhs.destructions; + } + + friend std::ostream& operator<<(std::ostream& out, smf_count const& c) + { + out << "[default_constructions: " << c.default_constructions + << ", copy_constructions: " << c.copy_constructions + << ", move_constructions: " << c.move_constructions + << ", copy_assignments: " << c.copy_assignments + << ", move_assignments: " << c.move_assignments + << ", destructions: " << c.destructions << "]"; + return out; + } + }; + + template class smf_counted_object + { + public: + static smf_count count; + static void reset_count() { count.reset(); } + + smf_counted_object() : index_(++running_index) + { + count.default_construct(); + } + smf_counted_object(smf_counted_object const& rhs) : index_(rhs.index_) + { + count.copy_construct(); + } + smf_counted_object(smf_counted_object&& rhs) noexcept : index_(rhs.index_) + { + count.move_construct(); + } + smf_counted_object& operator=(smf_counted_object const& rhs) + { + count.copy_assign(); + index_ = rhs.index_; + return *this; + } + smf_counted_object& operator=(smf_counted_object&& rhs) noexcept + { + count.move_assign(); + index_ = rhs.index_; + return *this; + } + ~smf_counted_object() { count.destruct(); } + + friend bool operator==( + smf_counted_object const& lhs, smf_counted_object const& rhs) + { + return lhs.index_ == rhs.index_; + } + + friend std::size_t hash_value(smf_counted_object const& x) + { + return boost::hash()(x.index_); + } + + private: + static int running_index; + int index_; + }; + template smf_count smf_counted_object::count = {}; + template int smf_counted_object::running_index = 0; +} // namespace test #endif diff --git a/test/unordered/emplace_smf_tests.cpp b/test/unordered/emplace_smf_tests.cpp new file mode 100644 index 00000000..368e1cf5 --- /dev/null +++ b/test/unordered/emplace_smf_tests.cpp @@ -0,0 +1,175 @@ +// +// Copyright 2023 Braden Ganetsky. +// 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 "../helpers/unordered.hpp" + +#include "../helpers/count.hpp" +#include "../helpers/test.hpp" + +namespace emplace_smf_tests { + using test::smf_count; + using test::smf_counted_object; + + using counted_key = smf_counted_object; + using counted_value = smf_counted_object; + void reset_counts() + { + counted_key::reset_count(); + counted_value::reset_count(); + } + +#ifdef BOOST_UNORDERED_FOA_TESTS + static boost::unordered_flat_map* test_smf_map; + static boost::unordered_node_map* + test_smf_node_map; + static boost::unordered_flat_set* test_smf_set; + static boost::unordered_node_set* test_smf_node_set; +#define EMPLACE_SMF_TESTS_MAP_ARGS ((test_smf_map)(test_smf_node_map)) +#define EMPLACE_SMF_TESTS_SET_ARGS ((test_smf_set)(test_smf_node_set)) +#else + static boost::unordered_map* test_smf_map; + static boost::unordered_set* test_smf_set; +#define EMPLACE_SMF_TESTS_MAP_ARGS ((test_smf_map)) +#define EMPLACE_SMF_TESTS_SET_ARGS ((test_smf_set)) +#endif + + template static void emplace_smf_value_type_map(X*) + { + using container = X; + using value_type = typename container::value_type; + + container x; + + { + value_type val{counted_key{}, counted_value{}}; + x.clear(); + reset_counts(); + x.emplace(val); + BOOST_TEST_EQ(counted_key::count, (smf_count{0, 1, 0, 0, 0, 0})); + BOOST_TEST_EQ(counted_value::count, (smf_count{0, 1, 0, 0, 0, 0})); + } + + { + value_type val{counted_key{}, counted_value{}}; + x.clear(); + reset_counts(); + x.emplace(std::move(val)); + BOOST_TEST_EQ(counted_key::count, (smf_count{0, 1, 0, 0, 0, 0})); + BOOST_TEST_EQ(counted_value::count, (smf_count{0, 0, 1, 0, 0, 0})); + } + + { + x.clear(); + reset_counts(); + x.emplace(value_type{counted_key{}, counted_value{}}); + BOOST_TEST_EQ(counted_key::count, (smf_count{1, 1, 1, 0, 0, 2})); + BOOST_TEST_EQ(counted_value::count, (smf_count{1, 0, 2, 0, 0, 2})); + } + + { + counted_key key{}; + counted_value value{}; + x.clear(); + reset_counts(); + x.emplace(value_type{std::move(key), std::move(value)}); + BOOST_TEST_EQ(counted_key::count, (smf_count{0, 1, 1, 0, 0, 1})); + BOOST_TEST_EQ(counted_value::count, (smf_count{0, 0, 2, 0, 0, 1})); + } + } + + UNORDERED_TEST(emplace_smf_value_type_map, EMPLACE_SMF_TESTS_MAP_ARGS) + + template static void emplace_smf_init_type_map(X*) + { + using container = X; +#ifdef BOOST_UNORDERED_FOA_TESTS + using init_type = typename container::init_type; +#else + using raw_key = + typename std::remove_const::type; + using init_type = std::pair; +#endif + + container x; + + { + init_type val{counted_key{}, counted_value{}}; + x.clear(); + reset_counts(); + x.emplace(val); + BOOST_TEST_EQ(counted_key::count, (smf_count{0, 1, 0, 0, 0, 0})); + BOOST_TEST_EQ(counted_value::count, (smf_count{0, 1, 0, 0, 0, 0})); + } + + { + init_type val{counted_key{}, counted_value{}}; + x.clear(); + reset_counts(); + x.emplace(std::move(val)); + BOOST_TEST_EQ(counted_key::count, (smf_count{0, 0, 1, 0, 0, 0})); + BOOST_TEST_EQ(counted_value::count, (smf_count{0, 0, 1, 0, 0, 0})); + } + + { + x.clear(); + reset_counts(); + x.emplace(init_type{counted_key{}, counted_value{}}); + BOOST_TEST_EQ(counted_key::count, (smf_count{1, 0, 2, 0, 0, 2})); + BOOST_TEST_EQ(counted_value::count, (smf_count{1, 0, 2, 0, 0, 2})); + } + + { + counted_key key{}; + counted_value value{}; + x.clear(); + reset_counts(); + x.emplace(init_type{std::move(key), std::move(value)}); + BOOST_TEST_EQ(counted_key::count, (smf_count{0, 0, 2, 0, 0, 1})); + BOOST_TEST_EQ(counted_value::count, (smf_count{0, 0, 2, 0, 0, 1})); + } + } + + UNORDERED_TEST(emplace_smf_init_type_map, EMPLACE_SMF_TESTS_MAP_ARGS) + + template static void emplace_smf_value_type_set(X*) + { + using container = X; + using value_type = typename container::value_type; +#ifdef BOOST_UNORDERED_FOA_TESTS + BOOST_STATIC_ASSERT( + std::is_same::value); +#endif + BOOST_STATIC_ASSERT(std::is_same::value); + + container x; + + { + counted_value val{}; + x.clear(); + reset_counts(); + x.emplace(val); + BOOST_TEST_EQ(counted_value::count, (smf_count{0, 1, 0, 0, 0, 0})); + } + + { + counted_value val{}; + x.clear(); + reset_counts(); + x.emplace(std::move(val)); + BOOST_TEST_EQ(counted_value::count, (smf_count{0, 0, 1, 0, 0, 0})); + } + + { + x.clear(); + reset_counts(); + x.emplace(counted_value{}); + BOOST_TEST_EQ(counted_value::count, (smf_count{1, 0, 1, 0, 0, 1})); + } + } + + UNORDERED_TEST(emplace_smf_value_type_set, EMPLACE_SMF_TESTS_SET_ARGS) +} // namespace emplace_smf_tests + +RUN_TESTS()