sparse_set/storage: small review and optimizations

src/entt/entity/sparse_set.hpp

@@ -191,34 +191,57 @@ class basic_sparse_set {
         return sparse[idx];
     }
 
-    void resize_packed(const std::size_t req) {
-        ENTT_ASSERT(req && !(req < count), "Invalid request");
-        auto old = std::exchange(packed, alloc_traits::allocate(allocator, req));
+    void maybe_release_pages() {
+        if(!count && bucket) {
+            for(size_type pos{}; pos < bucket; ++pos) {
+                if(sparse[pos]) {
+                    std::destroy(sparse[pos], sparse[pos] + page_size);
+                    alloc_traits::deallocate(allocator, sparse[pos], page_size);
+                }
 
-        if(const auto length = std::exchange(reserved, req); length) {
-            for(size_type pos{}; pos < count; ++pos) {
-                alloc_traits::construct(allocator, std::addressof(packed[pos]), std::move(old[pos]));
-                alloc_traits::destroy(allocator, std::addressof(old[pos]));
+                bucket_alloc_traits::destroy(bucket_allocator, std::addressof(sparse[pos]));
             }
 
-            alloc_traits::deallocate(allocator, old, length);
+            bucket_alloc_traits::deallocate(bucket_allocator, sparse, std::exchange(bucket, 0u));
         }
     }
 
-    template<typename It>
-    void push_back(It first, It last) {
-        if(const size_type req = count + std::distance(first, last); reserved < req) {
-            const size_type sz = size_type(reserved * growth_factor) + (reserved == 0u);
-            resize_packed(sz < req ? req : sz);
+    [[nodiscard]] std::size_t packed_size_for(const std::size_t req) {
+        auto sz = reserved;
+
+        while(req > sz) {
+            const size_type next(sz * growth_factor);
+            sz = next + !(next > sz);
         }
 
-        for(; first != last; ++first) {
-            ENTT_ASSERT(!contains(*first), "Set already contains entity");
-            assure_page(page(*first))[offset(*first)] = entity_type{static_cast<typename traits_type::entity_type>(count)};
-            alloc_traits::construct(allocator, std::addressof(packed[count++]), *first);
+        return sz;
+    }
+
+    void maybe_resize_packed(const std::size_t req) {
+        if(const auto length = std::exchange(reserved, req); !reserved) {
+            if(length) {
+                std::destroy(packed, packed + std::exchange(count, 0u));
+                alloc_traits::deallocate(allocator, packed, length);
+            }
+        } else if(reserved != length) {
+            ENTT_ASSERT(!(req < count), "Invalid request");
+            auto old = std::exchange(packed, alloc_traits::allocate(allocator, req));
+
+            for(size_type pos{}; pos < count; ++pos) {
+                alloc_traits::construct(allocator, std::addressof(packed[pos]), std::move(old[pos]));
+                alloc_traits::destroy(allocator, std::addressof(old[pos]));
+            }
+            
+            alloc_traits::deallocate(allocator, old, length);
         }
     }
 
+    void push_back(const Entity entt) {
+        ENTT_ASSERT(count != reserved, "No more space left");
+        assure_page(page(entt))[offset(entt)] = entity_type{static_cast<typename traits_type::entity_type>(count)};
+        alloc_traits::construct(allocator, std::addressof(packed[count++]), entt);
+    }
+
     void pop(const Entity entt, void *ud) {
         ENTT_ASSERT(contains(entt), "Set does not contain entity");
         // last chance to use the entity for derived classes and mixins, if any
@@ -238,26 +261,6 @@ class basic_sparse_set {
         swap_and_pop(pos);
     }
 
-    void reset_to_empty() {
-        if(const auto length = std::exchange(reserved, 0u); length) {
-            std::destroy(packed, packed + std::exchange(count, 0u));
-            alloc_traits::deallocate(allocator, packed, length);
-        }
-
-        if(const auto length = std::exchange(bucket, 0u); length) {
-            for(size_type pos{}; pos < length; ++pos) {
-                if(sparse[pos]) {
-                    std::destroy(sparse[pos], sparse[pos] + page_size);
-                    alloc_traits::deallocate(allocator, sparse[pos], page_size);
-                }
-
-                bucket_alloc_traits::destroy(bucket_allocator, std::addressof(sparse[pos]));
-            }
-
-            bucket_alloc_traits::deallocate(bucket_allocator, sparse, std::exchange(bucket, 0u));
-        }
-    }
-
 protected:
     /**
      * @brief Swaps two entities in the internal packed array.
@@ -323,7 +326,8 @@ public:
 
     /*! @brief Default destructor. */
     virtual ~basic_sparse_set() {
-        reset_to_empty();
+        maybe_resize_packed(0u);
+        maybe_release_pages();
     }
 
     /**
@@ -352,7 +356,7 @@ public:
      */
     void reserve(const size_type cap) {
         if(cap > reserved) {
-            resize_packed(cap);
+            maybe_resize_packed(cap);
         }
     }
 
@@ -367,11 +371,8 @@ public:
 
     /*! @brief Requests the removal of unused capacity. */
     void shrink_to_fit() {
-        if(!count) {
-            reset_to_empty();
-        } else if(reserved > count) {
-            resize_packed(count);
-        }
+        maybe_resize_packed(count);
+        maybe_release_pages();
     }
 
     /**
@@ -538,8 +539,9 @@ public:
      * @param entt A valid entity identifier.
      */
     void emplace(const entity_type entt) {
-        entity_type arr[1u]{entt};
-        push_back(arr, arr + 1u);
+        ENTT_ASSERT(!contains(entt), "Set already contains entity");
+        maybe_resize_packed(packed_size_for(count + 1u));
+        push_back(entt);
     }
 
     /**
@@ -555,7 +557,12 @@ public:
      */
     template<typename It>
     void insert(It first, It last) {
-        push_back(first, last);
+        maybe_resize_packed(packed_size_for(count + std::distance(first, last)));
+
+        for(; first != last; ++first) {
+            ENTT_ASSERT(!contains(*first), "Set already contains entity");
+            push_back(*first);
+        }
     }
 
     /**

src/entt/entity/storage.hpp

@@ -180,21 +180,31 @@ class basic_storage: public basic_sparse_set<Entity, typename std::allocator_tra
     }
 
     void maybe_resize_packed(const std::size_t req) {
-        ENTT_ASSERT(req && !(req < count), "Invalid request");
+        if(const auto length = std::exchange(bucket, (req + page_size - 1u) / page_size); !bucket) {
+            for(size_type pos{}; pos < length; ++pos) {
+                if(count) {
+                    const auto sz = count > page_size ? page_size : count;
+                    std::destroy(packed[pos], packed[pos] + sz);
+                    count -= sz;
+                }
+            
+                alloc_traits::deallocate(allocator, packed[pos], page_size);
+                bucket_alloc_traits::destroy(bucket_allocator, std::addressof(packed[pos]));
+            }
 
-        if(const auto length = std::exchange(bucket, (req + page_size - 1u) / page_size); bucket != length) {
+            if(length) {
+                bucket_alloc_traits::deallocate(bucket_allocator, packed, length);
+            }
+        } else if(bucket != length) {
+            ENTT_ASSERT(!(req < count), "Invalid request");
             const auto old = std::exchange(packed, bucket_alloc_traits::allocate(bucket_allocator, bucket));
 
             if(bucket > length) {
-                if(length) {
-                    for(size_type pos{}; pos < length; ++pos) {
-                        bucket_alloc_traits::construct(bucket_allocator, std::addressof(packed[pos]), old[pos]);
-                        bucket_alloc_traits::destroy(bucket_allocator, std::addressof(old[pos]));
-                    }
-
-                    bucket_alloc_traits::deallocate(bucket_allocator, old, length);
+                for(size_type pos{}; pos < length; ++pos) {
+                    bucket_alloc_traits::construct(bucket_allocator, std::addressof(packed[pos]), old[pos]);
+                    bucket_alloc_traits::destroy(bucket_allocator, std::addressof(old[pos]));
                 }
-
+    
                 for(auto pos = length; pos < bucket; ++pos) {
                     bucket_alloc_traits::construct(bucket_allocator, std::addressof(packed[pos]), alloc_traits::allocate(allocator, page_size));
                 }
@@ -203,12 +213,14 @@ class basic_storage: public basic_sparse_set<Entity, typename std::allocator_tra
                     bucket_alloc_traits::construct(bucket_allocator, std::addressof(packed[pos]), old[pos]);
                     bucket_alloc_traits::destroy(bucket_allocator, std::addressof(old[pos]));
                 }
-
+    
                 for(auto pos = bucket; pos < length; ++pos) {
                     alloc_traits::deallocate(allocator, old[pos], page_size);
                     bucket_alloc_traits::destroy(bucket_allocator, std::addressof(old[pos]));
                 }
+            }
 
+            if(length) {
                 bucket_alloc_traits::deallocate(bucket_allocator, old, length);
             }
         }
@@ -228,23 +240,6 @@ class basic_storage: public basic_sparse_set<Entity, typename std::allocator_tra
         return ref;
     }
 
-    void reset_to_empty() {
-        if(const auto length = std::exchange(bucket, 0u); length) {
-            for(size_type pos{}; pos < length; ++pos) {
-                if(count) {
-                    const auto sz = count > page_size ? page_size : count;
-                    std::destroy(packed[pos], packed[pos] + sz);
-                    count -= sz;
-                }
-
-                alloc_traits::deallocate(allocator, packed[pos], page_size);
-                bucket_alloc_traits::destroy(bucket_allocator, std::addressof(packed[pos]));
-            }
-
-            bucket_alloc_traits::deallocate(bucket_allocator, packed, length);
-        }
-    }
-
 protected:
     /*! @copydoc basic_sparse_set::swap_at */
     void swap_at(const std::size_t lhs, const std::size_t rhs) final {
@@ -255,12 +250,10 @@ protected:
     void swap_and_pop(const std::size_t pos) final {
         auto &&elem = packed[page(pos)][offset(pos)];
         [[maybe_unused]] auto other = std::move(elem);
+        auto &&last = (--count, packed[page(count)][offset(count)]);
 
-        auto &&last = packed[page(count - 1)][offset(count - 1)];
         elem = std::move(last);
         alloc_traits::destroy(allocator, std::addressof(last));
-
-        --count;
     }
 
 public:
@@ -312,7 +305,7 @@ public:
 
     /*! @brief Default destructor. */
     ~basic_storage() override {
-        reset_to_empty();
+        maybe_resize_packed(0u);
     }
 
     /**
@@ -357,7 +350,7 @@ public:
     /*! @brief Requests the removal of unused capacity. */
     void shrink_to_fit() {
         underlying_type::shrink_to_fit();
-        count ? maybe_resize_packed(count) : reset_to_empty();
+        maybe_resize_packed(count);
     }
 
     /**

test/entt/entity/sparse_set.cpp

@@ -24,6 +24,11 @@ TEST(SparseSet, Functionalities) {
     ASSERT_FALSE(set.contains(entt::entity{0}));
     ASSERT_FALSE(set.contains(entt::entity{42}));
 
+    set.reserve(0);
+
+    ASSERT_EQ(set.capacity(), 42u);
+    ASSERT_TRUE(set.empty());
+
     set.emplace(entt::entity{42});
 
     ASSERT_FALSE(set.empty());

test/entt/entity/storage.cpp

@@ -48,6 +48,11 @@ TEST(Storage, Functionalities) {
     ASSERT_FALSE(pool.contains(entt::entity{0}));
     ASSERT_FALSE(pool.contains(entt::entity{41}));
 
+    pool.reserve(0);
+
+    ASSERT_EQ(pool.capacity(), entt::page_size);
+    ASSERT_TRUE(pool.empty());
+
     pool.emplace(entt::entity{41}, 3);
 
     ASSERT_FALSE(pool.empty());