sparse_set/storage (close #707):

* improved perf on erase/remove (and therefore also registry::destroy)
* allow to inhibit a pop from derived classes
* removed about_to_pop (virtual) function

src/entt/entity/sparse_set.hpp

@@ -224,50 +224,6 @@ class basic_sparse_set {
         }
     }
 
-    void stable_erase(const Entity entt, void *ud = nullptr) {
-        // last chance to use the entity for derived classes and mixins, if any
-        about_to_pop(entt, ud);
-
-        auto &ref = sparse[page(entt)][offset(entt)];
-        const auto pos = size_type{traits_type::to_entity(ref)};
-        ENTT_ASSERT(packed[pos] == entt, "Invalid entity identifier");
-
-        packed[pos] = std::exchange(free_list, traits_type::construct(static_cast<typename traits_type::entity_type>(pos)));
-        ref = null;
-
-        // strong exception guarantee
-        in_place_pop(pos);
-    }
-
-    void unstable_erase(const Entity entt, void *ud = nullptr) {
-        // last chance to use the entity for derived classes and mixins, if any
-        about_to_pop(entt, ud);
-
-        auto &ref = sparse[page(entt)][offset(entt)];
-        const auto pos = size_type{traits_type::to_entity(ref)};
-        ENTT_ASSERT(packed[pos] == entt, "Invalid entity identifier");
-
-        auto &last = packed[count - 1u];
-
-        packed[pos] = last;
-        sparse[page(last)][offset(last)] = ref;
-        // lazy self-assignment guard
-        ref = null;
-        // unnecessary but it helps to detect nasty bugs
-        ENTT_ASSERT((last = tombstone, true), "");
-
-        --count;
-
-        ENTT_TRY {
-            // strong exception guarantee
-            swap_and_pop(pos);
-        } ENTT_CATCH {
-            last = std::exchange(packed[pos], entt);
-            ref = std::exchange(sparse[page(last)][offset(last)], traits_type::construct(static_cast<typename traits_type::entity_type>(count++)));
-            ENTT_THROW;
-        }
-    }
-
 protected:
     /**
      * @brief Swaps two entities in the internal packed array.
@@ -277,7 +233,7 @@ protected:
     virtual void swap_at([[maybe_unused]] const std::size_t lhs, [[maybe_unused]] const std::size_t rhs) {}
 
     /**
-     * @brief Attempts to move an entity in the internal packed array.
+     * @brief Moves an entity in the internal packed array.
      * @param from A valid position of an entity within storage.
      * @param to A valid position of an entity within storage.
      */
@@ -285,22 +241,37 @@ protected:
 
     /**
      * @brief Attempts to erase an entity from the internal packed array.
-     * @param pos A valid position of an entity within storage.
+     * @param entt A valid entity identifier.
+     * @param ud Optional user data that are forwarded as-is to derived classes.
      */
-    virtual void swap_and_pop([[maybe_unused]] const std::size_t pos) {}
+    virtual void swap_and_pop(const Entity entt, [[maybe_unused]] void *ud) {
+        auto &ref = sparse[page(entt)][offset(entt)];
+        const auto pos = size_type{traits_type::to_entity(ref)};
+        ENTT_ASSERT(packed[pos] == entt, "Invalid entity identifier");
+        auto &last = packed[--count];
 
-    /**
-     * @brief Attempts to erase an entity from the internal packed array.
-     * @param pos A valid position of an entity within storage.
-     */
-    virtual void in_place_pop([[maybe_unused]] const std::size_t pos) {}
+        packed[pos] = last;
+        sparse[page(last)][offset(last)] = ref;
+        // lazy self-assignment guard
+        ref = null;
+        // unnecessary but it helps to detect nasty bugs
+        ENTT_ASSERT((last = tombstone, true), "");
+    }
 
     /**
-     * @brief Last chance to use an entity that is about to be erased.
-     * @param entity A valid entity identifier.
+     * @brief Attempts to erase an entity from the internal packed array.
+     * @param entt A valid entity identifier.
      * @param ud Optional user data that are forwarded as-is to derived classes.
      */
-    virtual void about_to_pop([[maybe_unused]] const Entity entity, [[maybe_unused]] void *ud) {}
+    virtual void in_place_pop(const Entity entt, [[maybe_unused]] void *ud) {
+        auto &ref = sparse[page(entt)][offset(entt)];
+        const auto pos = size_type{traits_type::to_entity(ref)};
+        ENTT_ASSERT(packed[pos] == entt, "Invalid entity identifier");
+
+        packed[pos] = std::exchange(free_list, traits_type::construct(static_cast<typename traits_type::entity_type>(pos)));
+        // lazy self-assignment guard
+        ref = null;
+    }
 
 public:
     /*! @brief Allocator type. */
