sparse_set/storage: review memory management for fancy allocators and the like

src/entt/entity/sparse_set.hpp

@@ -173,14 +173,12 @@ class basic_sparse_set {
             const auto old = std::exchange(sparse, bucket_alloc_traits::allocate(bucket_allocator, sz));
             std::uninitialized_fill(sparse + bucket, sparse + sz, alloc_pointer{});
 
-            if(const auto curr = std::exchange(bucket, sz); curr) {
-                for(size_type pos{}; pos < curr; ++pos) {
-                    bucket_alloc_traits::construct(bucket_allocator, std::addressof(sparse[pos]), std::move(old[pos]));
-                    bucket_alloc_traits::destroy(bucket_allocator, std::addressof(old[pos]));
-                }
-
-                bucket_alloc_traits::deallocate(bucket_allocator, old, curr);
+            for(size_type pos{}; pos < bucket; ++pos) {
+                bucket_alloc_traits::construct(bucket_allocator, std::addressof(sparse[pos]), std::move(old[pos]));
+                bucket_alloc_traits::destroy(bucket_allocator, std::addressof(old[pos]));
             }
+
+            bucket_alloc_traits::deallocate(bucket_allocator, old, std::exchange(bucket, sz));
         }
 
         if(!sparse[idx]) {
@@ -192,33 +190,33 @@ class basic_sparse_set {
     }
 
     void resize_packed(const std::size_t req) {
-        ENTT_ASSERT(req && (req != reserved) && !(req < count), "Invalid request");
+        ENTT_ASSERT((req != reserved) && !(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, std::exchange(reserved, req));
     }
 
     void release_memory() {
-        if(reserved) {
+        if(packed) {
+            ENTT_ASSERT(sparse, "Something very wrong happened");
+
             std::destroy(packed, packed + std::exchange(count, 0u));
             alloc_traits::deallocate(allocator, std::exchange(packed, alloc_pointer{}), std::exchange(reserved, 0u));
-        }
 
-        if(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);
                 }
-            
+
                 bucket_alloc_traits::destroy(bucket_allocator, std::addressof(sparse[pos]));
             }
-            
+
             bucket_alloc_traits::deallocate(bucket_allocator, sparse, std::exchange(bucket, 0u));
         }
     }
@@ -292,11 +290,11 @@ public:
     explicit basic_sparse_set(const allocator_type &alloc = {})
         : allocator{alloc},
           bucket_allocator{alloc},
-          sparse{},
-          packed{},
-          bucket{},
-          count{},
-          reserved{}
+          sparse{bucket_alloc_traits::allocate(bucket_allocator, 0u)},
+          packed{alloc_traits::allocate(allocator, 0u)},
+          bucket{0u},
+          count{0u},
+          reserved{0u}
     {}
 
     /**
@@ -362,9 +360,7 @@ public:
 
     /*! @brief Requests the removal of unused capacity. */
     void shrink_to_fit() {
-        if(!count && reserved) {
-            release_memory();
-        } else if(count && count != reserved) {
+        if(count < reserved) {
             resize_packed(count);
         }
     }

src/entt/entity/storage.hpp

@@ -180,7 +180,7 @@ class basic_storage: public basic_sparse_set<Entity, typename std::allocator_tra
     }
 
     void release_memory() {
-        if(bucket) {
+        if(packed) {
             for(size_type pos{}; pos < bucket; ++pos) {
                 if(count) {
                     const auto sz = count > page_size ? page_size : count;
@@ -197,7 +197,7 @@ 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");
+        ENTT_ASSERT(!(req < count), "Invalid request");
 
         if(const auto length = std::exchange(bucket, (req + page_size - 1u) / page_size); bucket != length) {
             const auto old = std::exchange(packed, bucket_alloc_traits::allocate(bucket_allocator, bucket));
@@ -207,7 +207,7 @@ 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 = length; pos < bucket; ++pos) {
                     bucket_alloc_traits::construct(bucket_allocator, std::addressof(packed[pos]), alloc_traits::allocate(allocator, page_size));
                 }
@@ -216,16 +216,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);
-            }
+            bucket_alloc_traits::deallocate(bucket_allocator, old, length);
         }
     }
 
@@ -359,7 +357,7 @@ public:
     /*! @brief Requests the removal of unused capacity. */
     void shrink_to_fit() {
         underlying_type::shrink_to_fit();
-        count ? maybe_resize_packed(count) : release_memory();
+        maybe_resize_packed(count);
     }
 
     /**
@@ -545,14 +543,13 @@ public:
      */
     template<typename It>
     void insert(It first, It last, const value_type &value = {}) {
-        if(const auto sz = count + std::distance(first, last); sz != count) {
-            maybe_resize_packed(sz);
-            underlying_type::reserve(sz);
+        const auto sz = count + std::distance(first, last);
+        underlying_type::reserve(sz);
+        maybe_resize_packed(sz);
 
-            for(; first != last; ++first) {
-                push_back(value);
-                underlying_type::emplace(*first);
-            }
+        for(; first != last; ++first) {
+            push_back(value);
+            underlying_type::emplace(*first);
         }
     }
 
@@ -570,14 +567,13 @@ public:
      */
     template<typename EIt, typename CIt, typename = std::enable_if_t<std::is_same_v<std::decay_t<typename std::iterator_traits<CIt>::value_type>, value_type>>>
     void insert(EIt first, EIt last, CIt from) {
-        if(const auto sz = count + std::distance(first, last); sz != count) {
-            maybe_resize_packed(sz);
-            underlying_type::reserve(sz);
+        const auto sz = count + std::distance(first, last);
+        underlying_type::reserve(sz);
+        maybe_resize_packed(sz);
 
-            for(; first != last; ++first, ++from) {
-                push_back(*from);
-                underlying_type::emplace(*first);
-            }
+        for(; first != last; ++first, ++from) {
+            push_back(*from);
+            underlying_type::emplace(*first);
         }
     }
 

test/entt/entity/observer.cpp

@@ -11,7 +11,6 @@ TEST(Observer, Functionalities) {
 
     ASSERT_EQ(observer.size(), 0u);
     ASSERT_TRUE(observer.empty());
-    ASSERT_EQ(observer.data(), nullptr);
     ASSERT_EQ(observer.begin(), observer.end());
 
     const auto entity = registry.create();
@@ -19,7 +18,6 @@ TEST(Observer, Functionalities) {
 
     ASSERT_EQ(observer.size(), 1u);
     ASSERT_FALSE(observer.empty());
-    ASSERT_NE(observer.data(), nullptr);
     ASSERT_EQ(*observer.data(), entity);
     ASSERT_NE(observer.begin(), observer.end());
     ASSERT_EQ(++observer.begin(), observer.end());
@@ -100,7 +98,6 @@ TEST(Observer, AllOfFiltered) {
 
     ASSERT_EQ(observer.size(), 0u);
     ASSERT_TRUE(observer.empty());
-    ASSERT_EQ(observer.data(), nullptr);
 
     registry.erase<int>(entity);
     registry.emplace<char>(entity);
@@ -181,7 +178,6 @@ TEST(Observer, ObserveFiltered) {
 
     ASSERT_EQ(observer.size(), 0u);
     ASSERT_TRUE(observer.empty());
-    ASSERT_EQ(observer.data(), nullptr);
 
     registry.emplace<char>(entity);
     registry.emplace<double>(entity);

test/entt/entity/sparse_set.cpp

@@ -123,7 +123,7 @@ TEST(SparseSet, Pagination) {
 
     set.shrink_to_fit();
 
-    ASSERT_EQ(set.extent(), 0u);
+    ASSERT_EQ(set.extent(), 2 * entt::page_size);
 }
 
 TEST(SparseSet, Insert) {

test/entt/entity/view.cpp

@@ -73,10 +73,8 @@ TEST(SingleComponentView, RawData) {
 
     ASSERT_EQ(view.size(), 0u);
     ASSERT_EQ(cview.size(), 0u);
-    ASSERT_EQ(view.raw(), nullptr);
-    ASSERT_EQ(cview.raw(), nullptr);
-    ASSERT_EQ(view.data(), nullptr);
-    ASSERT_EQ(cview.data(), nullptr);
+    ASSERT_EQ(view.raw(), cview.raw());
+    ASSERT_EQ(view.data(), cview.data());
 
     registry.emplace<int>(entity, 42);