From b41bb5c5952e3800081ecd1f204c96fff0a85182 Mon Sep 17 00:00:00 2001 From: Christian Mazakas Date: Wed, 15 Dec 2021 09:42:33 -0800 Subject: [PATCH 1/2] Add failing test case for issue #12 --- test/Jamfile.v2 | 1 + test/unordered/reserve_tests.cpp | 224 +++++++++++++++++++++++++++++++ 2 files changed, 225 insertions(+) create mode 100644 test/unordered/reserve_tests.cpp diff --git a/test/Jamfile.v2 b/test/Jamfile.v2 index 4120e0aa..23022e3f 100644 --- a/test/Jamfile.v2 +++ b/test/Jamfile.v2 @@ -63,6 +63,7 @@ test-suite unordered [ run unordered/deduction_tests.cpp ] [ run unordered/scoped_allocator.cpp /boost/filesystem//boost_filesystem ] [ run unordered/transparent_tests.cpp ] + [ run unordered/reserve_tests.cpp ] [ run unordered/compile_set.cpp : : : BOOST_UNORDERED_USE_MOVE diff --git a/test/unordered/reserve_tests.cpp b/test/unordered/reserve_tests.cpp new file mode 100644 index 00000000..85841e19 --- /dev/null +++ b/test/unordered/reserve_tests.cpp @@ -0,0 +1,224 @@ +// Copyright 2021 Christian Mazakas. +// 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) + +// clang-format off +#include "../helpers/prefix.hpp" +#include +#include +#include "../helpers/postfix.hpp" +// clang-format on + +#include "../helpers/test.hpp" + +#include +#include + +#include +#include + +std::size_t total_allocation = 0; +std::size_t num_allocations = 0; + +struct B +{ + B() : i(++count) {} + static int count; + int i; + bool operator==(B const& b) const { return i == b.i; }; + bool operator!=(B const& b) const { return i != b.i; }; +}; + +int B::count = 0; + +template struct A : B +{ + typedef T value_type; + + A() {} + + template A(const A&) BOOST_NOEXCEPT {} + + T* allocate(std::size_t n) + { + total_allocation += n * sizeof(T); + ++num_allocations; + return (T*)std::calloc(n, sizeof(T)); + } + + void deallocate(T* p, std::size_t n) BOOST_NOEXCEPT + { + total_allocation -= n * sizeof(T); + std::free(p); + } +}; + +template void bucket_count_constructor() +{ + BOOST_TEST_EQ(num_allocations, 0); + BOOST_TEST_EQ(total_allocation, 0); + + { + std::size_t count = 50000; + + UnorderedContainer s(count); + + BOOST_TEST_GE(total_allocation, count * sizeof(void*)); + BOOST_TEST_GE(s.bucket_count(), count); + } + + BOOST_TEST_GT(num_allocations, 0); + BOOST_TEST_EQ(total_allocation, 0); + num_allocations = 0; +} + +template void range_bucket_constructor() +{ + BOOST_TEST_EQ(num_allocations, 0); + BOOST_TEST_EQ(total_allocation, 0); + + { + UnorderedContainer s1; + + std::size_t count = 50000; + + UnorderedContainer s2(s1.begin(), s1.end(), count); + + BOOST_TEST_GE(total_allocation, count * sizeof(void*)); + BOOST_TEST_GE(s2.bucket_count(), count); + } + + BOOST_TEST_GT(num_allocations, 0); + BOOST_TEST_EQ(total_allocation, 0); + num_allocations = 0; +} + +template void reserve_tests() +{ + BOOST_TEST_EQ(num_allocations, 0); + BOOST_TEST_EQ(total_allocation, 0); + + { + UnorderedContainer s; + + // simple math for the test: + // max_load_factor = max_size / bucket_count, before a rehashing occurs + // + // reserve() respects max load factor and its argument implies the max size + // + // reserve(count) => bucket_count = ceil(count / max_load_factor) + // internal policies reshape bucket_count accordingly but guarantee count as + // a minimum + // + + std::size_t count = 50000; + + s.max_load_factor(0.37f); + s.reserve(count); + + std::size_t expected_bucket_count = + static_cast(std::ceil(static_cast(count) / 0.37f)); + + BOOST_TEST_GE(total_allocation, expected_bucket_count * sizeof(void*)); + BOOST_TEST_GE(s.bucket_count(), expected_bucket_count); + + std::size_t prev_allocations = num_allocations; + s.reserve(count); + BOOST_TEST_EQ(num_allocations, prev_allocations); + } + + BOOST_TEST_GT(num_allocations, 0); + BOOST_TEST_EQ(total_allocation, 0); + num_allocations = 0; +} + +template void rehash_tests() +{ + BOOST_TEST_EQ(num_allocations, 0); + BOOST_TEST_EQ(total_allocation, 0); + + { + UnorderedContainer s; + + std::size_t count = 1000; + s.rehash(count); + + // test that an initial allocation occurs + // + BOOST_TEST_GE(total_allocation, count * sizeof(void*)); + BOOST_TEST_GE(s.bucket_count(), count); + + // prove idempotence, that rehashing with the exact same bucket count causes + // no reallocations + // + std::size_t prev_allocations = num_allocations; + + s.rehash(count); + BOOST_TEST_EQ(num_allocations, prev_allocations); + + // prove that when we rehash, exceeding the current bucket count, that we + // properly deallocate the current bucket array and then reallocate the + // larger one + // + std::size_t prev_count = count; + + count = s.bucket_count() + 1; + s.rehash(count); + + BOOST_TEST_GT(num_allocations, prev_allocations); + BOOST_TEST_GE(total_allocation, count * sizeof(void*)); + BOOST_TEST_GE(s.bucket_count(), count); + + // concurrent memory usage here should be less than the sum of the memory + // required for the previous bucket array and our current one + // note, the test is vulnerable to cases where the next calculated bucket + // count can exceed `prev_count + count` + // + BOOST_TEST_LT(s.bucket_count(), prev_count + count); + BOOST_TEST_LT(total_allocation, (prev_count + count) * sizeof(void*)); + } + + BOOST_TEST_GT(num_allocations, 0); + BOOST_TEST_EQ(total_allocation, 0); + num_allocations = 0; +} + +UNORDERED_AUTO_TEST (unordered_set_reserve) { + typedef boost::unordered_set, std::equal_to, + A > + unordered_set; + + typedef boost::unordered_multiset, std::equal_to, + A > + unordered_multiset; + + typedef boost::unordered_map, std::equal_to, + A > > + unordered_map; + + typedef boost::unordered_multimap, + std::equal_to, A > > + unordered_multimap; + + bucket_count_constructor(); + bucket_count_constructor(); + bucket_count_constructor(); + bucket_count_constructor(); + + range_bucket_constructor(); + range_bucket_constructor(); + range_bucket_constructor(); + range_bucket_constructor(); + + reserve_tests(); + reserve_tests(); + reserve_tests(); + reserve_tests(); + + rehash_tests(); + rehash_tests(); + rehash_tests(); + rehash_tests(); +} + +RUN_TESTS() From 1db53ba155f3bf2e7bf7eaf6f439a2b52abf170b Mon Sep 17 00:00:00 2001 From: Christian Mazakas Date: Wed, 15 Dec 2021 09:42:44 -0800 Subject: [PATCH 2/2] Update internal table to allocate on construction and when rehashing --- include/boost/unordered/detail/implementation.hpp | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/include/boost/unordered/detail/implementation.hpp b/include/boost/unordered/detail/implementation.hpp index b65d4bc0..8661e408 100644 --- a/include/boost/unordered/detail/implementation.hpp +++ b/include/boost/unordered/detail/implementation.hpp @@ -2486,6 +2486,7 @@ namespace boost { bucket_count_(policy::new_bucket_count(num_buckets)), size_(0), mlf_(1.0f), max_load_(0), buckets_() { + this->create_buckets(bucket_count_); } table(table const& x, node_allocator const& a) @@ -3951,8 +3952,10 @@ namespace boost { using namespace std; if (!size_) { - delete_buckets(); - bucket_count_ = policy::new_bucket_count(min_buckets); + min_buckets = policy::new_bucket_count(min_buckets); + if (min_buckets != bucket_count_) { + this->create_buckets(min_buckets); + } } else { min_buckets = policy::new_bucket_count((std::max)(min_buckets, boost::unordered::detail::double_to_size(