@@ -638,7 +609,7 @@ public:
      */
     void erase(const entity_type entt, void *ud = nullptr) {
         ENTT_ASSERT(contains(entt), "Set does not contain entity");
-        (mode == deletion_policy::in_place) ? stable_erase(entt, ud) : unstable_erase(entt, ud);
+        (mode == deletion_policy::in_place) ? in_place_pop(entt, ud) : swap_and_pop(entt, ud);
     }
 
     /**
@@ -721,12 +692,15 @@ public:
      * @param rhs A valid entity identifier.
      */
     void swap(const entity_type lhs, const entity_type rhs) {
-        const auto from = index(lhs);
-        const auto to = index(rhs);
+        auto &entt = sparse[page(lhs)][offset(lhs)];
+        auto &other = sparse[page(rhs)][offset(rhs)];
+
+        const auto from = size_type{traits_type::to_entity(entt)};
+        const auto to = size_type{traits_type::to_entity(other)};
 
         // basic no-leak guarantee (with invalid state) if swapping throws
         swap_at(from, to);
-        std::swap(sparse[page(lhs)][offset(lhs)], sparse[page(rhs)][offset(rhs)]);
+        std::swap(entt, other);
         std::swap(packed[from], packed[to]);
     }
 
@@ -840,7 +814,7 @@ public:
     void clear(void *ud = nullptr) {
         for(auto &&entity: *this) {
             if(entity != tombstone) {
-                stable_erase(entity, ud);
+                in_place_pop(entity, ud);
             }
         }
 

src/entt/entity/storage.hpp

@@ -265,20 +265,26 @@ protected:
     }
 
     /*! @copydoc basic_sparse_set::swap_and_pop */
