From b25e31f406721d7425a6e42106d38fbe64847dd2 Mon Sep 17 00:00:00 2001 From: Lukas Kerkemeier Date: Thu, 20 May 2021 15:51:44 +0200 Subject: [PATCH 1/4] Initial commit. First few tests, found one problem. --- include/tsl/sparse_hash.h | 9 +- tests/CMakeLists.txt | 4 +- .../sparse_array_tests.cpp | 86 +++++++++++++++++++ .../sparse_hash_tests.cpp | 57 ++++++++++++ 4 files changed, 151 insertions(+), 5 deletions(-) create mode 100644 tests/scoped_allocator_adaptor/sparse_array_tests.cpp create mode 100644 tests/scoped_allocator_adaptor/sparse_hash_tests.cpp diff --git a/include/tsl/sparse_hash.h b/include/tsl/sparse_hash.h index e383b4e..0205891 100644 --- a/include/tsl/sparse_hash.h +++ b/include/tsl/sparse_hash.h @@ -445,7 +445,8 @@ class sparse_array { m_capacity(0), m_last_array(last_bucket) {} - sparse_array(size_type capacity, Allocator &alloc) + //sparse_array(size_type capacity, Allocator &alloc) + sparse_array(size_type capacity, Allocator alloc) : m_values(nullptr), m_bitmap_vals(0), m_bitmap_deleted_vals(0), @@ -1082,9 +1083,9 @@ class sparse_hash : private Allocator, using sparse_buckets_allocator = typename std::allocator_traits< allocator_type>::template rebind_alloc; using sparse_buckets_container = - std::vector; + std::vector;//here - public: +public: /** * The `operator*()` and `operator->()` methods return a const reference and * const pointer respectively to the stored value type (`Key` for a set, @@ -1231,7 +1232,7 @@ class sparse_hash : private Allocator, * We can't use `vector(size_type count, const T& value, const Allocator& * alloc)` as it requires the value T to be copyable. */ - m_sparse_buckets_data.resize( + m_sparse_buckets_data.resize( //TODO: this is the current critical point sparse_array::nb_sparse_buckets(bucket_count)); m_sparse_buckets = m_sparse_buckets_data.data(); diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index cb1047f..e87c9c5 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -12,7 +12,9 @@ add_executable(tsl_sparse_map_tests "main.cpp" "fancy_pointer/sparse_hash_set_tests.cpp" "fancy_pointer/CustomAllocator.h" "fancy_pointer/sparse_hash_map_tests.cpp" - "../include/tsl/boost_offset_pointer.h") + "../include/tsl/boost_offset_pointer.h" + "scoped_allocator_adaptor/sparse_array_tests.cpp" + "scoped_allocator_adaptor/sparse_hash_tests.cpp") target_compile_features(tsl_sparse_map_tests PRIVATE cxx_std_11) diff --git a/tests/scoped_allocator_adaptor/sparse_array_tests.cpp b/tests/scoped_allocator_adaptor/sparse_array_tests.cpp new file mode 100644 index 0000000..33b6ac1 --- /dev/null +++ b/tests/scoped_allocator_adaptor/sparse_array_tests.cpp @@ -0,0 +1,86 @@ +#include +#include +#include +#include +#include +#include +#include + +// Globals +constexpr auto MAX_INDEX = 32; // BITMAP_NB_BITS + +template void compilation() { typename T::Array test; } + +template void construction() { + typename T::Allocator a; + typename T::Array test(MAX_INDEX, a); + test.clear(a); +} + +template +void set(std::initializer_list l) { + typename T::Allocator a; + typename T::Array array(MAX_INDEX, a); + std::vector check; + check.reserve(l.size()); + std::size_t counter = 0; + for (auto const &value : l) { + array.set(a, counter++, value); + check.emplace_back(value); + } + BOOST_TEST_REQUIRE(std::equal(array.begin(), array.end(), check.begin()), + "'set' did not create the correct order of items"); + array.clear(a); +} + +template void uses_allocator() { + BOOST_TEST_REQUIRE((std::uses_allocator::value), + "uses_allocator returns false"); +} + +template void trailing_allocator_convention(Args ...) { + using Alloc = typename T::Allocator; + BOOST_TEST_REQUIRE((std::is_constructible::value), + "trailing_allocator thinks construction is not possible"); +} + +template void is_constructable_test() { + auto alloc = typename std::allocator_traits::rebind_alloc< + typename T::Array>(); + auto const& alloc_ref = alloc; + std::vector vector(alloc_ref); +} + +template +struct NORMAL { + using value_type = T; + using Allocator = std::allocator; + using Array = tsl::detail_sparse_hash::sparse_array; +}; + +template +struct SCOPED { + using value_type = T; + using Allocator = std::scoped_allocator_adaptor>; + using Array = tsl::detail_sparse_hash::sparse_array; +}; + +BOOST_AUTO_TEST_SUITE(scoped_allocators) +BOOST_AUTO_TEST_SUITE(sparse_array_tests) + +BOOST_AUTO_TEST_CASE(normal_compilation) { compilation>(); } +BOOST_AUTO_TEST_CASE(normal_construction) { construction>(); } +BOOST_AUTO_TEST_CASE(normal_set) { set>({0, 1, 2, 3, 4}); } +BOOST_AUTO_TEST_CASE(normal_uses_allocator) { uses_allocator>(); } +BOOST_AUTO_TEST_CASE(normal_leading_allocator_convention) {trailing_allocator_convention>(0); } +BOOST_AUTO_TEST_CASE(normal_is_constructable_test) {is_constructable_test>(); } + +BOOST_AUTO_TEST_CASE(scoped_compilation) { compilation>(); } +BOOST_AUTO_TEST_CASE(scoped_construction) { construction>(); } +BOOST_AUTO_TEST_CASE(scoped_set) { set>({0, 1, 2, 3, 4}); } +BOOST_AUTO_TEST_CASE(scoped_uses_allocator) { uses_allocator>(); } +BOOST_AUTO_TEST_CASE(scoped_leading_allocator_convention) {trailing_allocator_convention>(0); } +BOOST_AUTO_TEST_CASE(scoped_is_constructable_test) {is_constructable_test>(); } + +BOOST_AUTO_TEST_SUITE_END() +BOOST_AUTO_TEST_SUITE_END() diff --git a/tests/scoped_allocator_adaptor/sparse_hash_tests.cpp b/tests/scoped_allocator_adaptor/sparse_hash_tests.cpp new file mode 100644 index 0000000..53fc775 --- /dev/null +++ b/tests/scoped_allocator_adaptor/sparse_hash_tests.cpp @@ -0,0 +1,57 @@ +#include +#include +#include + +namespace details { +template struct KeySelect { + using key_type = Key; + const key_type &operator()(Key const &key) const noexcept { return key; } + key_type &operator()(Key &key) noexcept { return key; } +}; + +template +using sparse_set = tsl::detail_sparse_hash::sparse_hash< + T, details::KeySelect, void, std::hash, std::equal_to, Alloc, + tsl::sh::power_of_two_growth_policy<2>, + tsl::sh::exception_safety::basic, + tsl::sh::sparsity::medium, + tsl::sh::probing::quadratic>; +} // namespace details + +template void construction() { + using Type = typename T::value_type; + typename T::Set(T::Set::DEFAULT_INIT_BUCKET_COUNT, std::hash(), + std::equal_to(), typename T::Allocator(), + T::Set::DEFAULT_MAX_LOAD_FACTOR); +} + + +template +struct NORMAL { + using value_type = T; + using Allocator = std::allocator; + using Set = details::sparse_set; +}; + +template +struct SCOPED { + using value_type = T; + using Allocator = std::scoped_allocator_adaptor>; + using Set = details::sparse_set; +}; + +BOOST_AUTO_TEST_SUITE(scoped_allocators) +BOOST_AUTO_TEST_SUITE(sparse_hash_set_tests) + +BOOST_AUTO_TEST_CASE(normal_construction){construction>();} + +BOOST_AUTO_TEST_CASE(scoped_construction){ + SCOPED::Set (0, + std::hash(), + std::equal_to(), + SCOPED::Allocator(), + 200); +} + +BOOST_AUTO_TEST_SUITE_END() +BOOST_AUTO_TEST_SUITE_END() \ No newline at end of file From a4d5399e8964a652f752bb75fc3521c38ba57586 Mon Sep 17 00:00:00 2001 From: Lukas Kerkemeier Date: Thu, 27 May 2021 13:00:08 +0200 Subject: [PATCH 2/4] Now scoped allocators can be used. However emplace still does not work as it should. --- include/tsl/sparse_hash.h | 16 +++- .../sparse_array_tests.cpp | 82 +++++++++++++++---- 2 files changed, 79 insertions(+), 19 deletions(-) diff --git a/include/tsl/sparse_hash.h b/include/tsl/sparse_hash.h index 0205891..2a4a09d 100644 --- a/include/tsl/sparse_hash.h +++ b/include/tsl/sparse_hash.h @@ -437,6 +437,9 @@ class sparse_array { m_capacity(0), m_last_array(false) {} + //needed for "is_constructible" with no parameters + sparse_array(std::allocator_arg_t, Allocator const&) noexcept : sparse_array() {} + explicit sparse_array(bool last_bucket) noexcept : m_values(nullptr), m_bitmap_vals(0), @@ -445,8 +448,8 @@ class sparse_array { m_capacity(0), m_last_array(last_bucket) {} - //sparse_array(size_type capacity, Allocator &alloc) - sparse_array(size_type capacity, Allocator alloc) + //const Allocator needed for MoveInsertable requirement + sparse_array(size_type capacity, Allocator const &const_alloc) : m_values(nullptr), m_bitmap_vals(0), m_bitmap_deleted_vals(0), @@ -454,13 +457,15 @@ class sparse_array { m_capacity(capacity), m_last_array(false) { if (m_capacity > 0) { + auto alloc = const_cast(const_alloc); m_values = alloc.allocate(m_capacity); tsl_sh_assert(m_values != nullptr); // allocate should throw if there is a failure } } - sparse_array(const sparse_array &other, Allocator &alloc) + //const Allocator needed for MoveInsertable requirement + sparse_array(const sparse_array &other, Allocator const &const_alloc) : m_values(nullptr), m_bitmap_vals(other.m_bitmap_vals), m_bitmap_deleted_vals(other.m_bitmap_deleted_vals), @@ -472,6 +477,7 @@ class sparse_array { return; } + auto alloc = const_cast(const_alloc); m_values = alloc.allocate(m_capacity); tsl_sh_assert(m_values != nullptr); // allocate should throw if there is a failure @@ -500,7 +506,8 @@ class sparse_array { other.m_capacity = 0; } - sparse_array(sparse_array &&other, Allocator &alloc) + //const Allocator needed for MoveInsertable requirement + sparse_array(sparse_array &&other, Allocator const &const_alloc) : m_values(nullptr), m_bitmap_vals(other.m_bitmap_vals), m_bitmap_deleted_vals(other.m_bitmap_deleted_vals), @@ -512,6 +519,7 @@ class sparse_array { return; } + auto alloc = const_cast(const_alloc); m_values = alloc.allocate(m_capacity); tsl_sh_assert(m_values != nullptr); // allocate should throw if there is a failure diff --git a/tests/scoped_allocator_adaptor/sparse_array_tests.cpp b/tests/scoped_allocator_adaptor/sparse_array_tests.cpp index 33b6ac1..0145c86 100644 --- a/tests/scoped_allocator_adaptor/sparse_array_tests.cpp +++ b/tests/scoped_allocator_adaptor/sparse_array_tests.cpp @@ -3,8 +3,8 @@ #include #include #include -#include #include +#include // Globals constexpr auto MAX_INDEX = 32; // BITMAP_NB_BITS @@ -34,21 +34,59 @@ void set(std::initializer_list l) { } template void uses_allocator() { - BOOST_TEST_REQUIRE((std::uses_allocator::value), - "uses_allocator returns false"); + BOOST_TEST_REQUIRE( + (std::uses_allocator::value), + "uses_allocator returns false"); } -template void trailing_allocator_convention(Args ...) { +template +void trailing_allocator_convention(Args...) { using Alloc = typename T::Allocator; - BOOST_TEST_REQUIRE((std::is_constructible::value), - "trailing_allocator thinks construction is not possible"); + BOOST_TEST_REQUIRE( + (std::is_constructible::value), + "trailing_allocator thinks construction is not possible"); +} + +template void trailing_allocator_convention_without_parameters() { + using Alloc = typename std::allocator_traits< + typename T::Allocator>::template rebind_alloc; + BOOST_TEST_REQUIRE( + (std::is_constructible::value), + "trailing_allocator thinks construction is not possible"); +} + +/** This test creates a memory leak. + * However it is only one single sparse_array and with this, the test is + * simpler. + */ +template +void is_move_insertable(std::initializer_list l) { + using A = typename std::allocator_traits< + typename T::Allocator>::template rebind_alloc; + A m; + auto p = m.allocate(1); + typename T::Allocator ArrayAlloc; + typename T::Array rv(MAX_INDEX, ArrayAlloc); + std::size_t counter = 0; + for (auto const &value : l) { + rv.set(ArrayAlloc, counter++, value); + } + std::cout << "Before\n"; + std::allocator_traits::construct(m, p, std::move(rv)); + std::cout << "After\n"; + rv.clear(ArrayAlloc); } -template void is_constructable_test() { - auto alloc = typename std::allocator_traits::rebind_alloc< - typename T::Array>(); - auto const& alloc_ref = alloc; - std::vector vector(alloc_ref); +/** This test creates a memory leak. + * However it is only one single sparse_array and with this, the test is + * simpler. + */ +template void is_default_insertable() { + using A = typename std::allocator_traits< + typename T::Allocator>::template rebind_alloc; + A m; + typename T::Array *p = m.allocate(1); + std::allocator_traits::construct(m, p); } template @@ -72,15 +110,29 @@ BOOST_AUTO_TEST_CASE(normal_compilation) { compilation>(); } BOOST_AUTO_TEST_CASE(normal_construction) { construction>(); } BOOST_AUTO_TEST_CASE(normal_set) { set>({0, 1, 2, 3, 4}); } BOOST_AUTO_TEST_CASE(normal_uses_allocator) { uses_allocator>(); } -BOOST_AUTO_TEST_CASE(normal_leading_allocator_convention) {trailing_allocator_convention>(0); } -BOOST_AUTO_TEST_CASE(normal_is_constructable_test) {is_constructable_test>(); } +BOOST_AUTO_TEST_CASE(normal_trailing_allocator_convention) { + trailing_allocator_convention>(0); +} +BOOST_AUTO_TEST_CASE(normal_is_move_insertable) { + is_move_insertable>({0, 1, 2, 3, 4, 5}); +} +BOOST_AUTO_TEST_CASE(normal_is_default_insertable) { + is_default_insertable>(); +} BOOST_AUTO_TEST_CASE(scoped_compilation) { compilation>(); } BOOST_AUTO_TEST_CASE(scoped_construction) { construction>(); } BOOST_AUTO_TEST_CASE(scoped_set) { set>({0, 1, 2, 3, 4}); } BOOST_AUTO_TEST_CASE(scoped_uses_allocator) { uses_allocator>(); } -BOOST_AUTO_TEST_CASE(scoped_leading_allocator_convention) {trailing_allocator_convention>(0); } -BOOST_AUTO_TEST_CASE(scoped_is_constructable_test) {is_constructable_test>(); } +BOOST_AUTO_TEST_CASE(scoped_trailing_allocator_convention) { + trailing_allocator_convention>(0); +} +BOOST_AUTO_TEST_CASE(scoped_is_move_insertable) { + is_move_insertable>({0, 1, 2, 3, 4, 5}); +} +BOOST_AUTO_TEST_CASE(scoped_is_default_insertable) { + is_default_insertable>(); +} BOOST_AUTO_TEST_SUITE_END() BOOST_AUTO_TEST_SUITE_END() From f67de44836cc30655d8d0eeb2fce5776670b7668 Mon Sep 17 00:00:00 2001 From: Lukas Kerkemeier Date: Thu, 27 May 2021 13:47:27 +0200 Subject: [PATCH 3/4] Some cleanup. --- tests/CMakeLists.txt | 2 +- .../{sparse_hash_tests.cpp => sparse_hash_set_tests.cpp} | 8 +------- 2 files changed, 2 insertions(+), 8 deletions(-) rename tests/scoped_allocator_adaptor/{sparse_hash_tests.cpp => sparse_hash_set_tests.cpp} (87%) diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index e87c9c5..622d0b3 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -14,7 +14,7 @@ add_executable(tsl_sparse_map_tests "main.cpp" "fancy_pointer/sparse_hash_map_tests.cpp" "../include/tsl/boost_offset_pointer.h" "scoped_allocator_adaptor/sparse_array_tests.cpp" - "scoped_allocator_adaptor/sparse_hash_tests.cpp") + "scoped_allocator_adaptor/sparse_hash_set_tests.cpp") target_compile_features(tsl_sparse_map_tests PRIVATE cxx_std_11) diff --git a/tests/scoped_allocator_adaptor/sparse_hash_tests.cpp b/tests/scoped_allocator_adaptor/sparse_hash_set_tests.cpp similarity index 87% rename from tests/scoped_allocator_adaptor/sparse_hash_tests.cpp rename to tests/scoped_allocator_adaptor/sparse_hash_set_tests.cpp index 53fc775..4fbf569 100644 --- a/tests/scoped_allocator_adaptor/sparse_hash_tests.cpp +++ b/tests/scoped_allocator_adaptor/sparse_hash_set_tests.cpp @@ -45,13 +45,7 @@ BOOST_AUTO_TEST_SUITE(sparse_hash_set_tests) BOOST_AUTO_TEST_CASE(normal_construction){construction>();} -BOOST_AUTO_TEST_CASE(scoped_construction){ - SCOPED::Set (0, - std::hash(), - std::equal_to(), - SCOPED::Allocator(), - 200); -} +BOOST_AUTO_TEST_CASE(scoped_construction){construction>();} BOOST_AUTO_TEST_SUITE_END() BOOST_AUTO_TEST_SUITE_END() \ No newline at end of file From f04e86073142b79dcd9525ccf3f8fff95e51b651 Mon Sep 17 00:00:00 2001 From: Lukas Kerkemeier Date: Thu, 27 May 2021 14:16:31 +0200 Subject: [PATCH 4/4] Formatting and removing comments. --- include/tsl/sparse_hash.h | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/include/tsl/sparse_hash.h b/include/tsl/sparse_hash.h index 2a4a09d..fd9d64b 100644 --- a/include/tsl/sparse_hash.h +++ b/include/tsl/sparse_hash.h @@ -1091,9 +1091,9 @@ class sparse_hash : private Allocator, using sparse_buckets_allocator = typename std::allocator_traits< allocator_type>::template rebind_alloc; using sparse_buckets_container = - std::vector;//here + std::vector; -public: + public: /** * The `operator*()` and `operator->()` methods return a const reference and * const pointer respectively to the stored value type (`Key` for a set, @@ -1240,7 +1240,7 @@ class sparse_hash : private Allocator, * We can't use `vector(size_type count, const T& value, const Allocator& * alloc)` as it requires the value T to be copyable. */ - m_sparse_buckets_data.resize( //TODO: this is the current critical point + m_sparse_buckets_data.resize( sparse_array::nb_sparse_buckets(bucket_count)); m_sparse_buckets = m_sparse_buckets_data.data();