diff --git a/TODO b/TODO index e182994fe..34432af05 100644 --- a/TODO +++ b/TODO @@ -4,9 +4,8 @@ * add examples (and credits) from @alanjfs :) WIP: -* reintroduce swap-and-pop vs in-place-pop difference for perf reasons -* iterator based try_emplace vs try_insert for perf reasons * make clear check free_list, then set it to null and shrink +* iterator based try_emplace vs try_insert for perf reasons * runtime events (dispatcher/emitter), runtime context variables... * registry: remove reference to basic_sparse_set * dedicated entity storage, in-place O(1) release/destroy for non-orphaned entities, out-of-sync model diff --git a/src/entt/entity/sparse_set.hpp b/src/entt/entity/sparse_set.hpp index 6b9aebaf0..d3d436e72 100644 --- a/src/entt/entity/sparse_set.hpp +++ b/src/entt/entity/sparse_set.hpp @@ -234,26 +234,30 @@ protected: /** * @brief Erases entities from a sparse set. - * @param it Iterator to the first element to erase. - * @param count Number of elements to erase. + * @param first An iterator to the first element of the range of entities. + * @param last An iterator past the last element of the range of entities. */ - virtual void try_erase(basic_iterator it, const std::size_t count) { - if(mode == deletion_policy::in_place) { - for(const auto last = it + count; it != last; ++it) { - sparse_ref(*it) = null; - packed[it.index()] = std::exchange(free_list, entity_traits::combine(static_cast(it.index()), entity_traits::reserved)); - } - } else { - for(const auto last = it + count; it != last; ++it) { - auto &ref = sparse_ref(*it); - packed[it.index()] = packed.back(); - sparse_ref(packed.back()) = entity_traits::combine(static_cast(it.index()), entity_traits::to_integral(packed.back())); - // unnecessary but it helps to detect nasty bugs - ENTT_ASSERT((packed.back() = tombstone, true), ""); - packed.pop_back(); - // lazy self-assignment guard - ref = null; - } + virtual void swap_and_pop(basic_iterator first, basic_iterator last) { + for(; first != last; ++first) { + sparse_ref(packed.back()) = entity_traits::combine(static_cast(first.index()), entity_traits::to_integral(packed.back())); + const auto entt = std::exchange(packed[first.index()], packed.back()); + // unnecessary but it helps to detect nasty bugs + ENTT_ASSERT((packed.back() = tombstone, true), ""); + // lazy self-assignment guard + sparse_ref(entt) = null; + packed.pop_back(); + } + } + + /** + * @brief Erases entities from a sparse set. + * @param first An iterator to the first element of the range of entities. + * @param last An iterator past the last element of the range of entities. + */ + virtual void in_place_pop(basic_iterator first, basic_iterator last) { + for(; first != last; ++first) { + sparse_ref(*first) = null; + packed[first.index()] = std::exchange(free_list, entity_traits::combine(static_cast(first.index()), entity_traits::reserved)); } } @@ -688,7 +692,8 @@ public: * @param entt A valid identifier. */ void erase(const entity_type entt) { - try_erase(--(end() - index(entt)), 1u); + const auto it = --(end() - index(entt)); + (mode == deletion_policy::in_place) ? in_place_pop(it, it + 1u) : swap_and_pop(it, it + 1u); } /** @@ -703,7 +708,7 @@ public: template void erase(It first, It last) { if constexpr(std::is_same_v) { - try_erase(first, static_cast(std::distance(first, last))); + (mode == deletion_policy::in_place) ? in_place_pop(first, last) : swap_and_pop(first, last); } else { for(; first != last; ++first) { erase(*first); @@ -895,7 +900,7 @@ public: /*! @brief Clears a sparse set. */ void clear() { if(free_list == null) { - try_erase(begin(), size()); + erase(begin(), end()); } else { for(auto &&entity: *this) { // tombstone filter diff --git a/src/entt/entity/storage.hpp b/src/entt/entity/storage.hpp index cc9cc0f78..cee59a019 100644 --- a/src/entt/entity/storage.hpp +++ b/src/entt/entity/storage.hpp @@ -277,7 +277,12 @@ class basic_storage: public basic_sparse_set(args)...); } ENTT_CATCH { - base_type::try_erase(it, 1u); + if constexpr(comp_traits::in_place_delete) { + base_type::in_place_pop(it, it + 1u); + } else { + base_type::swap_and_pop(it, it + 1u); + } + ENTT_THROW; } @@ -324,16 +329,28 @@ private: protected: /** * @brief Erases elements from a storage. - * @param it Iterator to the first element to erase. - * @param count Number of elements to erase. + * @param first An iterator to the first element to erase. + * @param last An iterator past the last element to erase. */ - void try_erase(typename underlying_type::basic_iterator it, const std::size_t count) override { - for(const auto last = it + count; it != last; ++it) { - const auto pos = static_cast(it.index()); - auto &elem = element_at(comp_traits::in_place_delete ? pos : (base_type::size() - 1u)); - [[maybe_unused]] auto unused = std::exchange(element_at(pos), std::move(elem)); + void swap_and_pop(typename underlying_type::basic_iterator first, typename underlying_type::basic_iterator last) override { + for(; first != last; ++first) { + auto &elem = element_at(base_type::size() - 1u); + // destroying on exit allows reentrant destructors + [[maybe_unused]] auto unused = std::exchange(element_at(static_cast(first.index())), std::move(elem)); std::destroy_at(std::addressof(elem)); - base_type::try_erase(it, 1u); + base_type::swap_and_pop(first, first + 1u); + } + } + + /** + * @brief Erases elements from a storage. + * @param first An iterator to the first element to erase. + * @param last An iterator past the last element to erase. + */ + void in_place_pop(typename underlying_type::basic_iterator first, typename underlying_type::basic_iterator last) override { + for(; first != last; ++first) { + base_type::in_place_pop(first, first + 1u); + std::destroy_at(std::addressof(element_at(static_cast(first.index())))); } } @@ -893,16 +910,26 @@ public: */ template class sigh_storage_mixin final: public Type { - void try_erase(typename Type::basic_iterator it, const std::size_t count) override { + template + void notify_destruction(typename Type::basic_iterator first, typename Type::basic_iterator last, Func func) { ENTT_ASSERT(owner != nullptr, "Invalid pointer to registry"); - for(const auto last = it + count; it != last; ++it) { - const auto entt = *it; + for(; first != last; ++first) { + const auto entt = *first; destruction.publish(*owner, entt); - Type::try_erase(Type::find(entt), 1u); + const auto it = Type::find(entt); + func(it, it + 1u); } } + void swap_and_pop(typename Type::basic_iterator first, typename Type::basic_iterator last) final { + notify_destruction(std::move(first), std::move(last), [this](auto... args) { Type::swap_and_pop(args...); }); + } + + void in_place_pop(typename Type::basic_iterator first, typename Type::basic_iterator last) final { + notify_destruction(std::move(first), std::move(last), [this](auto... args) { Type::in_place_pop(args...); }); + } + typename Type::basic_iterator try_emplace(const typename Type::entity_type entt, const bool force_back, const void *value) final { ENTT_ASSERT(owner != nullptr, "Invalid pointer to registry"); Type::try_emplace(entt, force_back, value); diff --git a/test/entt/entity/sigh_storage_mixin.cpp b/test/entt/entity/sigh_storage_mixin.cpp index 04f0fabbd..1c35f2032 100644 --- a/test/entt/entity/sigh_storage_mixin.cpp +++ b/test/entt/entity/sigh_storage_mixin.cpp @@ -8,7 +8,7 @@ struct empty_type {}; struct stable_type { - int value; + int value{}; }; struct non_default_constructible { @@ -17,7 +17,7 @@ struct non_default_constructible { non_default_constructible(int v) : value{v} {} - int value; + int value{}; }; template<> @@ -34,9 +34,9 @@ void listener(counter &counter, entt::registry &, entt::entity) { } TEST(SighStorageMixin, GenericType) { + entt::entity entities[2u]{entt::entity{3}, entt::entity{42}}; entt::sigh_storage_mixin> pool; entt::sparse_set &base = pool; - entt::entity entities[2u]{entt::entity{3}, entt::entity{42}}; entt::registry registry{}; pool.bind(entt::forward_as_any(registry)); @@ -99,10 +99,76 @@ TEST(SighStorageMixin, GenericType) { ASSERT_TRUE(pool.empty()); } +TEST(SighStorageMixin, StableType) { + entt::entity entities[2u]{entt::entity{3}, entt::entity{42}}; + entt::sigh_storage_mixin> pool; + entt::sparse_set &base = pool; + entt::registry registry{}; + + pool.bind(entt::forward_as_any(registry)); + + counter on_construct{}; + counter on_destroy{}; + + pool.on_construct().connect<&listener>(on_construct); + pool.on_destroy().connect<&listener>(on_destroy); + + ASSERT_NE(base.emplace(entities[0u]), base.end()); + + pool.emplace(entities[1u]); + + ASSERT_EQ(on_construct.value, 2); + ASSERT_EQ(on_destroy.value, 0); + ASSERT_FALSE(pool.empty()); + + ASSERT_EQ(pool.get(entities[0u]).value, 0); + ASSERT_EQ(pool.get(entities[1u]).value, 0); + + base.erase(entities[0u]); + pool.erase(entities[1u]); + + ASSERT_EQ(on_construct.value, 2); + ASSERT_EQ(on_destroy.value, 2); + ASSERT_FALSE(pool.empty()); + + ASSERT_NE(base.insert(std::begin(entities), std::end(entities)), base.end()); + + ASSERT_EQ(pool.get(entities[0u]).value, 0); + ASSERT_EQ(pool.get(entities[1u]).value, 0); + ASSERT_FALSE(pool.empty()); + + base.erase(entities[1u]); + + ASSERT_EQ(on_construct.value, 4); + ASSERT_EQ(on_destroy.value, 3); + ASSERT_FALSE(pool.empty()); + + base.erase(entities[0u]); + + ASSERT_EQ(on_construct.value, 4); + ASSERT_EQ(on_destroy.value, 4); + ASSERT_FALSE(pool.empty()); + + pool.insert(std::begin(entities), std::end(entities), stable_type{3}); + + ASSERT_EQ(on_construct.value, 6); + ASSERT_EQ(on_destroy.value, 4); + ASSERT_FALSE(pool.empty()); + + ASSERT_EQ(pool.get(entities[0u]).value, 3); + ASSERT_EQ(pool.get(entities[1u]).value, 3); + + pool.erase(std::begin(entities), std::end(entities)); + + ASSERT_EQ(on_construct.value, 6); + ASSERT_EQ(on_destroy.value, 6); + ASSERT_FALSE(pool.empty()); +} + TEST(SighStorageMixin, EmptyType) { + entt::entity entities[2u]{entt::entity{3}, entt::entity{42}}; entt::sigh_storage_mixin> pool; entt::sparse_set &base = pool; - entt::entity entities[2u]{entt::entity{3}, entt::entity{42}}; entt::registry registry{}; pool.bind(entt::forward_as_any(registry)); @@ -166,9 +232,9 @@ TEST(SighStorageMixin, EmptyType) { } TEST(SighStorageMixin, NonDefaultConstructibleType) { + entt::entity entities[2u]{entt::entity{3}, entt::entity{42}}; entt::sigh_storage_mixin> pool; entt::sparse_set &base = pool; - entt::entity entities[2u]{entt::entity{3}, entt::entity{42}}; entt::registry registry{}; pool.bind(entt::forward_as_any(registry)); diff --git a/test/entt/entity/storage.cpp b/test/entt/entity/storage.cpp index 91104fe06..888b1495d 100644 --- a/test/entt/entity/storage.cpp +++ b/test/entt/entity/storage.cpp @@ -1658,70 +1658,80 @@ TEST(Storage, CustomAllocator) { pool.clear(); ASSERT_NE(pool.capacity(), 0u); - ASSERT_EQ(pool.size(), 0u); + ASSERT_EQ(pool.size(), pool.policy() == entt::deletion_policy::in_place ? 2u : 0u); pool.shrink_to_fit(); - ASSERT_EQ(pool.capacity(), 0u); + ASSERT_EQ(pool.capacity(), pool.policy() == entt::deletion_policy::in_place ? ENTT_PACKED_PAGE : 0u); }; test::throwing_allocator allocator{}; test(entt::basic_storage>{allocator}, allocator); test(entt::basic_storage>{allocator}, allocator); + test(entt::basic_storage>{allocator}, allocator); } TEST(Storage, ThrowingAllocator) { - entt::basic_storage> pool; - typename std::decay_t::base_type &base = pool; + auto test = [](auto pool) { + using pool_allocator_type = typename decltype(pool)::allocator_type; + using value_type = typename decltype(pool)::value_type; - test::throwing_allocator::trigger_on_allocate = true; + typename std::decay_t::base_type &base = pool; - ASSERT_THROW(pool.reserve(1u), test::throwing_allocator::exception_type); - ASSERT_EQ(pool.capacity(), 0u); + pool_allocator_type::trigger_on_allocate = true; - test::throwing_allocator::trigger_after_allocate = true; + ASSERT_THROW(pool.reserve(1u), typename pool_allocator_type::exception_type); + ASSERT_EQ(pool.capacity(), 0u); - ASSERT_THROW(pool.reserve(2 * ENTT_PACKED_PAGE), test::throwing_allocator::exception_type); - ASSERT_EQ(pool.capacity(), ENTT_PACKED_PAGE); + pool_allocator_type::trigger_after_allocate = true; - pool.shrink_to_fit(); + ASSERT_THROW(pool.reserve(2 * ENTT_PACKED_PAGE), typename pool_allocator_type::exception_type); + ASSERT_EQ(pool.capacity(), ENTT_PACKED_PAGE); - ASSERT_EQ(pool.capacity(), 0u); + pool.shrink_to_fit(); - test::throwing_allocator::trigger_on_allocate = true; + ASSERT_EQ(pool.capacity(), 0u); - 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; - test::throwing_allocator::trigger_on_allocate = true; + ASSERT_THROW(pool.emplace(entt::entity{0}, 0), test::throwing_allocator::exception_type); + ASSERT_FALSE(pool.contains(entt::entity{0})); + ASSERT_TRUE(pool.empty()); - ASSERT_THROW(base.emplace(entt::entity{0}), test::throwing_allocator::exception_type); - ASSERT_FALSE(base.contains(entt::entity{0})); - ASSERT_TRUE(base.empty()); + test::throwing_allocator::trigger_on_allocate = true; - test::throwing_allocator::trigger_on_allocate = true; + ASSERT_THROW(base.emplace(entt::entity{0}), test::throwing_allocator::exception_type); + ASSERT_FALSE(base.contains(entt::entity{0})); + ASSERT_TRUE(base.empty()); - ASSERT_THROW(pool.emplace(entt::entity{0}, 0), test::throwing_allocator::exception_type); - ASSERT_FALSE(pool.contains(entt::entity{0})); - ASSERT_TRUE(pool.empty()); + pool_allocator_type::trigger_on_allocate = true; - pool.emplace(entt::entity{0}, 0); - const entt::entity entities[2u]{entt::entity{1}, entt::entity{ENTT_SPARSE_PAGE}}; - test::throwing_allocator::trigger_after_allocate = true; + ASSERT_THROW(pool.emplace(entt::entity{0}, 0), pool_allocator_type::exception_type); + ASSERT_FALSE(pool.contains(entt::entity{0})); + ASSERT_NO_THROW(pool.compact()); + ASSERT_TRUE(pool.empty()); - 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})); + pool.emplace(entt::entity{0}, 0); + const entt::entity entities[2u]{entt::entity{1}, entt::entity{ENTT_SPARSE_PAGE}}; + test::throwing_allocator::trigger_after_allocate = true; - pool.erase(entt::entity{1}); - const int components[2u]{1, ENTT_SPARSE_PAGE}; - test::throwing_allocator::trigger_on_allocate = true; + ASSERT_THROW(pool.insert(std::begin(entities), std::end(entities), value_type{0}), test::throwing_allocator::exception_type); + ASSERT_TRUE(pool.contains(entt::entity{1})); + ASSERT_FALSE(pool.contains(entt::entity{ENTT_SPARSE_PAGE})); - 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})); + pool.erase(entt::entity{1}); + const value_type components[2u]{value_type{1}, value_type{ENTT_SPARSE_PAGE}}; + test::throwing_allocator::trigger_on_allocate = true; + pool.compact(); + + 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})); + }; + + test(entt::basic_storage>{}); + test(entt::basic_storage>{}); } TEST(Storage, ThrowingComponent) {