From 7bb93420c4615562016a4a8e74b38774a87d8c87 Mon Sep 17 00:00:00 2001 From: Michele Caini Date: Mon, 27 Sep 2021 08:31:24 +0200 Subject: [PATCH] storage: avoid reinventing the wheel :) --- src/entt/entity/storage.hpp | 173 +++++++++++++++-------------------- test/entt/entity/storage.cpp | 12 +-- 2 files changed, 76 insertions(+), 109 deletions(-) diff --git a/src/entt/entity/storage.hpp b/src/entt/entity/storage.hpp index 346cd39d4..eeb42e084 100644 --- a/src/entt/entity/storage.hpp +++ b/src/entt/entity/storage.hpp @@ -7,6 +7,7 @@ #include #include #include +#include #include "../config/config.h" #include "../core/algorithm.hpp" #include "../core/compressed_pair.hpp" @@ -27,23 +28,28 @@ namespace entt { namespace internal { -template +template class storage_iterator final { static constexpr auto packed_page_v = ENTT_PACKED_PAGE; - using internal_traits = std::iterator_traits; - using data_pointer = typename Traits::pointer; + using container_type = std::remove_const_t; + using allocator_traits = std::allocator_traits; + + using iterator_traits = std::iterator_traits, + typename allocator_traits::template rebind_traits::element_type>::const_pointer, + typename allocator_traits::template rebind_traits::element_type>::pointer>>; public: - using difference_type = typename internal_traits::difference_type; - using value_type = typename internal_traits::value_type; - using pointer = typename internal_traits::pointer; - using reference = typename internal_traits::reference; + using difference_type = typename iterator_traits::difference_type; + using value_type = typename iterator_traits::value_type; + using pointer = typename iterator_traits::pointer; + using reference = typename iterator_traits::reference; using iterator_category = std::random_access_iterator_tag; storage_iterator() ENTT_NOEXCEPT = default; - storage_iterator(const data_pointer *ref, difference_type idx) ENTT_NOEXCEPT + storage_iterator(Container *ref, difference_type idx) ENTT_NOEXCEPT : packed{ref}, index{idx} {} @@ -125,7 +131,7 @@ public: } private: - const data_pointer *packed; + Container *packed; difference_type index; }; @@ -167,107 +173,77 @@ class basic_storage: public basic_sparse_set; - using alloc = typename allocator_traits::template rebind_alloc; using alloc_traits = typename std::allocator_traits; - using alloc_const_pointer = typename alloc_traits::const_pointer; - using alloc_pointer = typename alloc_traits::pointer; - - using alloc_ptr = typename allocator_traits::template rebind_alloc; - using alloc_ptr_traits = typename std::allocator_traits; - using alloc_ptr_const_pointer = typename allocator_traits::template rebind_traits::const_pointer; - using alloc_ptr_pointer = typename alloc_ptr_traits::pointer; using entity_traits = entt_traits; using comp_traits = component_traits; using underlying_type = basic_sparse_set>; + using container_type = std::vector>; [[nodiscard]] auto &element_at(const std::size_t pos) const { - return packed[pos / packed_page_v][fast_mod(pos)]; + return packed.first()[pos / packed_page_v][fast_mod(pos)]; } auto assure_at_least(const std::size_t pos) { + auto &&container = packed.first(); const auto idx = pos / packed_page_v; - if(!(idx < bucket.second())) { - const size_type sz = idx + 1u; - alloc_ptr allocator_ptr{bucket.first()}; - const auto mem = alloc_ptr_traits::allocate(allocator_ptr, sz); - std::uninitialized_value_construct(mem + bucket.second(), mem + sz); + if(!(idx < container.size())) { + auto curr = container.size(); + container.resize(idx + 1u, nullptr); ENTT_TRY { - for(auto next = bucket.second(); next < sz; ++next) { - mem[next] = alloc_traits::allocate(bucket.first(), packed_page_v); + for(const auto last = container.size(); curr < last; ++curr) { + container[curr] = alloc_traits::allocate(packed.second(), packed_page_v); } } ENTT_CATCH { - for(auto next = bucket.second(); next < sz && mem[next]; ++next) { - alloc_traits::deallocate(bucket.first(), mem[next], packed_page_v); - } - - std::destroy(mem + bucket.second(), mem + sz); - alloc_ptr_traits::deallocate(allocator_ptr, mem, sz); + container.resize(curr); ENTT_THROW; } - - if(packed) { - std::uninitialized_copy(packed, packed + bucket.second(), mem); - std::destroy(packed, packed + bucket.second()); - alloc_ptr_traits::deallocate(allocator_ptr, packed, bucket.second()); - } - - packed = mem; - bucket.second() = sz; } - return packed[idx] + fast_mod(pos); + return container[idx] + fast_mod(pos); } void release_unused_pages() { - if(const auto length = base_type::size() / packed_page_v; length < bucket.second()) { - alloc_ptr allocator_ptr{bucket.first()}; - const auto mem = alloc_ptr_traits::allocate(allocator_ptr, length); - std::uninitialized_copy(packed, packed + length, mem); + auto &&container = packed.first(); + auto page_allocator{packed.second()}; + const auto in_use = (base_type::size() + packed_page_v - 1u) / packed_page_v; - for(auto pos = length, last = bucket.second(); pos < last; ++pos) { - alloc_traits::deallocate(bucket.first(), packed[pos], packed_page_v); - } - - std::destroy(packed, packed + bucket.second()); - alloc_ptr_traits::deallocate(allocator_ptr, packed, bucket.second()); - - packed = mem; - bucket.second() = length; + for(auto pos = in_use, last = container.size(); pos < last; ++pos) { + alloc_traits::deallocate(page_allocator, container[pos], packed_page_v); } + + container.resize(in_use); } - void release_memory() { - if(packed) { + void release_all_pages() { + for(size_type pos{}, last = base_type::size(); pos < last; ++pos) { if constexpr(comp_traits::in_place_delete::value) { - // no-throw stable erase iteration - base_type::clear(); - } else { - for(size_type pos{}, last = base_type::size(); pos < last; ++pos) { + if(base_type::at(pos) != tombstone) { std::destroy_at(std::addressof(element_at(pos))); } + } else { + std::destroy_at(std::addressof(element_at(pos))); } + } - for(size_type pos{}, last = bucket.second(); pos < last; ++pos) { - alloc_traits::deallocate(bucket.first(), packed[pos], packed_page_v); - std::destroy_at(std::addressof(packed[pos])); - } + auto &&container = packed.first(); + auto page_allocator{packed.second()}; - alloc_ptr allocator_ptr{bucket.first()}; - alloc_ptr_traits::deallocate(allocator_ptr, packed, bucket.second()); + for(size_type pos{}, last = container.size(); pos < last; ++pos) { + alloc_traits::deallocate(page_allocator, container[pos], packed_page_v); } } template - void construct(alloc_pointer ptr, Args &&...args) { + void construct(typename alloc_traits::pointer ptr, Args &&...args) { if constexpr(std::is_aggregate_v) { - alloc_traits::construct(bucket.first(), to_address(ptr), Type{std::forward(args)...}); + alloc_traits::construct(packed.second(), to_address(ptr), Type{std::forward(args)...}); } else { - alloc_traits::construct(bucket.first(), to_address(ptr), std::forward(args)...); + alloc_traits::construct(packed.second(), to_address(ptr), std::forward(args)...); } } @@ -294,9 +270,8 @@ protected: void swap_contents(underlying_type &base) override { using std::swap; auto &other = static_cast(base); - propagate_on_container_swap(bucket.first(), other.bucket.first()); - swap(bucket.second(), other.bucket.second()); - swap(packed, other.packed); + propagate_on_container_swap(packed.second(), other.packed.second()); + swap(packed.first(), other.packed.first()); } /** @@ -384,13 +359,13 @@ public: /*! @brief Unsigned integer type. */ using size_type = std::size_t; /*! @brief Pointer type to contained elements. */ - using pointer = alloc_ptr_pointer; + using pointer = typename container_type::pointer; /*! @brief Constant pointer type to contained elements. */ - using const_pointer = alloc_ptr_const_pointer; + using const_pointer = typename alloc_traits::template rebind_traits::const_pointer; /*! @brief Random access iterator type. */ - using iterator = internal::storage_iterator>; + using iterator = internal::storage_iterator; /*! @brief Constant random access iterator type. */ - using const_iterator = internal::storage_iterator>; + using const_iterator = internal::storage_iterator; /*! @brief Reverse iterator type. */ using reverse_iterator = std::reverse_iterator; /*! @brief Constant reverse iterator type. */ @@ -399,7 +374,6 @@ public: /*! @brief Default constructor. */ basic_storage() : base_type{deletion_policy{comp_traits::in_place_delete::value}}, - bucket{allocator_type{}, size_type{}}, packed{} {} /** @@ -408,8 +382,7 @@ public: */ explicit basic_storage(const allocator_type &allocator) : base_type{deletion_policy{comp_traits::in_place_delete::value}, allocator}, - bucket{allocator, size_type{}}, - packed{} {} + packed{container_type{allocator}, allocator} {} /** * @brief Move constructor. @@ -417,8 +390,7 @@ public: */ basic_storage(basic_storage &&other) ENTT_NOEXCEPT : base_type{std::move(other)}, - bucket{std::move(other.bucket.first()), std::exchange(other.bucket.second(), size_type{})}, - packed{std::exchange(other.packed, alloc_ptr_pointer{})} {} + packed{std::move(other.packed)} {} /** * @brief Allocator-extended move constructor. @@ -427,14 +399,13 @@ public: */ basic_storage(basic_storage &&other, const allocator_type &allocator) ENTT_NOEXCEPT : base_type{std::move(other), allocator}, - bucket{allocator, std::exchange(other.bucket.second(), size_type{})}, - packed{std::exchange(other.packed, alloc_ptr_pointer{})} { - ENTT_ASSERT(alloc_traits::is_always_equal::value || bucket.first() == other.bucket.first(), "Copying a storage is not allowed"); + packed{container_type{std::move(other.packed.first()), allocator}, allocator} { + ENTT_ASSERT(alloc_traits::is_always_equal::value || packed.second() == other.packed.second(), "Copying a storage is not allowed"); } /*! @brief Default destructor. */ ~basic_storage() override { - release_memory(); + release_all_pages(); } /** @@ -443,11 +414,12 @@ public: * @return This storage. */ basic_storage &operator=(basic_storage &&other) ENTT_NOEXCEPT { - release_memory(); + ENTT_ASSERT(alloc_traits::is_always_equal::value || packed.second() == other.packed.second(), "Copying a sparse set is not allowed"); + + release_all_pages(); base_type::operator=(std::move(other)); - propagate_on_container_move_assignment(bucket.first(), other.bucket.first()); - bucket.second() = std::exchange(other.bucket.second(), size_type{}); - packed = std::exchange(other.packed, alloc_ptr_pointer{}); + packed.first() = std::move(other.packed.first()); + propagate_on_container_move_assignment(packed.second(), other.packed.second()); return *this; } @@ -456,7 +428,7 @@ public: * @return The associated allocator. */ [[nodiscard]] constexpr allocator_type get_allocator() const ENTT_NOEXCEPT { - return allocator_type{bucket.first()}; + return allocator_type{packed.second()}; } /** @@ -481,7 +453,7 @@ public: * @return Capacity of the storage. */ [[nodiscard]] size_type capacity() const ENTT_NOEXCEPT override { - return bucket.second() * packed_page_v; + return packed.first().size() * packed_page_v; } /*! @brief Requests the removal of unused capacity. */ @@ -495,12 +467,12 @@ public: * @return A pointer to the array of objects. */ [[nodiscard]] const_pointer raw() const ENTT_NOEXCEPT { - return packed; + return packed.first().data(); } /*! @copydoc raw */ [[nodiscard]] pointer raw() ENTT_NOEXCEPT { - return packed; + return packed.first().data(); } /** @@ -513,7 +485,7 @@ public: */ [[nodiscard]] const_iterator cbegin() const ENTT_NOEXCEPT { const auto pos = static_cast(base_type::size()); - return const_iterator{std::addressof(packed), pos}; + return const_iterator{&packed.first(), pos}; } /*! @copydoc cbegin */ @@ -524,7 +496,7 @@ public: /*! @copydoc begin */ [[nodiscard]] iterator begin() ENTT_NOEXCEPT { const auto pos = static_cast(base_type::size()); - return iterator{std::addressof(packed), pos}; + return iterator{&packed.first(), pos}; } /** @@ -538,7 +510,7 @@ public: * internal array. */ [[nodiscard]] const_iterator cend() const ENTT_NOEXCEPT { - return const_iterator{std::addressof(packed), {}}; + return const_iterator{&packed.first(), {}}; } /*! @copydoc cend */ @@ -548,7 +520,7 @@ public: /*! @copydoc end */ [[nodiscard]] iterator end() ENTT_NOEXCEPT { - return iterator{std::addressof(packed), {}}; + return iterator{&packed.first(), {}}; } /** @@ -653,7 +625,7 @@ public: template value_type &emplace(const entity_type entt, Args &&...args) { const auto pos = base_type::slot(); - alloc_pointer elem = assure_at_least(pos); + auto elem = assure_at_least(pos); construct(elem, std::forward(args)...); ENTT_TRY { @@ -661,7 +633,7 @@ public: ENTT_ASSERT(pos == base_type::index(entt), "Misplaced component"); } ENTT_CATCH { - std::destroy_at(std::addressof(element_at(pos))); + std::destroy_at(std::addressof(*elem)); ENTT_THROW; } @@ -719,8 +691,7 @@ public: } private: - compressed_pair bucket; - alloc_ptr_pointer packed; + compressed_pair packed; }; /*! @copydoc basic_storage */ diff --git a/test/entt/entity/storage.cpp b/test/entt/entity/storage.cpp index 940ca4fcd..5a8d17fc6 100644 --- a/test/entt/entity/storage.cpp +++ b/test/entt/entity/storage.cpp @@ -1326,34 +1326,32 @@ TEST(Storage, ThrowingAllocator) { test::throwing_allocator::trigger_on_allocate = true; - // strong exception safety ASSERT_THROW(pool.reserve(1u), test::throwing_allocator::exception_type); ASSERT_EQ(pool.capacity(), 0u); test::throwing_allocator::trigger_after_allocate = true; - // strong exception safety ASSERT_THROW(pool.reserve(2 * ENTT_PACKED_PAGE), test::throwing_allocator::exception_type); - ASSERT_EQ(pool.capacity(), 0u); + ASSERT_EQ(pool.capacity(), ENTT_PACKED_PAGE); pool.shrink_to_fit(); + + ASSERT_EQ(pool.capacity(), 0u); + test::throwing_allocator::trigger_on_allocate = true; - // strong exception safety ASSERT_THROW(pool.emplace(entt::entity{0}, 0), test::throwing_allocator::exception_type); ASSERT_FALSE(pool.contains(entt::entity{0})); ASSERT_TRUE(pool.empty()); test::throwing_allocator::trigger_on_allocate = true; - // strong exception safety ASSERT_THROW(pool.emplace(entt::entity{0}, 0), test::throwing_allocator::exception_type); ASSERT_FALSE(pool.contains(entt::entity{0})); ASSERT_TRUE(pool.empty()); test::throwing_allocator::trigger_on_allocate = true; - // strong exception safety ASSERT_THROW(base.emplace(entt::entity{0}), test::throwing_allocator::exception_type); ASSERT_FALSE(base.contains(entt::entity{0})); ASSERT_TRUE(base.empty()); @@ -1362,7 +1360,6 @@ TEST(Storage, ThrowingAllocator) { const entt::entity entities[2u]{entt::entity{1}, entt::entity{ENTT_SPARSE_PAGE}}; test::throwing_allocator::trigger_after_allocate = true; - // basic exception safety ASSERT_THROW(pool.insert(std::begin(entities), std::end(entities), 0), test::throwing_allocator::exception_type); ASSERT_TRUE(pool.contains(entt::entity{1})); ASSERT_FALSE(pool.contains(entt::entity{ENTT_SPARSE_PAGE})); @@ -1371,7 +1368,6 @@ TEST(Storage, ThrowingAllocator) { const int components[2u]{1, ENTT_SPARSE_PAGE}; test::throwing_allocator::trigger_on_allocate = true; - // basic exception safety ASSERT_THROW(pool.insert(std::begin(entities), std::end(entities), std::begin(components)), test::throwing_allocator::exception_type); ASSERT_TRUE(pool.contains(entt::entity{1})); ASSERT_FALSE(pool.contains(entt::entity{ENTT_SPARSE_PAGE}));