From 4f8aff4352745bc86bb9e9c0a7cc6604cf7e6b7a Mon Sep 17 00:00:00 2001 From: Wojtek Figat Date: Tue, 28 Nov 2023 16:02:36 +0100 Subject: [PATCH] Refactor memory allocators to use dedicated path when moving collection data that is not blittable #2001 #1920 --- Source/Engine/Core/Collections/Array.h | 34 +++++++++++--- Source/Engine/Core/Collections/Dictionary.h | 52 +++++++++++++++++---- Source/Engine/Core/Collections/HashSet.h | 50 ++++++++++++++++---- Source/Engine/Core/Compiler.h | 7 +++ Source/Engine/Core/Memory/Allocation.h | 22 ++++----- Source/Engine/Renderer/RendererAllocation.h | 2 + 6 files changed, 132 insertions(+), 35 deletions(-) diff --git a/Source/Engine/Core/Collections/Array.h b/Source/Engine/Core/Collections/Array.h index 58117cf0a..cf45ed060 100644 --- a/Source/Engine/Core/Collections/Array.h +++ b/Source/Engine/Core/Collections/Array.h @@ -25,6 +25,19 @@ private: int32 _capacity; AllocationData _allocation; + FORCE_INLINE static void MoveToEmpty(AllocationData& to, AllocationData& from, int32 fromCount, int32 fromCapacity) + { + if IF_CONSTEXPR (AllocationType::HasSwap) + to.Swap(from); + else + { + to.Allocate(fromCapacity); + Memory::MoveItems(to.Get(), from.Get(), fromCount); + Memory::DestructItems(from.Get(), fromCount); + from.Free(); + } + } + public: /// /// Initializes a new instance of the class. @@ -134,7 +147,7 @@ public: _capacity = other._capacity; other._count = 0; other._capacity = 0; - _allocation.Swap(other._allocation); + MoveToEmpty(_allocation, other._allocation, _count, _capacity); } /// @@ -191,7 +204,7 @@ public: _capacity = other._capacity; other._count = 0; other._capacity = 0; - _allocation.Swap(other._allocation); + MoveToEmpty(_allocation, other._allocation, _count, _capacity); } return *this; } @@ -713,9 +726,18 @@ public: /// The other collection. void Swap(Array& other) { - ::Swap(_count, other._count); - ::Swap(_capacity, other._capacity); - _allocation.Swap(other._allocation); + if IF_CONSTEXPR (AllocationType::HasSwap) + { + _allocation.Swap(other._allocation); + ::Swap(_count, other._count); + ::Swap(_capacity, other._capacity); + } + else + { + Array tmp = MoveTemp(other); + other = *this; + *this = MoveTemp(tmp); + } } /// @@ -726,9 +748,7 @@ public: T* data = _allocation.Get(); const int32 count = _count / 2; for (int32 i = 0; i < count; i++) - { ::Swap(data[i], data[_count - i - 1]); - } } public: diff --git a/Source/Engine/Core/Collections/Dictionary.h b/Source/Engine/Core/Collections/Dictionary.h index 4d65a4123..dd73be390 100644 --- a/Source/Engine/Core/Collections/Dictionary.h +++ b/Source/Engine/Core/Collections/Dictionary.h @@ -110,6 +110,33 @@ private: int32 _size = 0; AllocationData _allocation; + FORCE_INLINE static void MoveToEmpty(AllocationData& to, AllocationData& from, int32 fromSize) + { + if IF_CONSTEXPR (AllocationType::HasSwap) + to.Swap(from); + else + { + to.Allocate(fromSize); + Bucket* toData = to.Get(); + Bucket* fromData = from.Get(); + for (int32 i = 0; i < fromSize; i++) + { + Bucket& fromBucket = fromData[i]; + if (fromBucket.IsOccupied()) + { + Bucket& toBucket = toData[i]; + Memory::MoveItems(&toBucket.Key, &fromBucket.Key, 1); + Memory::MoveItems(&toBucket.Value, &fromBucket.Value, 1); + toBucket._state = Bucket::Occupied; + Memory::DestructItem(&fromBucket.Key); + Memory::DestructItem(&fromBucket.Value); + fromBucket._state = Bucket::Empty; + } + } + from.Free(); + } + } + public: /// /// Initializes a new instance of the class. @@ -139,7 +166,7 @@ public: other._elementsCount = 0; other._deletedCount = 0; other._size = 0; - _allocation.Swap(other._allocation); + MoveToEmpty(_allocation, other._allocation, _size); } /// @@ -180,7 +207,7 @@ public: other._elementsCount = 0; other._deletedCount = 0; other._size = 0; - _allocation.Swap(other._allocation); + MoveToEmpty(_allocation, other._allocation, _size); } return *this; } @@ -510,7 +537,7 @@ public: return; ASSERT(capacity >= 0); AllocationData oldAllocation; - oldAllocation.Swap(_allocation); + MoveToEmpty(oldAllocation, _allocation, _size); const int32 oldSize = _size; const int32 oldElementsCount = _elementsCount; _deletedCount = _elementsCount = 0; @@ -580,10 +607,19 @@ public: /// The other collection. void Swap(Dictionary& other) { - ::Swap(_elementsCount, other._elementsCount); - ::Swap(_deletedCount, other._deletedCount); - ::Swap(_size, other._size); - _allocation.Swap(other._allocation); + if IF_CONSTEXPR (AllocationType::HasSwap) + { + ::Swap(_elementsCount, other._elementsCount); + ::Swap(_deletedCount, other._deletedCount); + ::Swap(_size, other._size); + _allocation.Swap(other._allocation); + } + else + { + Dictionary tmp = MoveTemp(other); + other = *this; + *this = MoveTemp(tmp); + } } public: @@ -930,7 +966,7 @@ private: { // Rebuild entire table completely AllocationData oldAllocation; - oldAllocation.Swap(_allocation); + MoveToEmpty(oldAllocation, _allocation, _size); _allocation.Allocate(_size); Bucket* data = _allocation.Get(); for (int32 i = 0; i < _size; i++) diff --git a/Source/Engine/Core/Collections/HashSet.h b/Source/Engine/Core/Collections/HashSet.h index 4a4f3e924..a683edf15 100644 --- a/Source/Engine/Core/Collections/HashSet.h +++ b/Source/Engine/Core/Collections/HashSet.h @@ -93,6 +93,31 @@ private: int32 _size = 0; AllocationData _allocation; + FORCE_INLINE static void MoveToEmpty(AllocationData& to, AllocationData& from, int32 fromSize) + { + if IF_CONSTEXPR (AllocationType::HasSwap) + to.Swap(from); + else + { + to.Allocate(fromSize); + Bucket* toData = to.Get(); + Bucket* fromData = from.Get(); + for (int32 i = 0; i < fromSize; i++) + { + Bucket& fromBucket = fromData[i]; + if (fromBucket.IsOccupied()) + { + Bucket& toBucket = toData[i]; + Memory::MoveItems(&toBucket.Item, &fromBucket.Item, 1); + toBucket._state = Bucket::Occupied; + Memory::DestructItem(&fromBucket.Item); + fromBucket._state = Bucket::Empty; + } + } + from.Free(); + } + } + public: /// /// Initializes a new instance of the class. @@ -122,7 +147,7 @@ public: other._elementsCount = 0; other._deletedCount = 0; other._size = 0; - _allocation.Swap(other._allocation); + MoveToEmpty(_allocation, other._allocation, _size); } /// @@ -163,7 +188,7 @@ public: other._elementsCount = 0; other._deletedCount = 0; other._size = 0; - _allocation.Swap(other._allocation); + MoveToEmpty(_allocation, other._allocation, _size); } return *this; } @@ -389,7 +414,7 @@ public: return; ASSERT(capacity >= 0); AllocationData oldAllocation; - oldAllocation.Swap(_allocation); + MoveToEmpty(oldAllocation, _allocation, _size); const int32 oldSize = _size; const int32 oldElementsCount = _elementsCount; _deletedCount = _elementsCount = 0; @@ -458,10 +483,19 @@ public: /// The other collection. void Swap(HashSet& other) { - ::Swap(_elementsCount, other._elementsCount); - ::Swap(_deletedCount, other._deletedCount); - ::Swap(_size, other._size); - _allocation.Swap(other._allocation); + if IF_CONSTEXPR (AllocationType::HasSwap) + { + ::Swap(_elementsCount, other._elementsCount); + ::Swap(_deletedCount, other._deletedCount); + ::Swap(_size, other._size); + _allocation.Swap(other._allocation); + } + else + { + HashSet tmp = MoveTemp(other); + other = *this; + *this = MoveTemp(tmp); + } } public: @@ -726,7 +760,7 @@ private: { // Rebuild entire table completely AllocationData oldAllocation; - oldAllocation.Swap(_allocation); + MoveToEmpty(oldAllocation, _allocation, _size); _allocation.Allocate(_size); Bucket* data = _allocation.Get(); for (int32 i = 0; i < _size; i++) diff --git a/Source/Engine/Core/Compiler.h b/Source/Engine/Core/Compiler.h index 9a33b8758..4ea246077 100644 --- a/Source/Engine/Core/Compiler.h +++ b/Source/Engine/Core/Compiler.h @@ -93,3 +93,10 @@ #endif #define PACK_STRUCT(__Declaration__) PACK_BEGIN() __Declaration__ PACK_END() + +// C++ 17 +#if __cplusplus >= 201703L +#define IF_CONSTEXPR constexpr +#else +#define IF_CONSTEXPR +#endif diff --git a/Source/Engine/Core/Memory/Allocation.h b/Source/Engine/Core/Memory/Allocation.h index 8d7188d91..fb6b4555b 100644 --- a/Source/Engine/Core/Memory/Allocation.h +++ b/Source/Engine/Core/Memory/Allocation.h @@ -12,6 +12,8 @@ template class FixedAllocation { public: + enum { HasSwap = false }; + template class Data { @@ -61,12 +63,9 @@ public: { } - FORCE_INLINE void Swap(Data& other) + void Swap(Data& other) { - byte tmp[Capacity * sizeof(T)]; - Platform::MemoryCopy(tmp, _data, Capacity * sizeof(T)); - Platform::MemoryCopy(_data, other._data, Capacity * sizeof(T)); - Platform::MemoryCopy(other._data, tmp, Capacity * sizeof(T)); + // Not supported } }; }; @@ -77,6 +76,8 @@ public: class HeapAllocation { public: + enum { HasSwap = true }; + template class Data { @@ -179,6 +180,8 @@ template class InlinedAllocation { public: + enum { HasSwap = false }; + template class Data { @@ -267,14 +270,9 @@ public: } } - FORCE_INLINE void Swap(Data& other) + void Swap(Data& other) { - byte tmp[Capacity * sizeof(T)]; - Platform::MemoryCopy(tmp, _data, Capacity * sizeof(T)); - Platform::MemoryCopy(_data, other._data, Capacity * sizeof(T)); - Platform::MemoryCopy(other._data, tmp, Capacity * sizeof(T)); - ::Swap(_useOther, other._useOther); - _other.Swap(other._other); + // Not supported } }; }; diff --git a/Source/Engine/Renderer/RendererAllocation.h b/Source/Engine/Renderer/RendererAllocation.h index 22e7518c6..2a4df4b5f 100644 --- a/Source/Engine/Renderer/RendererAllocation.h +++ b/Source/Engine/Renderer/RendererAllocation.h @@ -11,6 +11,8 @@ public: static FLAXENGINE_API void* Allocate(uintptr size); static FLAXENGINE_API void Free(void* ptr, uintptr size); + enum { HasSwap = true }; + template class Data {