-    void swap_and_pop(const std::size_t pos) final {
-        const auto length = underlying_type::size();
+    void swap_and_pop(const Entity entt, void *ud) {
+        const auto pos = underlying_type::index(entt);
+        const auto length = underlying_type::size() - 1u;
+
         auto &&elem = packed[page(pos)][offset(pos)];
         auto &&last = packed[page(length)][offset(length)];
 
         // support for nosy destructors
         [[maybe_unused]] auto unused = std::move(elem);
         elem = std::move(last);
+
         alloc_traits::destroy(allocator, std::addressof(last));
+        underlying_type::swap_and_pop(entt, ud);
     }
 
     /*! @copydoc basic_sparse_set::in_place_pop */
-    void in_place_pop(const std::size_t pos) final {
+    void in_place_pop(const Entity entt, void *ud) {
+        const auto pos = underlying_type::index(entt);
         alloc_traits::destroy(allocator, std::addressof(packed[page(pos)][offset(pos)]));
+        underlying_type::in_place_pop(entt, ud);
     }
 
 public:
@@ -543,13 +549,13 @@ public:
     /**
      * @brief Updates the instance assigned to a given entity in-place.
      * @tparam Func Types of the function objects to invoke.
-     * @param entity A valid entity identifier.
+     * @param entt A valid entity identifier.
      * @param func Valid function objects.
      * @return A reference to the updated instance.
      */
     template<typename... Func>
-    decltype(auto) patch(const entity_type entity, Func &&... func) {
-        const auto idx = underlying_type::index(entity);
+    decltype(auto) patch(const entity_type entt, Func &&... func) {
+        const auto idx = underlying_type::index(entt);
         auto &&elem = packed[page(idx)][offset(idx)];
         (std::forward<Func>(func)(elem), ...);
         return elem;
@@ -748,12 +754,12 @@ public:
     /**
     * @brief Updates the instance assigned to a given entity in-place.
     * @tparam Func Types of the function objects to invoke.
-    * @param entity A valid entity identifier.
+    * @param entt A valid entity identifier.
     * @param func Valid function objects.
     */
     template<typename... Func>
-    void patch([[maybe_unused]] const entity_type entity, Func &&... func) {
-        ENTT_ASSERT(underlying_type::contains(entity), "Storage does not contain entity");
+    void patch([[maybe_unused]] const entity_type entt, Func &&... func) {
+        ENTT_ASSERT(underlying_type::contains(entt), "Storage does not contain entity");
         (std::forward<Func>(func)(), ...);
     }
 
@@ -794,13 +800,13 @@ struct storage_adapter_mixin: Type {
     /**
      * @brief Assigns entities to a storage.
      * @tparam Args Types of arguments to use to construct the object.
-     * @param entity A valid entity identifier.
+     * @param entt A valid entity identifier.
      * @param args Parameters to use to initialize the object.
      * @return A reference to the newly created object.
      */
     template<typename... Args>
-    decltype(auto) emplace(basic_registry<entity_type> &, const entity_type entity, Args &&... args) {
-        return Type::emplace(entity, std::forward<Args>(args)...);
+    decltype(auto) emplace(basic_registry<entity_type> &, const entity_type entt, Args &&... args) {
+        return Type::emplace(entt, std::forward<Args>(args)...);
     }
 
     /**
@@ -821,13 +827,13 @@ struct storage_adapter_mixin: Type {
     /**
      * @brief Patches the given instance for an entity.
      * @tparam Func Types of the function objects to invoke.
-     * @param entity A valid entity identifier.
+     * @param entt A valid entity identifier.
      * @param func Valid function objects.
      * @return A reference to the patched instance.
      */
     template<typename... Func>
-    decltype(auto) patch(basic_registry<entity_type> &, const entity_type entity, Func &&... func) {
-        return Type::patch(entity, std::forward<Func>(func)...);
+    decltype(auto) patch(basic_registry<entity_type> &, const entity_type entt, Func &&... func) {
+        return Type::patch(entt, std::forward<Func>(func)...);
     }
 };
 
@@ -838,11 +844,18 @@ struct storage_adapter_mixin: Type {
  */
 template<typename Type>
 class sigh_storage_mixin final: public Type {
-    /*! @copydoc basic_sparse_set::about_to_pop */
-    void about_to_pop(const typename Type::entity_type entity, void *ud) final {
+    /*! @copydoc basic_sparse_set::swap_and_pop */
+    void swap_and_pop(const typename Type::entity_type entt, void *ud) final {
         ENTT_ASSERT(ud != nullptr, "Invalid pointer to registry");
-        destruction.publish(*static_cast<basic_registry<typename Type::entity_type> *>(ud), entity);
-        Type::about_to_pop(entity, ud);
+        destruction.publish(*static_cast<basic_registry<typename Type::entity_type> *>(ud), entt);
+        Type::swap_and_pop(entt, ud);
+    }
+
+    /*! @copydoc basic_sparse_set::in_place_pop */
+    void in_place_pop(const typename Type::entity_type entt, void *ud) final {
+        ENTT_ASSERT(ud != nullptr, "Invalid pointer to registry");
+        destruction.publish(*static_cast<basic_registry<typename Type::entity_type> *>(ud), entt);
+        Type::in_place_pop(entt, ud);
     }
 
 public:
@@ -923,15 +936,15 @@ public:
      * @brief Assigns entities to a storage.
      * @tparam Args Types of arguments to use to construct the object.
      * @param owner The registry that issued the request.
-     * @param entity A valid entity identifier.
+     * @param entt A valid entity identifier.
      * @param args Parameters to use to initialize the object.
      * @return A reference to the newly created object.
      */
     template<typename... Args>
-    decltype(auto) emplace(basic_registry<entity_type> &owner, const entity_type entity, Args &&... args) {
-        Type::emplace(entity, std::forward<Args>(args)...);
-        construction.publish(owner, entity);
-        return this->get(entity);
+    decltype(auto) emplace(basic_registry<entity_type> &owner, const entity_type entt, Args &&... args) {
+        Type::emplace(entt, std::forward<Args>(args)...);
+        construction.publish(owner, entt);
+        return this->get(entt);
     }
 
     /**
@@ -960,15 +973,15 @@ public:
      * @brief Patches the given instance for an entity.
      * @tparam Func Types of the function objects to invoke.
      * @param owner The registry that issued the request.
-     * @param entity A valid entity identifier.
+     * @param entt A valid entity identifier.
      * @param func Valid function objects.
      * @return A reference to the patched instance.
      */
     template<typename... Func>
-    decltype(auto) patch(basic_registry<entity_type> &owner, const entity_type entity, Func &&... func) {
-        Type::patch(entity, std::forward<Func>(func)...);
-        update.publish(owner, entity);
-        return this->get(entity);
+    decltype(auto) patch(basic_registry<entity_type> &owner, const entity_type entt, Func &&... func) {
+        Type::patch(entt, std::forward<Func>(func)...);
+        update.publish(owner, entt);
+        return this->get(entt);
     }
 
 private:
@@ -1006,17 +1019,17 @@ struct storage_traits {
  * @brief Gets the element assigned to an entity from a storage, if any.
  * @tparam Type Storage type.
  * @param container A valid instance of a storage class.
- * @param entity A valid entity identifier.
+ * @param entt A valid entity identifier.
  * @return A possibly empty tuple containing the requested element.
  */
 template<typename Type>
-[[nodiscard]] auto get_as_tuple([[maybe_unused]] Type &container, [[maybe_unused]] const typename Type::entity_type entity) {
+[[nodiscard]] auto get_as_tuple([[maybe_unused]] Type &container, [[maybe_unused]] const typename Type::entity_type entt) {
     static_assert(std::is_same_v<std::remove_const_t<Type>, typename storage_traits<typename Type::entity_type, typename Type::value_type>::storage_type>, "Invalid storage");
 
     if constexpr(std::is_void_v<decltype(container.get({}))>) {
         return std::make_tuple();
     } else {
-        return std::forward_as_tuple(container.get(entity));
+        return std::forward_as_tuple(container.get(entt));
     }
 }