From 02db7d02f270469137590dc29ac9ae5b551d9436 Mon Sep 17 00:00:00 2001 From: Mateusz Karbowiak <69864511+mtszkarbowiak@users.noreply.github.com> Date: Wed, 30 Oct 2024 21:06:16 +0100 Subject: [PATCH 01/13] Collections const-correctness fix --- Source/Engine/Core/Collections/BitArray.h | 20 ++++++++++---------- Source/Engine/Core/Collections/Dictionary.h | 6 +++--- Source/Engine/Core/Collections/HashSet.h | 6 +++--- Source/Engine/Core/Collections/RingBuffer.h | 4 ++-- Source/Engine/Core/Memory/Allocation.h | 12 ++++++------ Source/Engine/Renderer/RenderListBuffer.h | 4 ++-- 6 files changed, 26 insertions(+), 26 deletions(-) diff --git a/Source/Engine/Core/Collections/BitArray.h b/Source/Engine/Core/Collections/BitArray.h index c7853b989..267885438 100644 --- a/Source/Engine/Core/Collections/BitArray.h +++ b/Source/Engine/Core/Collections/BitArray.h @@ -22,12 +22,12 @@ private: int32 _capacity; AllocationData _allocation; - FORCE_INLINE static int32 ToItemCount(int32 size) + FORCE_INLINE static int32 ToItemCount(const int32 size) { return Math::DivideAndRoundUp(size, sizeof(ItemType)); } - FORCE_INLINE static int32 ToItemCapacity(int32 size) + FORCE_INLINE static int32 ToItemCapacity(const int32 size) { return Math::Max(Math::DivideAndRoundUp(size, sizeof(ItemType)), 1); } @@ -46,7 +46,7 @@ public: /// Initializes a new instance of the class. /// /// The initial capacity. - BitArray(int32 capacity) + BitArray(const int32 capacity) : _count(0) , _capacity(capacity) { @@ -200,7 +200,7 @@ public: /// /// The index of the item. /// The value of the item. - FORCE_INLINE bool operator[](int32 index) const + FORCE_INLINE bool operator[](const int32 index) const { return Get(index); } @@ -210,7 +210,7 @@ public: /// /// The index of the item. /// The value of the item. - bool Get(int32 index) const + bool Get(const int32 index) const { ASSERT(index >= 0 && index < _count); const ItemType offset = index / sizeof(ItemType); @@ -224,7 +224,7 @@ public: /// /// The index of the item. /// The value to set. - void Set(int32 index, bool value) + void Set(const int32 index, const bool value) { ASSERT(index >= 0 && index < _count); const ItemType offset = index / sizeof(ItemType); @@ -250,7 +250,7 @@ public: /// /// The new capacity. /// True if preserve collection data when changing its size, otherwise collection after resize will be empty. - void SetCapacity(const int32 capacity, bool preserveContents = true) + void SetCapacity(const int32 capacity, const bool preserveContents = true) { if (capacity == _capacity) return; @@ -266,7 +266,7 @@ public: /// /// The new collection size. /// True if preserve collection data when changing its size, otherwise collection after resize might not contain the previous data. - void Resize(int32 size, bool preserveContents = true) + void Resize(const int32 size, const bool preserveContents = true) { if (_count <= size) EnsureCapacity(size, preserveContents); @@ -278,7 +278,7 @@ public: /// /// The minimum capacity. /// True if preserve collection data when changing its size, otherwise collection after resize will be empty. - void EnsureCapacity(int32 minCapacity, bool preserveContents = true) + void EnsureCapacity(int32 minCapacity, const bool preserveContents = true) { if (_capacity < minCapacity) { @@ -313,7 +313,7 @@ public: /// /// The items to add. /// The items count. - void Add(const bool* items, int32 count) + void Add(const bool* items, const int32 count) { EnsureCapacity(_count + count); for (int32 i = 0; i < count; i++) diff --git a/Source/Engine/Core/Collections/Dictionary.h b/Source/Engine/Core/Collections/Dictionary.h index 9e516dae7..1ce1db21b 100644 --- a/Source/Engine/Core/Collections/Dictionary.h +++ b/Source/Engine/Core/Collections/Dictionary.h @@ -149,7 +149,7 @@ public: /// Initializes a new instance of the class. /// /// The initial capacity. - Dictionary(int32 capacity) + Dictionary(const int32 capacity) { SetCapacity(capacity); } @@ -538,7 +538,7 @@ public: /// /// The new capacity. /// Enables preserving collection contents during resizing. - void SetCapacity(int32 capacity, bool preserveContents = true) + void SetCapacity(int32 capacity, const bool preserveContents = true) { if (capacity == Capacity()) return; @@ -598,7 +598,7 @@ public: /// /// The minimum required capacity. /// True if preserve collection data when changing its size, otherwise collection after resize will be empty. - void EnsureCapacity(int32 minCapacity, bool preserveContents = true) + void EnsureCapacity(int32 minCapacity, const bool preserveContents = true) { if (_size >= minCapacity) return; diff --git a/Source/Engine/Core/Collections/HashSet.h b/Source/Engine/Core/Collections/HashSet.h index b53225044..1bfcc75eb 100644 --- a/Source/Engine/Core/Collections/HashSet.h +++ b/Source/Engine/Core/Collections/HashSet.h @@ -130,7 +130,7 @@ public: /// Initializes a new instance of the class. /// /// The initial capacity. - HashSet(int32 capacity) + HashSet(const int32 capacity) { SetCapacity(capacity); } @@ -415,7 +415,7 @@ public: /// /// New capacity /// Enable/disable preserving collection contents during resizing - void SetCapacity(int32 capacity, bool preserveContents = true) + void SetCapacity(int32 capacity, const bool preserveContents = true) { if (capacity == Capacity()) return; @@ -474,7 +474,7 @@ public: /// /// The minimum required capacity. /// True if preserve collection data when changing its size, otherwise collection after resize will be empty. - void EnsureCapacity(int32 minCapacity, bool preserveContents = true) + void EnsureCapacity(int32 minCapacity, const bool preserveContents = true) { if (_size >= minCapacity) return; diff --git a/Source/Engine/Core/Collections/RingBuffer.h b/Source/Engine/Core/Collections/RingBuffer.h index 4dcc75e33..1bc0ae5f4 100644 --- a/Source/Engine/Core/Collections/RingBuffer.h +++ b/Source/Engine/Core/Collections/RingBuffer.h @@ -75,13 +75,13 @@ public: return _allocation.Get()[_front]; } - FORCE_INLINE T& operator[](int32 index) + FORCE_INLINE T& operator[](const int32 index) { ASSERT(index >= 0 && index < _count); return _allocation.Get()[(_front + index) % _capacity]; } - FORCE_INLINE const T& operator[](int32 index) const + FORCE_INLINE const T& operator[](const int32 index) const { ASSERT(index >= 0 && index < _count); return _allocation.Get()[(_front + index) % _capacity]; diff --git a/Source/Engine/Core/Memory/Allocation.h b/Source/Engine/Core/Memory/Allocation.h index 7d28c70cb..6e8342515 100644 --- a/Source/Engine/Core/Memory/Allocation.h +++ b/Source/Engine/Core/Memory/Allocation.h @@ -39,20 +39,20 @@ public: return (T*)_data; } - FORCE_INLINE int32 CalculateCapacityGrow(int32 capacity, int32 minCapacity) const + FORCE_INLINE int32 CalculateCapacityGrow(int32 capacity, const int32 minCapacity) const { ASSERT(minCapacity <= Capacity); return Capacity; } - FORCE_INLINE void Allocate(int32 capacity) + FORCE_INLINE void Allocate(const int32 capacity) { #if ENABLE_ASSERTION_LOW_LAYERS ASSERT(capacity <= Capacity); #endif } - FORCE_INLINE void Relocate(int32 capacity, int32 oldCount, int32 newCount) + FORCE_INLINE void Relocate(const int32 capacity, int32 oldCount, int32 newCount) { #if ENABLE_ASSERTION_LOW_LAYERS ASSERT(capacity <= Capacity); @@ -104,7 +104,7 @@ public: return _data; } - FORCE_INLINE int32 CalculateCapacityGrow(int32 capacity, int32 minCapacity) const + FORCE_INLINE int32 CalculateCapacityGrow(int32 capacity, const int32 minCapacity) const { if (capacity < minCapacity) capacity = minCapacity; @@ -129,7 +129,7 @@ public: return capacity; } - FORCE_INLINE void Allocate(int32 capacity) + FORCE_INLINE void Allocate(const int32 capacity) { #if ENABLE_ASSERTION_LOW_LAYERS ASSERT(!_data); @@ -141,7 +141,7 @@ public: #endif } - FORCE_INLINE void Relocate(int32 capacity, int32 oldCount, int32 newCount) + FORCE_INLINE void Relocate(const int32 capacity, int32 oldCount, int32 newCount) { T* newData = capacity != 0 ? (T*)Allocator::Allocate(capacity * sizeof(T)) : nullptr; #if !BUILD_RELEASE diff --git a/Source/Engine/Renderer/RenderListBuffer.h b/Source/Engine/Renderer/RenderListBuffer.h index e4f8ab19e..486b1715b 100644 --- a/Source/Engine/Renderer/RenderListBuffer.h +++ b/Source/Engine/Renderer/RenderListBuffer.h @@ -242,7 +242,7 @@ public: /// /// The new capacity. /// True if preserve collection data when changing its size, otherwise collection after resize will be empty. - void SetCapacity(const int32 capacity, bool preserveContents = true) + void SetCapacity(const int32 capacity, const bool preserveContents = true) { if (capacity == Capacity()) return; @@ -365,7 +365,7 @@ private: return (int32)Platform::InterlockedIncrement(&_count) - 1; } - FORCE_INLINE static int32 GetMinCapacity(int32 count) + FORCE_INLINE static int32 GetMinCapacity(const int32 count) { // Ensure there is a slack for others threads to reduce resize counts in highly multi-threaded environment constexpr int32 slack = PLATFORM_THREADS_LIMIT * 8; From a55866d5589ee3fee14edf50aff319c1a66e9657 Mon Sep 17 00:00:00 2001 From: Mateusz Karbowiak <69864511+mtszkarbowiak@users.noreply.github.com> Date: Wed, 30 Oct 2024 22:18:47 +0100 Subject: [PATCH 02/13] Collections casts fix --- Source/Engine/Core/Collections/Array.h | 6 ++-- Source/Engine/Core/Collections/BitArray.h | 8 ++--- Source/Engine/Core/Collections/Dictionary.h | 2 +- Source/Engine/Core/Memory/Allocation.h | 20 ++++++----- Source/Engine/Renderer/RenderListBuffer.h | 38 ++++++++++----------- 5 files changed, 38 insertions(+), 36 deletions(-) diff --git a/Source/Engine/Core/Collections/Array.h b/Source/Engine/Core/Collections/Array.h index a17d8f686..8f73cfba7 100644 --- a/Source/Engine/Core/Collections/Array.h +++ b/Source/Engine/Core/Collections/Array.h @@ -66,7 +66,7 @@ public: /// The initial values defined in the array. Array(std::initializer_list initList) { - _count = _capacity = (int32)initList.size(); + _count = _capacity = static_cast(initList.size()); if (_count > 0) { _allocation.Allocate(_count); @@ -160,8 +160,8 @@ public: Clear(); if (initList.size() > 0) { - EnsureCapacity((int32)initList.size()); - _count = (int32)initList.size(); + EnsureCapacity(static_cast(initList.size())); + _count = static_cast(initList.size()); Memory::ConstructItems(_allocation.Get(), initList.begin(), _count); } return *this; diff --git a/Source/Engine/Core/Collections/BitArray.h b/Source/Engine/Core/Collections/BitArray.h index 267885438..97674974c 100644 --- a/Source/Engine/Core/Collections/BitArray.h +++ b/Source/Engine/Core/Collections/BitArray.h @@ -214,8 +214,8 @@ public: { ASSERT(index >= 0 && index < _count); const ItemType offset = index / sizeof(ItemType); - const ItemType bitMask = (ItemType)(int32)(1 << (index & ((int32)sizeof(ItemType) - 1))); - const ItemType item = ((ItemType*)_allocation.Get())[offset]; + const ItemType bitMask = static_cast((int32)(1 << (index & (static_cast(sizeof(ItemType)) - 1)))); + const ItemType item = static_cast(_allocation.Get())[offset]; return (item & bitMask) != 0; } @@ -228,8 +228,8 @@ public: { ASSERT(index >= 0 && index < _count); const ItemType offset = index / sizeof(ItemType); - const ItemType bitMask = (ItemType)(int32)(1 << (index & ((int32)sizeof(ItemType) - 1))); - ItemType& item = ((ItemType*)_allocation.Get())[offset]; + const ItemType bitMask = static_cast((int32)(1 << (index & (static_cast(sizeof(ItemType)) - 1)))); + ItemType& item = reinterpret_cast(_allocation.Get())[offset]; if (value) item |= bitMask; else diff --git a/Source/Engine/Core/Collections/Dictionary.h b/Source/Engine/Core/Collections/Dictionary.h index 1ce1db21b..438e1bb75 100644 --- a/Source/Engine/Core/Collections/Dictionary.h +++ b/Source/Engine/Core/Collections/Dictionary.h @@ -498,7 +498,7 @@ public: FindPosition(key, pos); if (pos.ObjectIndex == -1) return nullptr; - return (ValueType*)&_allocation.Get()[pos.ObjectIndex].Value; + return static_cast(&_allocation.Get()[pos.ObjectIndex].Value); } public: diff --git a/Source/Engine/Core/Memory/Allocation.h b/Source/Engine/Core/Memory/Allocation.h index 6e8342515..bdf30a16d 100644 --- a/Source/Engine/Core/Memory/Allocation.h +++ b/Source/Engine/Core/Memory/Allocation.h @@ -31,12 +31,12 @@ public: FORCE_INLINE T* Get() { - return (T*)_data; + return reinterpret_cast(_data); } FORCE_INLINE const T* Get() const { - return (T*)_data; + return reinterpret_cast(_data); } FORCE_INLINE int32 CalculateCapacityGrow(int32 capacity, const int32 minCapacity) const @@ -134,7 +134,7 @@ public: #if ENABLE_ASSERTION_LOW_LAYERS ASSERT(!_data); #endif - _data = (T*)Allocator::Allocate(capacity * sizeof(T)); + _data = static_cast(Allocator::Allocate(capacity * sizeof(T))); #if !BUILD_RELEASE if (!_data) OUT_OF_MEMORY; @@ -143,7 +143,7 @@ public: FORCE_INLINE void Relocate(const int32 capacity, int32 oldCount, int32 newCount) { - T* newData = capacity != 0 ? (T*)Allocator::Allocate(capacity * sizeof(T)) : nullptr; + T* newData = capacity != 0 ? static_cast(Allocator::Allocate(capacity * sizeof(T))) : nullptr; #if !BUILD_RELEASE if (!newData && capacity != 0) OUT_OF_MEMORY; @@ -203,12 +203,12 @@ public: FORCE_INLINE T* Get() { - return _useOther ? _other.Get() : (T*)_data; + return _useOther ? _other.Get() : reinterpret_cast(_data); } FORCE_INLINE const T* Get() const { - return _useOther ? _other.Get() : (T*)_data; + return _useOther ? _other.Get() : reinterpret_cast(_data); } FORCE_INLINE int32 CalculateCapacityGrow(int32 capacity, int32 minCapacity) const @@ -227,13 +227,15 @@ public: FORCE_INLINE void Relocate(int32 capacity, int32 oldCount, int32 newCount) { + T* data = reinterpret_cast(_data); + // Check if the new allocation will fit into inlined storage if (capacity <= Capacity) { if (_useOther) { // Move the items from other allocation to the inlined storage - Memory::MoveItems((T*)_data, _other.Get(), newCount); + Memory::MoveItems(data, _other.Get(), newCount); // Free the other allocation Memory::DestructItems(_other.Get(), oldCount); @@ -255,8 +257,8 @@ public: _useOther = true; // Move the items from the inlined storage to the other allocation - Memory::MoveItems(_other.Get(), (T*)_data, newCount); - Memory::DestructItems((T*)_data, oldCount); + Memory::MoveItems(_other.Get(), data, newCount); + Memory::DestructItems(data, oldCount); } } } diff --git a/Source/Engine/Renderer/RenderListBuffer.h b/Source/Engine/Renderer/RenderListBuffer.h index 486b1715b..bbe087e6e 100644 --- a/Source/Engine/Renderer/RenderListBuffer.h +++ b/Source/Engine/Renderer/RenderListBuffer.h @@ -76,7 +76,7 @@ public: if (_capacity > 0) { _allocation.Allocate(_capacity); - Memory::ConstructItems(_allocation.Get(), other.Get(), (int32)other._count); + Memory::ConstructItems(_allocation.Get(), other.Get(), static_cast(other._count)); } } @@ -102,7 +102,7 @@ public: { if (this != &other) { - Memory::DestructItems(_allocation.Get(), (int32)_count); + Memory::DestructItems(_allocation.Get(), static_cast(_count)); if (_capacity < other.Count()) { _allocation.Free(); @@ -110,7 +110,7 @@ public: _allocation.Allocate(_capacity); } _count = other.Count(); - Memory::ConstructItems(_allocation.Get(), other.Get(), (int32)_count); + Memory::ConstructItems(_allocation.Get(), other.Get(), static_cast(_count)); } return *this; } @@ -124,7 +124,7 @@ public: { if (this != &other) { - Memory::DestructItems(_allocation.Get(), (int32)_count); + Memory::DestructItems(_allocation.Get(), static_cast(_count)); _allocation.Free(); _count = other._count; _capacity = other._capacity; @@ -140,7 +140,7 @@ public: /// ~RenderListBuffer() { - Memory::DestructItems(_allocation.Get(), (int32)_count); + Memory::DestructItems(_allocation.Get(), static_cast(_count)); } public: @@ -149,7 +149,7 @@ public: /// FORCE_INLINE int32 Count() const { - return (int32)Platform::AtomicRead((volatile int64*)&_count); + return static_cast(Platform::AtomicRead((volatile int64*)&_count)); } /// @@ -157,7 +157,7 @@ public: /// FORCE_INLINE int32 Capacity() const { - return (int32)Platform::AtomicRead((volatile int64*)&_capacity); + return static_cast(Platform::AtomicRead((volatile int64*)&_capacity)); } /// @@ -232,7 +232,7 @@ public: void Clear() { _locker.Lock(); - Memory::DestructItems(_allocation.Get(), (int32)_count); + Memory::DestructItems(_allocation.Get(), static_cast(_count)); _count = 0; _locker.Unlock(); } @@ -248,8 +248,8 @@ public: return; _locker.Lock(); ASSERT(capacity >= 0); - const int32 count = preserveContents ? ((int32)_count < capacity ? (int32)_count : capacity) : 0; - _allocation.Relocate(capacity, (int32)_count, count); + const int32 count = preserveContents ? (static_cast(_count) < capacity ? static_cast(_count) : capacity) : 0; + _allocation.Relocate(capacity, static_cast(_count), count); Platform::AtomicStore(&_capacity, capacity); Platform::AtomicStore(&_count, count); _locker.Unlock(); @@ -265,12 +265,12 @@ public: _locker.Lock(); if (_count > size) { - Memory::DestructItems(_allocation.Get() + size, (int32)_count - size); + Memory::DestructItems(_allocation.Get() + size, static_cast(_count) - size); } else { EnsureCapacity(size, preserveContents); - Memory::ConstructItems(_allocation.Get() + _count, size - (int32)_count); + Memory::ConstructItems(_allocation.Get() + _count, size - static_cast(_count)); } _count = size; _locker.Unlock(); @@ -283,11 +283,11 @@ public: void EnsureCapacity(int32 minCapacity) { _locker.Lock(); - int32 capacity = (int32)Platform::AtomicRead(&_capacity); + int32 capacity = static_cast(Platform::AtomicRead(&_capacity)); if (capacity < minCapacity) { capacity = _allocation.CalculateCapacityGrow(capacity, minCapacity); - const int32 count = (int32)_count; + const int32 count = static_cast(_count); _allocation.Relocate(capacity, count, count); Platform::AtomicStore(&_capacity, capacity); } @@ -324,8 +324,8 @@ private: int32 AddOne() { Platform::InterlockedIncrement(&_threadsAdding); - int32 count = (int32)Platform::AtomicRead(&_count); - int32 capacity = (int32)Platform::AtomicRead(&_capacity); + int32 count = static_cast(Platform::AtomicRead(&_count)); + int32 capacity = static_cast(Platform::AtomicRead(&_capacity)); const int32 minCapacity = GetMinCapacity(count); if (minCapacity > capacity || Platform::AtomicRead(&_threadsResizing)) // Resize if not enough space or someone else is already doing it (don't add mid-resizing) { @@ -340,7 +340,7 @@ private: // Thread-safe resizing _locker.Lock(); - capacity = (int32)Platform::AtomicRead(&_capacity); + capacity = static_cast(Platform::AtomicRead(&_capacity)); if (capacity < minCapacity) { if (Platform::AtomicRead(&_threadsAdding)) @@ -350,7 +350,7 @@ private: goto RETRY; } capacity = _allocation.CalculateCapacityGrow(capacity, minCapacity); - count = (int32)Platform::AtomicRead(&_count); + count = static_cast(Platform::AtomicRead(&_count)); _allocation.Relocate(capacity, count, count); Platform::AtomicStore(&_capacity, capacity); } @@ -362,7 +362,7 @@ private: // Let other thread enter resizing-area _locker.Unlock(); } - return (int32)Platform::InterlockedIncrement(&_count) - 1; + return static_cast(Platform::InterlockedIncrement(&_count)) - 1; } FORCE_INLINE static int32 GetMinCapacity(const int32 count) From f77f551b72f88b4fdd9161dd91cce58be344a75d Mon Sep 17 00:00:00 2001 From: Mateusz Karbowiak <69864511+mtszkarbowiak@users.noreply.github.com> Date: Wed, 30 Oct 2024 22:22:56 +0100 Subject: [PATCH 03/13] Collections type aliasing fix This one is debatable. It follows modern C++. --- Source/Engine/Core/Collections/Array.h | 4 ++-- Source/Engine/Core/Collections/BitArray.h | 4 ++-- Source/Engine/Core/Collections/Dictionary.h | 2 +- Source/Engine/Core/Collections/HashSet.h | 2 +- Source/Engine/Core/Collections/RingBuffer.h | 4 ++-- Source/Engine/Core/Memory/Allocation.h | 2 +- Source/Engine/Renderer/RenderListBuffer.h | 4 ++-- 7 files changed, 11 insertions(+), 11 deletions(-) diff --git a/Source/Engine/Core/Collections/Array.h b/Source/Engine/Core/Collections/Array.h index 8f73cfba7..f60d0dec1 100644 --- a/Source/Engine/Core/Collections/Array.h +++ b/Source/Engine/Core/Collections/Array.h @@ -17,8 +17,8 @@ API_CLASS(InBuild) class Array { friend Array; public: - typedef T ItemType; - typedef typename AllocationType::template Data AllocationData; + using ItemType = T; + using AllocationData = typename AllocationType::template Data; private: int32 _count; diff --git a/Source/Engine/Core/Collections/BitArray.h b/Source/Engine/Core/Collections/BitArray.h index 97674974c..c62362780 100644 --- a/Source/Engine/Core/Collections/BitArray.h +++ b/Source/Engine/Core/Collections/BitArray.h @@ -14,8 +14,8 @@ API_CLASS(InBuild) class BitArray { friend BitArray; public: - typedef uint64 ItemType; - typedef typename AllocationType::template Data AllocationData; + using ItemType = uint64; + using AllocationData = typename AllocationType::template Data; private: int32 _count; diff --git a/Source/Engine/Core/Collections/Dictionary.h b/Source/Engine/Core/Collections/Dictionary.h index 438e1bb75..468025ffe 100644 --- a/Source/Engine/Core/Collections/Dictionary.h +++ b/Source/Engine/Core/Collections/Dictionary.h @@ -102,7 +102,7 @@ public: } }; - typedef typename AllocationType::template Data AllocationData; + using AllocationData = typename AllocationType::template Data; private: int32 _elementsCount = 0; diff --git a/Source/Engine/Core/Collections/HashSet.h b/Source/Engine/Core/Collections/HashSet.h index 1bfcc75eb..0be793d96 100644 --- a/Source/Engine/Core/Collections/HashSet.h +++ b/Source/Engine/Core/Collections/HashSet.h @@ -85,7 +85,7 @@ public: } }; - typedef typename AllocationType::template Data AllocationData; + using AllocationData = typename AllocationType::template Data; private: int32 _elementsCount = 0; diff --git a/Source/Engine/Core/Collections/RingBuffer.h b/Source/Engine/Core/Collections/RingBuffer.h index 1bc0ae5f4..9d1c16483 100644 --- a/Source/Engine/Core/Collections/RingBuffer.h +++ b/Source/Engine/Core/Collections/RingBuffer.h @@ -14,8 +14,8 @@ template class RingBuffer { public: - typedef T ItemType; - typedef typename AllocationType::template Data AllocationData; + using ItemType = T; + using AllocationData = typename AllocationType::template Data; private: int32 _front = 0, _back = 0, _count = 0, _capacity = 0; diff --git a/Source/Engine/Core/Memory/Allocation.h b/Source/Engine/Core/Memory/Allocation.h index bdf30a16d..5fce2ad73 100644 --- a/Source/Engine/Core/Memory/Allocation.h +++ b/Source/Engine/Core/Memory/Allocation.h @@ -279,4 +279,4 @@ public: }; }; -typedef HeapAllocation DefaultAllocation; +using DefaultAllocation = HeapAllocation; diff --git a/Source/Engine/Renderer/RenderListBuffer.h b/Source/Engine/Renderer/RenderListBuffer.h index bbe087e6e..d12c430f1 100644 --- a/Source/Engine/Renderer/RenderListBuffer.h +++ b/Source/Engine/Renderer/RenderListBuffer.h @@ -17,8 +17,8 @@ class RenderListBuffer { friend RenderListBuffer; public: - typedef T ItemType; - typedef typename AllocationType::template Data AllocationData; + using ItemType = T; + using AllocationData = typename AllocationType::template Data; private: volatile int64 _count; From 5439efc5591e1ef1813ceff5c79cb71da7d64989 Mon Sep 17 00:00:00 2001 From: Mateusz Karbowiak <69864511+mtszkarbowiak@users.noreply.github.com> Date: Wed, 30 Oct 2024 22:29:26 +0100 Subject: [PATCH 04/13] Collections implicit cast constructor fix This one prohibits annoying casts. It is also important for future context injection and follows STL practice. --- Source/Engine/Core/Collections/Array.h | 2 +- Source/Engine/Core/Collections/BitArray.h | 2 +- Source/Engine/Core/Collections/Dictionary.h | 2 +- Source/Engine/Core/Collections/HashSet.h | 2 +- Source/Engine/Renderer/RenderListBuffer.h | 2 +- 5 files changed, 5 insertions(+), 5 deletions(-) diff --git a/Source/Engine/Core/Collections/Array.h b/Source/Engine/Core/Collections/Array.h index f60d0dec1..e096992f8 100644 --- a/Source/Engine/Core/Collections/Array.h +++ b/Source/Engine/Core/Collections/Array.h @@ -52,7 +52,7 @@ public: /// Initializes a new instance of the class. /// /// The initial capacity. - Array(int32 capacity) + explicit Array(int32 capacity) : _count(0) , _capacity(capacity) { diff --git a/Source/Engine/Core/Collections/BitArray.h b/Source/Engine/Core/Collections/BitArray.h index c62362780..70d51bac0 100644 --- a/Source/Engine/Core/Collections/BitArray.h +++ b/Source/Engine/Core/Collections/BitArray.h @@ -46,7 +46,7 @@ public: /// Initializes a new instance of the class. /// /// The initial capacity. - BitArray(const int32 capacity) + explicit BitArray(const int32 capacity) : _count(0) , _capacity(capacity) { diff --git a/Source/Engine/Core/Collections/Dictionary.h b/Source/Engine/Core/Collections/Dictionary.h index 468025ffe..ad8793d36 100644 --- a/Source/Engine/Core/Collections/Dictionary.h +++ b/Source/Engine/Core/Collections/Dictionary.h @@ -149,7 +149,7 @@ public: /// Initializes a new instance of the class. /// /// The initial capacity. - Dictionary(const int32 capacity) + explicit Dictionary(const int32 capacity) { SetCapacity(capacity); } diff --git a/Source/Engine/Core/Collections/HashSet.h b/Source/Engine/Core/Collections/HashSet.h index 0be793d96..1c31eaef4 100644 --- a/Source/Engine/Core/Collections/HashSet.h +++ b/Source/Engine/Core/Collections/HashSet.h @@ -130,7 +130,7 @@ public: /// Initializes a new instance of the class. /// /// The initial capacity. - HashSet(const int32 capacity) + explicit HashSet(const int32 capacity) { SetCapacity(capacity); } diff --git a/Source/Engine/Renderer/RenderListBuffer.h b/Source/Engine/Renderer/RenderListBuffer.h index d12c430f1..50f6ed519 100644 --- a/Source/Engine/Renderer/RenderListBuffer.h +++ b/Source/Engine/Renderer/RenderListBuffer.h @@ -42,7 +42,7 @@ public: /// Initializes a new instance of the class. /// /// The initial capacity. - RenderListBuffer(int32 capacity) + explicit RenderListBuffer(int32 capacity) : _count(0) , _capacity(capacity) { From 8cb7fb48ceb817f2c15d0cff95f093a94c7e7111 Mon Sep 17 00:00:00 2001 From: Mateusz Karbowiak <69864511+mtszkarbowiak@users.noreply.github.com> Date: Wed, 30 Oct 2024 22:35:36 +0100 Subject: [PATCH 05/13] Dictionary returning const item fix Returning const object does not prevent from using mutable by triggering copy constructor. --- Source/Engine/Core/Collections/Dictionary.h | 4 ++-- Source/Engine/Core/Collections/HashSet.h | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/Source/Engine/Core/Collections/Dictionary.h b/Source/Engine/Core/Collections/Dictionary.h index ad8793d36..fd035e6f4 100644 --- a/Source/Engine/Core/Collections/Dictionary.h +++ b/Source/Engine/Core/Collections/Dictionary.h @@ -862,14 +862,14 @@ public: return Iterator(this, _size); } - const Iterator begin() const + Iterator begin() const { Iterator i(this, -1); ++i; return i; } - FORCE_INLINE const Iterator end() const + FORCE_INLINE Iterator end() const { return Iterator(this, _size); } diff --git a/Source/Engine/Core/Collections/HashSet.h b/Source/Engine/Core/Collections/HashSet.h index 1c31eaef4..af9e59566 100644 --- a/Source/Engine/Core/Collections/HashSet.h +++ b/Source/Engine/Core/Collections/HashSet.h @@ -656,14 +656,14 @@ public: return Iterator(this, _size); } - const Iterator begin() const + Iterator begin() const { Iterator i(this, -1); ++i; return i; } - FORCE_INLINE const Iterator end() const + FORCE_INLINE Iterator end() const { return Iterator(this, _size); } From 66b6a29ed405846821b583069a26859665828116 Mon Sep 17 00:00:00 2001 From: Mateusz Karbowiak <69864511+mtszkarbowiak@users.noreply.github.com> Date: Thu, 31 Oct 2024 00:19:12 +0100 Subject: [PATCH 06/13] Collections iterator move noexcept fix --- Source/Engine/Core/Collections/Dictionary.h | 4 ++-- Source/Engine/Core/Collections/HashSet.h | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/Source/Engine/Core/Collections/Dictionary.h b/Source/Engine/Core/Collections/Dictionary.h index fd035e6f4..32f61a30c 100644 --- a/Source/Engine/Core/Collections/Dictionary.h +++ b/Source/Engine/Core/Collections/Dictionary.h @@ -289,7 +289,7 @@ public: { } - Iterator(Iterator&& i) + Iterator(Iterator&& i) noexcept : _collection(i._collection) , _index(i._index) { @@ -348,7 +348,7 @@ public: return *this; } - Iterator& operator=(Iterator&& v) + Iterator& operator=(Iterator&& v) noexcept { _collection = v._collection; _index = v._index; diff --git a/Source/Engine/Core/Collections/HashSet.h b/Source/Engine/Core/Collections/HashSet.h index af9e59566..a5bf8ae3e 100644 --- a/Source/Engine/Core/Collections/HashSet.h +++ b/Source/Engine/Core/Collections/HashSet.h @@ -270,7 +270,7 @@ public: { } - Iterator(Iterator&& i) + Iterator(Iterator&& i) noexcept : _collection(i._collection) , _index(i._index) { @@ -329,7 +329,7 @@ public: return *this; } - Iterator& operator=(Iterator&& v) + Iterator& operator=(Iterator&& v) noexcept { _collection = v._collection; _index = v._index; From 9c448f75d8c13acfdbd4d05ba42b2af597a7c7da Mon Sep 17 00:00:00 2001 From: Mateusz Karbowiak <69864511+mtszkarbowiak@users.noreply.github.com> Date: Thu, 31 Oct 2024 00:29:38 +0100 Subject: [PATCH 07/13] Collections const correctness fix (style) --- Source/Engine/Core/Collections/Array.h | 44 ++++++++++----------- Source/Engine/Core/Collections/BitArray.h | 2 +- Source/Engine/Core/Collections/Dictionary.h | 2 +- Source/Engine/Core/Collections/HashSet.h | 8 ++-- Source/Engine/Renderer/RenderListBuffer.h | 12 +++--- 5 files changed, 34 insertions(+), 34 deletions(-) diff --git a/Source/Engine/Core/Collections/Array.h b/Source/Engine/Core/Collections/Array.h index e096992f8..3006d045e 100644 --- a/Source/Engine/Core/Collections/Array.h +++ b/Source/Engine/Core/Collections/Array.h @@ -25,7 +25,7 @@ private: int32 _capacity; AllocationData _allocation; - FORCE_INLINE static void MoveToEmpty(AllocationData& to, AllocationData& from, int32 fromCount, int32 fromCapacity) + FORCE_INLINE static void MoveToEmpty(AllocationData& to, AllocationData& from, const int32 fromCount, const int32 fromCapacity) { if IF_CONSTEXPR (AllocationType::HasSwap) to.Swap(from); @@ -52,7 +52,7 @@ public: /// Initializes a new instance of the class. /// /// The initial capacity. - explicit Array(int32 capacity) + explicit Array(const int32 capacity) : _count(0) , _capacity(capacity) { @@ -79,7 +79,7 @@ public: /// /// The initial data. /// The amount of items. - Array(const T* data, int32 length) + Array(const T* data, const int32 length) { ASSERT(length >= 0); _count = _capacity = length; @@ -255,7 +255,7 @@ public: /// /// The index. /// true if is valid a index; otherwise, false. - bool IsValidIndex(int32 index) const + bool IsValidIndex(const int32 index) const { return index < _count && index >= 0; } @@ -280,7 +280,7 @@ public: /// Gets item at the given index. /// /// The reference to the item. - FORCE_INLINE T& At(int32 index) + FORCE_INLINE T& At(const int32 index) { ASSERT(index >= 0 && index < _count); return _allocation.Get()[index]; @@ -290,7 +290,7 @@ public: /// Gets item at the given index. /// /// The reference to the item. - FORCE_INLINE const T& At(int32 index) const + FORCE_INLINE const T& At(const int32 index) const { ASSERT(index >= 0 && index < _count); return _allocation.Get()[index]; @@ -300,7 +300,7 @@ public: /// Gets or sets the item at the given index. /// /// The reference to the item. - FORCE_INLINE T& operator[](int32 index) + FORCE_INLINE T& operator[](const int32 index) { ASSERT(index >= 0 && index < _count); return _allocation.Get()[index]; @@ -310,7 +310,7 @@ public: /// Gets the item at the given index. /// /// The reference to the item. - FORCE_INLINE const T& operator[](int32 index) const + FORCE_INLINE const T& operator[](const int32 index) const { ASSERT(index >= 0 && index < _count); return _allocation.Get()[index]; @@ -406,7 +406,7 @@ public: /// /// The new capacity. /// True if preserve collection data when changing its size, otherwise collection after resize will be empty. - void SetCapacity(const int32 capacity, bool preserveContents = true) + void SetCapacity(const int32 capacity, const bool preserveContents = true) { if (capacity == _capacity) return; @@ -422,7 +422,7 @@ public: /// /// The new collection size. /// True if preserve collection data when changing its size, otherwise collection after resize might not contain the previous data. - void Resize(int32 size, bool preserveContents = true) + void Resize(const int32 size, const bool preserveContents = true) { if (_count > size) { @@ -441,7 +441,7 @@ public: /// /// The minimum capacity. /// True if preserve collection data when changing its size, otherwise collection after resize will be empty. - void EnsureCapacity(int32 minCapacity, bool preserveContents = true) + void EnsureCapacity(const int32 minCapacity, const bool preserveContents = true) { if (_capacity < minCapacity) { @@ -466,7 +466,7 @@ public: /// /// The data. /// The amount of items. - void Set(const T* data, int32 count) + void Set(const T* data, const int32 count) { EnsureCapacity(count, false); Memory::DestructItems(_allocation.Get(), _count); @@ -501,7 +501,7 @@ public: /// /// The items to add. /// The items count. - void Add(const T* items, int32 count) + void Add(const T* items, const int32 count) { EnsureCapacity(_count + count); Memory::ConstructItems(_allocation.Get() + _count, items, count); @@ -532,7 +532,7 @@ public: /// Adds the given amount of items to the collection. /// /// The items count. - FORCE_INLINE void AddDefault(int32 count = 1) + FORCE_INLINE void AddDefault(const int32 count = 1) { EnsureCapacity(_count + count); Memory::ConstructItems(_allocation.Get() + _count, count); @@ -543,7 +543,7 @@ public: /// Adds the given amount of uninitialized items to the collection without calling the constructor. /// /// The items count. - FORCE_INLINE void AddUninitialized(int32 count = 1) + FORCE_INLINE void AddUninitialized(const int32 count = 1) { EnsureCapacity(_count + count); _count += count; @@ -568,7 +568,7 @@ public: /// Warning! AddZeroed() will create items without calling the constructor and this is not appropriate for item types that require a constructor to function properly. /// /// The number of new items to add. - void AddZeroed(int32 count = 1) + void AddZeroed(const int32 count = 1) { EnsureCapacity(_count + count); Platform::MemoryClear(_allocation.Get() + _count, count * sizeof(T)); @@ -580,7 +580,7 @@ public: /// /// The zero-based index at which item should be inserted. /// The item to be inserted by copying. - void Insert(int32 index, const T& item) + void Insert(const int32 index, const T& item) { ASSERT(index >= 0 && index <= _count); EnsureCapacity(_count + 1); @@ -597,7 +597,7 @@ public: /// /// The zero-based index at which item should be inserted. /// The item to inserted by moving. - void Insert(int32 index, T&& item) + void Insert(const int32 index, T&& item) { ASSERT(index >= 0 && index <= _count); EnsureCapacity(_count + 1); @@ -613,7 +613,7 @@ public: /// Insert the given item at specified index with keeping items order. /// /// The zero-based index at which item should be inserted. - void Insert(int32 index) + void Insert(const int32 index) { ASSERT(index >= 0 && index <= _count); EnsureCapacity(_count + 1); @@ -954,7 +954,7 @@ public: { } - Iterator(Array const* array, const int32 index) + Iterator(const Array* array, const int32 index) : _array(const_cast(array)) , _index(index) { @@ -1084,7 +1084,7 @@ public: }; template -void* operator new(size_t size, Array& array) +void* operator new(const size_t size, Array& array) { ASSERT(size == sizeof(T)); const int32 index = array.Count(); @@ -1093,7 +1093,7 @@ void* operator new(size_t size, Array& array) } template -void* operator new(size_t size, Array& array, int32 index) +void* operator new(const size_t size, Array& array, const int32 index) { ASSERT(size == sizeof(T)); array.Insert(index); diff --git a/Source/Engine/Core/Collections/BitArray.h b/Source/Engine/Core/Collections/BitArray.h index 70d51bac0..bc101f4ae 100644 --- a/Source/Engine/Core/Collections/BitArray.h +++ b/Source/Engine/Core/Collections/BitArray.h @@ -278,7 +278,7 @@ public: /// /// The minimum capacity. /// True if preserve collection data when changing its size, otherwise collection after resize will be empty. - void EnsureCapacity(int32 minCapacity, const bool preserveContents = true) + void EnsureCapacity(const int32 minCapacity, const bool preserveContents = true) { if (_capacity < minCapacity) { diff --git a/Source/Engine/Core/Collections/Dictionary.h b/Source/Engine/Core/Collections/Dictionary.h index 32f61a30c..2b1bb8916 100644 --- a/Source/Engine/Core/Collections/Dictionary.h +++ b/Source/Engine/Core/Collections/Dictionary.h @@ -110,7 +110,7 @@ private: int32 _size = 0; AllocationData _allocation; - FORCE_INLINE static void MoveToEmpty(AllocationData& to, AllocationData& from, int32 fromSize) + FORCE_INLINE static void MoveToEmpty(AllocationData& to, AllocationData& from, const int32 fromSize) { if IF_CONSTEXPR (AllocationType::HasSwap) to.Swap(from); diff --git a/Source/Engine/Core/Collections/HashSet.h b/Source/Engine/Core/Collections/HashSet.h index a5bf8ae3e..1974051e5 100644 --- a/Source/Engine/Core/Collections/HashSet.h +++ b/Source/Engine/Core/Collections/HashSet.h @@ -93,7 +93,7 @@ private: int32 _size = 0; AllocationData _allocation; - FORCE_INLINE static void MoveToEmpty(AllocationData& to, AllocationData& from, int32 fromSize) + FORCE_INLINE static void MoveToEmpty(AllocationData& to, AllocationData& from, const int32 fromSize) { if IF_CONSTEXPR (AllocationType::HasSwap) to.Swap(from); @@ -252,7 +252,7 @@ public: { } - Iterator(HashSet const* collection, const int32 index) + Iterator(const HashSet* collection, const int32 index) : _collection(const_cast(collection)) , _index(index) { @@ -307,7 +307,7 @@ public: return _index >= 0 && _index < _collection->_size; } - FORCE_INLINE bool operator !() const + FORCE_INLINE bool operator!() const { return !(bool)*this; } @@ -474,7 +474,7 @@ public: /// /// The minimum required capacity. /// True if preserve collection data when changing its size, otherwise collection after resize will be empty. - void EnsureCapacity(int32 minCapacity, const bool preserveContents = true) + void EnsureCapacity(const int32 minCapacity, const bool preserveContents = true) { if (_size >= minCapacity) return; diff --git a/Source/Engine/Renderer/RenderListBuffer.h b/Source/Engine/Renderer/RenderListBuffer.h index 50f6ed519..e6a22949a 100644 --- a/Source/Engine/Renderer/RenderListBuffer.h +++ b/Source/Engine/Renderer/RenderListBuffer.h @@ -42,7 +42,7 @@ public: /// Initializes a new instance of the class. /// /// The initial capacity. - explicit RenderListBuffer(int32 capacity) + explicit RenderListBuffer(const int32 capacity) : _count(0) , _capacity(capacity) { @@ -55,7 +55,7 @@ public: /// /// The initial data. /// The amount of items. - RenderListBuffer(const T* data, int32 length) + RenderListBuffer(const T* data, const int32 length) { ASSERT(length >= 0); _count = _capacity = length; @@ -188,7 +188,7 @@ public: /// Gets or sets the item at the given index. /// /// The reference to the item. - FORCE_INLINE T& operator[](int32 index) + FORCE_INLINE T& operator[](const int32 index) { ASSERT(index >= 0 && index < Count()); return _allocation.Get()[index]; @@ -198,7 +198,7 @@ public: /// Gets the item at the given index. /// /// The reference to the item. - FORCE_INLINE const T& operator[](int32 index) const + FORCE_INLINE const T& operator[](const int32 index) const { ASSERT(index >= 0 && index < Count()); return _allocation.Get()[index]; @@ -260,7 +260,7 @@ public: /// /// The new collection size. /// True if preserve collection data when changing its size, otherwise collection after resize might not contain the previous data. - void Resize(int32 size, bool preserveContents = true) + void Resize(const int32 size, const bool preserveContents = true) { _locker.Lock(); if (_count > size) @@ -280,7 +280,7 @@ public: /// Ensures the collection has given capacity (or more). /// /// The minimum capacity. - void EnsureCapacity(int32 minCapacity) + void EnsureCapacity(const int32 minCapacity) { _locker.Lock(); int32 capacity = static_cast(Platform::AtomicRead(&_capacity)); From fbb840dff341096cab6646812264de4a80afa41e Mon Sep 17 00:00:00 2001 From: Mateusz Karbowiak <69864511+mtszkarbowiak@users.noreply.github.com> Date: Thu, 31 Oct 2024 00:45:00 +0100 Subject: [PATCH 08/13] Collections de/increment operations fix This one is more stylistic but is consistent with practice of using pre- operations for iterators. --- Source/Engine/Core/Collections/Array.h | 28 +++++++++---------- Source/Engine/Core/Collections/BitArray.h | 6 ++--- Source/Engine/Core/Collections/Dictionary.h | 30 +++++++++++---------- Source/Engine/Core/Collections/HashSet.h | 30 +++++++++++---------- Source/Engine/Core/Collections/RingBuffer.h | 4 +-- 5 files changed, 51 insertions(+), 47 deletions(-) diff --git a/Source/Engine/Core/Collections/Array.h b/Source/Engine/Core/Collections/Array.h index 3006d045e..ec2d28f49 100644 --- a/Source/Engine/Core/Collections/Array.h +++ b/Source/Engine/Core/Collections/Array.h @@ -482,7 +482,7 @@ public: { EnsureCapacity(_count + 1); Memory::ConstructItems(_allocation.Get() + _count, &item, 1); - _count++; + ++_count; } /// @@ -493,7 +493,7 @@ public: { EnsureCapacity(_count + 1); Memory::MoveItems(_allocation.Get() + _count, &item, 1); - _count++; + ++_count; } /// @@ -557,7 +557,7 @@ public: { EnsureCapacity(_count + 1); Memory::ConstructItems(_allocation.Get() + _count, 1); - _count++; + ++_count; return _allocation.Get()[_count - 1]; } @@ -605,7 +605,7 @@ public: Memory::ConstructItems(data + _count, 1); for (int32 i = _count - 1; i >= index; i--) data[i + 1] = MoveTemp(data[i]); - _count++; + ++_count; data[index] = MoveTemp(item); } @@ -621,7 +621,7 @@ public: Memory::ConstructItems(data + _count, 1); for (int32 i = _count - 1; i >= index; i--) data[i + 1] = data[i]; - _count++; + ++_count; } /// @@ -661,7 +661,7 @@ public: /// The item to remove. void RemoveAllKeepOrder(const T& item) { - for (int32 i = Count() - 1; i >= 0; i--) + for (int32 i = Count() - 1; i >= 0; --i) { if (_allocation.Get()[i] == item) { @@ -679,14 +679,14 @@ public: void RemoveAtKeepOrder(const int32 index) { ASSERT(index < _count && index >= 0); - _count--; + --_count; T* data = _allocation.Get(); if (index < _count) { T* dst = data + index; T* src = data + (index + 1); const int32 count = _count - index; - for (int32 i = 0; i < count; i++) + for (int32 i = 0; i < count; ++i) dst[i] = MoveTemp(src[i]); } Memory::DestructItems(data + _count, 1); @@ -712,7 +712,7 @@ public: /// The item to remove. void RemoveAll(const T& item) { - for (int32 i = Count() - 1; i >= 0; i--) + for (int32 i = Count() - 1; i >= 0; --i) { if (_allocation.Get()[i] == item) { @@ -730,7 +730,7 @@ public: void RemoveAt(const int32 index) { ASSERT(index < _count && index >= 0); - _count--; + --_count; T* data = _allocation.Get(); if (_count) data[index] = data[_count]; @@ -743,7 +743,7 @@ public: void RemoveLast() { ASSERT(_count > 0); - _count--; + --_count; Memory::DestructItems(_allocation.Get() + _count, 1); } @@ -772,7 +772,7 @@ public: { T* data = _allocation.Get(); const int32 count = _count / 2; - for (int32 i = 0; i < count; i++) + for (int32 i = 0; i < count; ++i) ::Swap(data[i], data[_count - i - 1]); } @@ -1052,7 +1052,7 @@ public: Iterator& operator--() { if (_index > 0) - _index--; + --_index; return *this; } @@ -1060,7 +1060,7 @@ public: { Iterator temp = *this; if (_index > 0) - _index--; + --_index; return temp; } }; diff --git a/Source/Engine/Core/Collections/BitArray.h b/Source/Engine/Core/Collections/BitArray.h index bc101f4ae..f0a08d8e2 100644 --- a/Source/Engine/Core/Collections/BitArray.h +++ b/Source/Engine/Core/Collections/BitArray.h @@ -304,7 +304,7 @@ public: void Add(const bool item) { EnsureCapacity(_count + 1); - _count++; + ++_count; Set(_count - 1, item); } @@ -316,7 +316,7 @@ public: void Add(const bool* items, const int32 count) { EnsureCapacity(_count + count); - for (int32 i = 0; i < count; i++) + for (int32 i = 0; i < count; ++i) Add(items[i]); } @@ -327,7 +327,7 @@ public: void Add(const BitArray& other) { EnsureCapacity(_count, other.Count()); - for (int32 i = 0; i < other.Count(); i++) + for (int32 i = 0; i < other.Count(); ++i) Add(other[i]); } diff --git a/Source/Engine/Core/Collections/Dictionary.h b/Source/Engine/Core/Collections/Dictionary.h index 2b1bb8916..e9dc8f582 100644 --- a/Source/Engine/Core/Collections/Dictionary.h +++ b/Source/Engine/Core/Collections/Dictionary.h @@ -363,8 +363,9 @@ public: const Bucket* data = _collection->_allocation.Get(); do { - _index++; - } while (_index != capacity && data[_index].IsNotOccupied()); + ++_index; + } + while (_index != capacity && data[_index].IsNotOccupied()); } return *this; } @@ -383,8 +384,9 @@ public: const Bucket* data = _collection->_allocation.Get(); do { - _index--; - } while (_index > 0 && data[_index].IsNotOccupied()); + --_index; + } + while (_index > 0 && data[_index].IsNotOccupied()); } return *this; } @@ -423,7 +425,7 @@ public: // Insert ASSERT(pos.FreeSlotIndex != -1); - _elementsCount++; + ++_elementsCount; Bucket& bucket = _allocation.Get()[pos.FreeSlotIndex]; bucket.Occupy(key); return bucket.Value; @@ -582,7 +584,7 @@ public: Memory::MoveItems(&bucket->Key, &oldBucket.Key, 1); Memory::MoveItems(&bucket->Value, &oldBucket.Value, 1); bucket->_state = Bucket::Occupied; - _elementsCount++; + ++_elementsCount; } } } @@ -682,8 +684,8 @@ public: if (pos.ObjectIndex != -1) { _allocation.Get()[pos.ObjectIndex].Delete(); - _elementsCount--; - _deletedCount++; + --_elementsCount; + ++_deletedCount; return true; } return false; @@ -701,8 +703,8 @@ public: { ASSERT(_allocation.Get()[i._index].IsOccupied()); _allocation.Get()[i._index].Delete(); - _elementsCount--; - _deletedCount++; + --_elementsCount; + ++_deletedCount; return true; } return false; @@ -721,7 +723,7 @@ public: if (i->Value == value) { Remove(i); - result++; + ++result; } } return result; @@ -768,7 +770,7 @@ public: if (HasItems()) { const Bucket* data = _allocation.Get(); - for (int32 i = 0; i < _size; i++) + for (int32 i = 0; i < _size; ++i) { if (data[i].IsOccupied() && data[i].Value == value) return true; @@ -788,7 +790,7 @@ public: if (HasItems()) { const Bucket* data = _allocation.Get(); - for (int32 i = 0; i < _size; i++) + for (int32 i = 0; i < _size; ++i) { if (data[i].IsOccupied() && data[i].Value == value) { @@ -928,7 +930,7 @@ private: result.ObjectIndex = bucketIndex; return; } - checksCount++; + ++checksCount; bucketIndex = (bucketIndex + DICTIONARY_PROB_FUNC(_size, checksCount)) & tableSizeMinusOne; } result.ObjectIndex = -1; diff --git a/Source/Engine/Core/Collections/HashSet.h b/Source/Engine/Core/Collections/HashSet.h index 1974051e5..2e90dae4b 100644 --- a/Source/Engine/Core/Collections/HashSet.h +++ b/Source/Engine/Core/Collections/HashSet.h @@ -102,7 +102,7 @@ private: to.Allocate(fromSize); Bucket* toData = to.Get(); Bucket* fromData = from.Get(); - for (int32 i = 0; i < fromSize; i++) + for (int32 i = 0; i < fromSize; ++i) { Bucket& fromBucket = fromData[i]; if (fromBucket.IsOccupied()) @@ -344,8 +344,9 @@ public: const Bucket* data = _collection->_allocation.Get(); do { - _index++; - } while (_index != capacity && data[_index].IsNotOccupied()); + ++_index; + } + while (_index != capacity && data[_index].IsNotOccupied()); } return *this; } @@ -364,8 +365,9 @@ public: const Bucket* data = _collection->_allocation.Get(); do { - _index--; - } while (_index > 0 && data[_index].IsNotOccupied()); + --_index; + } + while (_index > 0 && data[_index].IsNotOccupied()); } return *this; } @@ -559,8 +561,8 @@ public: if (pos.ObjectIndex != -1) { _allocation.Get()[pos.ObjectIndex].Delete(); - _elementsCount--; - _deletedCount++; + --_elementsCount; + ++_deletedCount; return true; } return false; @@ -578,8 +580,8 @@ public: { ASSERT(_allocation.Get()[i._index].IsOccupied()); _allocation.Get()[i._index].Delete(); - _elementsCount--; - _deletedCount++; + --_elementsCount; + ++_deletedCount; return true; } return false; @@ -750,7 +752,7 @@ private: // Insert ASSERT(pos.FreeSlotIndex != -1); - _elementsCount++; + ++_elementsCount; return &_allocation.Get()[pos.FreeSlotIndex]; } @@ -760,7 +762,7 @@ private: { // Fast path if it's empty Bucket* data = _allocation.Get(); - for (int32 i = 0; i < _size; i++) + for (int32 i = 0; i < _size; ++i) data[i]._state = Bucket::Empty; } else @@ -770,11 +772,11 @@ private: MoveToEmpty(oldAllocation, _allocation, _size); _allocation.Allocate(_size); Bucket* data = _allocation.Get(); - for (int32 i = 0; i < _size; i++) + for (int32 i = 0; i < _size; ++i) data[i]._state = Bucket::Empty; Bucket* oldData = oldAllocation.Get(); FindPositionResult pos; - for (int32 i = 0; i < _size; i++) + for (int32 i = 0; i < _size; ++i) { Bucket& oldBucket = oldData[i]; if (oldBucket.IsOccupied()) @@ -786,7 +788,7 @@ private: bucket->_state = Bucket::Occupied; } } - for (int32 i = 0; i < _size; i++) + for (int32 i = 0; i < _size; ++i) oldData[i].Free(); } _deletedCount = 0; diff --git a/Source/Engine/Core/Collections/RingBuffer.h b/Source/Engine/Core/Collections/RingBuffer.h index 9d1c16483..c46f13031 100644 --- a/Source/Engine/Core/Collections/RingBuffer.h +++ b/Source/Engine/Core/Collections/RingBuffer.h @@ -62,7 +62,7 @@ public: } Memory::ConstructItems(_allocation.Get() + _back, &data, 1); _back = (_back + 1) % _capacity; - _count++; + ++_count; } FORCE_INLINE T& PeekFront() @@ -91,7 +91,7 @@ public: { Memory::DestructItems(_allocation.Get() + _front, 1); _front = (_front + 1) % _capacity; - _count--; + --_count; } void Clear() From 5d32ed7f8eb6ff7e237c5b7a5d824d8fea33514a Mon Sep 17 00:00:00 2001 From: Mateusz Karbowiak <69864511+mtszkarbowiak@users.noreply.github.com> Date: Thu, 31 Oct 2024 00:49:59 +0100 Subject: [PATCH 09/13] Collections casts constiness fix --- Source/Engine/Core/Collections/BitArray.h | 4 ++-- Source/Engine/Core/Collections/Dictionary.h | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/Source/Engine/Core/Collections/BitArray.h b/Source/Engine/Core/Collections/BitArray.h index f0a08d8e2..3ae573df6 100644 --- a/Source/Engine/Core/Collections/BitArray.h +++ b/Source/Engine/Core/Collections/BitArray.h @@ -215,7 +215,7 @@ public: ASSERT(index >= 0 && index < _count); const ItemType offset = index / sizeof(ItemType); const ItemType bitMask = static_cast((int32)(1 << (index & (static_cast(sizeof(ItemType)) - 1)))); - const ItemType item = static_cast(_allocation.Get())[offset]; + const ItemType item = static_cast(_allocation.Get())[offset]; return (item & bitMask) != 0; } @@ -229,7 +229,7 @@ public: ASSERT(index >= 0 && index < _count); const ItemType offset = index / sizeof(ItemType); const ItemType bitMask = static_cast((int32)(1 << (index & (static_cast(sizeof(ItemType)) - 1)))); - ItemType& item = reinterpret_cast(_allocation.Get())[offset]; + ItemType& item = reinterpret_cast(_allocation.Get())[offset]; if (value) item |= bitMask; else diff --git a/Source/Engine/Core/Collections/Dictionary.h b/Source/Engine/Core/Collections/Dictionary.h index e9dc8f582..169b7bc3f 100644 --- a/Source/Engine/Core/Collections/Dictionary.h +++ b/Source/Engine/Core/Collections/Dictionary.h @@ -500,7 +500,7 @@ public: FindPosition(key, pos); if (pos.ObjectIndex == -1) return nullptr; - return static_cast(&_allocation.Get()[pos.ObjectIndex].Value); + return static_cast(&_allocation.Get()[pos.ObjectIndex].Value); } public: From 2ae3932fcc60fb76f2bb2cf3a91144d217a60c3c Mon Sep 17 00:00:00 2001 From: Mateusz Karbowiak <69864511+mtszkarbowiak@users.noreply.github.com> Date: Thu, 31 Oct 2024 01:06:04 +0100 Subject: [PATCH 10/13] Revert BitArray changes --- Source/Engine/Core/Collections/BitArray.h | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/Source/Engine/Core/Collections/BitArray.h b/Source/Engine/Core/Collections/BitArray.h index 3ae573df6..1b601c72f 100644 --- a/Source/Engine/Core/Collections/BitArray.h +++ b/Source/Engine/Core/Collections/BitArray.h @@ -214,8 +214,8 @@ public: { ASSERT(index >= 0 && index < _count); const ItemType offset = index / sizeof(ItemType); - const ItemType bitMask = static_cast((int32)(1 << (index & (static_cast(sizeof(ItemType)) - 1)))); - const ItemType item = static_cast(_allocation.Get())[offset]; + const ItemType bitMask = (ItemType)(int32)(1 << (index & ((int32)sizeof(ItemType) - 1))); + const ItemType item = ((ItemType*)_allocation.Get())[offset]; return (item & bitMask) != 0; } @@ -228,12 +228,12 @@ public: { ASSERT(index >= 0 && index < _count); const ItemType offset = index / sizeof(ItemType); - const ItemType bitMask = static_cast((int32)(1 << (index & (static_cast(sizeof(ItemType)) - 1)))); - ItemType& item = reinterpret_cast(_allocation.Get())[offset]; + const ItemType bitMask = (ItemType)(int32)(1 << (index & ((int32)sizeof(ItemType) - 1))); + ItemType& item = ((ItemType*)_allocation.Get())[offset]; if (value) item |= bitMask; else - item &= ~bitMask; + item &= ~bitMask; // Clear the bit } public: From 7cca26bb97274cff0c7f4d48b774aac768cd34be Mon Sep 17 00:00:00 2001 From: Mateusz Karbowiak <69864511+mtszkarbowiak@users.noreply.github.com> Date: Thu, 31 Oct 2024 01:08:44 +0100 Subject: [PATCH 11/13] Dictionary cast constiness fix --- Source/Engine/Core/Collections/Dictionary.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Source/Engine/Core/Collections/Dictionary.h b/Source/Engine/Core/Collections/Dictionary.h index 169b7bc3f..e9dc8f582 100644 --- a/Source/Engine/Core/Collections/Dictionary.h +++ b/Source/Engine/Core/Collections/Dictionary.h @@ -500,7 +500,7 @@ public: FindPosition(key, pos); if (pos.ObjectIndex == -1) return nullptr; - return static_cast(&_allocation.Get()[pos.ObjectIndex].Value); + return static_cast(&_allocation.Get()[pos.ObjectIndex].Value); } public: From 26309a0d4108aa378b08a0ca337303c5b86a713f Mon Sep 17 00:00:00 2001 From: Mateusz Karbowiak <69864511+mtszkarbowiak@users.noreply.github.com> Date: Thu, 31 Oct 2024 01:15:05 +0100 Subject: [PATCH 12/13] Dictionary mutable key access for const accessor hack --- Source/Engine/Core/Collections/Dictionary.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Source/Engine/Core/Collections/Dictionary.h b/Source/Engine/Core/Collections/Dictionary.h index e9dc8f582..794cfc455 100644 --- a/Source/Engine/Core/Collections/Dictionary.h +++ b/Source/Engine/Core/Collections/Dictionary.h @@ -500,7 +500,7 @@ public: FindPosition(key, pos); if (pos.ObjectIndex == -1) return nullptr; - return static_cast(&_allocation.Get()[pos.ObjectIndex].Value); + return const_cast(&_allocation.Get()[pos.ObjectIndex].Value); //TODO This one is problematic. I think this entire method should be removed. } public: From dca48b335a87da5afb2e03ed0297013610329428 Mon Sep 17 00:00:00 2001 From: Mateusz Karbowiak <69864511+mtszkarbowiak@users.noreply.github.com> Date: Thu, 31 Oct 2024 02:41:51 +0100 Subject: [PATCH 13/13] Hash collections bucket state enum extracted This is a small compilation time optimization by reducing total number of generated types. Should not change runtime behavior. --- Source/Engine/Core/Collections/BucketState.h | 15 +++++++ Source/Engine/Core/Collections/Dictionary.h | 44 +++++++++----------- Source/Engine/Core/Collections/HashSet.h | 42 ++++++++----------- 3 files changed, 52 insertions(+), 49 deletions(-) create mode 100644 Source/Engine/Core/Collections/BucketState.h diff --git a/Source/Engine/Core/Collections/BucketState.h b/Source/Engine/Core/Collections/BucketState.h new file mode 100644 index 000000000..16cbdb845 --- /dev/null +++ b/Source/Engine/Core/Collections/BucketState.h @@ -0,0 +1,15 @@ +// Copyright (c) 2012-2024 Wojciech Figat. All rights reserved. + +#pragma once + +#include "Engine/Core/Types/BaseTypes.h" + +/// +/// Tells if the object is occupied, and if not, if the bucket is a subject of compaction. +/// +enum class BucketState : byte +{ + Empty = 0, + Deleted = 1, + Occupied = 2, +}; diff --git a/Source/Engine/Core/Collections/Dictionary.h b/Source/Engine/Core/Collections/Dictionary.h index 794cfc455..efb7bafcf 100644 --- a/Source/Engine/Core/Collections/Dictionary.h +++ b/Source/Engine/Core/Collections/Dictionary.h @@ -4,6 +4,7 @@ #include "Engine/Core/Memory/Memory.h" #include "Engine/Core/Memory/Allocation.h" +#include "Engine/Core/Collections/BucketState.h" #include "Engine/Core/Collections/HashFunctions.h" #include "Engine/Core/Collections/Config.h" @@ -25,34 +26,27 @@ public: { friend Dictionary; - enum State : byte - { - Empty = 0, - Deleted = 1, - Occupied = 2, - }; - /// The key. KeyType Key; /// The value. ValueType Value; private: - State _state; + BucketState _state; FORCE_INLINE void Free() { - if (_state == Occupied) + if (_state == BucketState::Occupied) { Memory::DestructItem(&Key); Memory::DestructItem(&Value); } - _state = Empty; + _state = BucketState::Empty; } FORCE_INLINE void Delete() { - _state = Deleted; + _state = BucketState::Deleted; Memory::DestructItem(&Key); Memory::DestructItem(&Value); } @@ -62,7 +56,7 @@ public: { Memory::ConstructItems(&Key, &key, 1); Memory::ConstructItem(&Value); - _state = Occupied; + _state = BucketState::Occupied; } template @@ -70,7 +64,7 @@ public: { Memory::ConstructItems(&Key, &key, 1); Memory::ConstructItems(&Value, &value, 1); - _state = Occupied; + _state = BucketState::Occupied; } template @@ -78,27 +72,27 @@ public: { Memory::ConstructItems(&Key, &key, 1); Memory::MoveItems(&Value, &value, 1); - _state = Occupied; + _state = BucketState::Occupied; } FORCE_INLINE bool IsEmpty() const { - return _state == Empty; + return _state == BucketState::Empty; } FORCE_INLINE bool IsDeleted() const { - return _state == Deleted; + return _state == BucketState::Deleted; } FORCE_INLINE bool IsOccupied() const { - return _state == Occupied; + return _state == BucketState::Occupied; } FORCE_INLINE bool IsNotOccupied() const { - return _state != Occupied; + return _state != BucketState::Occupied; } }; @@ -127,10 +121,10 @@ private: Bucket& toBucket = toData[i]; Memory::MoveItems(&toBucket.Key, &fromBucket.Key, 1); Memory::MoveItems(&toBucket.Value, &fromBucket.Value, 1); - toBucket._state = Bucket::Occupied; + toBucket._state = BucketState::Occupied; Memory::DestructItem(&fromBucket.Key); Memory::DestructItem(&fromBucket.Value); - fromBucket._state = Bucket::Empty; + fromBucket._state = BucketState::Empty; } } from.Free(); @@ -566,7 +560,7 @@ public: _allocation.Allocate(capacity); Bucket* data = _allocation.Get(); for (int32 i = 0; i < capacity; i++) - data[i]._state = Bucket::Empty; + data[i]._state = BucketState::Empty; } _size = capacity; Bucket* oldData = oldAllocation.Get(); @@ -583,7 +577,7 @@ public: Bucket* bucket = &_allocation.Get()[pos.FreeSlotIndex]; Memory::MoveItems(&bucket->Key, &oldBucket.Key, 1); Memory::MoveItems(&bucket->Value, &oldBucket.Value, 1); - bucket->_state = Bucket::Occupied; + bucket->_state = BucketState::Occupied; ++_elementsCount; } } @@ -967,7 +961,7 @@ private: // Fast path if it's empty Bucket* data = _allocation.Get(); for (int32 i = 0; i < _size; i++) - data[i]._state = Bucket::Empty; + data[i]._state = BucketState::Empty; } else { @@ -977,7 +971,7 @@ private: _allocation.Allocate(_size); Bucket* data = _allocation.Get(); for (int32 i = 0; i < _size; i++) - data[i]._state = Bucket::Empty; + data[i]._state = BucketState::Empty; Bucket* oldData = oldAllocation.Get(); FindPositionResult pos; for (int32 i = 0; i < _size; i++) @@ -990,7 +984,7 @@ private: Bucket* bucket = &_allocation.Get()[pos.FreeSlotIndex]; Memory::MoveItems(&bucket->Key, &oldBucket.Key, 1); Memory::MoveItems(&bucket->Value, &oldBucket.Value, 1); - bucket->_state = Bucket::Occupied; + bucket->_state = BucketState::Occupied; } } for (int32 i = 0; i < _size; i++) diff --git a/Source/Engine/Core/Collections/HashSet.h b/Source/Engine/Core/Collections/HashSet.h index 2e90dae4b..aa61354fd 100644 --- a/Source/Engine/Core/Collections/HashSet.h +++ b/Source/Engine/Core/Collections/HashSet.h @@ -4,6 +4,7 @@ #include "Engine/Core/Memory/Memory.h" #include "Engine/Core/Memory/Allocation.h" +#include "Engine/Core/Collections/BucketState.h" #include "Engine/Core/Collections/HashFunctions.h" #include "Engine/Core/Collections/Config.h" @@ -24,29 +25,22 @@ public: { friend HashSet; - enum State : byte - { - Empty, - Deleted, - Occupied, - }; - /// The item. T Item; private: - State _state; + BucketState _state; FORCE_INLINE void Free() { - if (_state == Occupied) + if (_state == BucketState::Occupied) Memory::DestructItem(&Item); - _state = Empty; + _state = BucketState::Empty; } FORCE_INLINE void Delete() { - _state = Deleted; + _state = BucketState::Deleted; Memory::DestructItem(&Item); } @@ -54,34 +48,34 @@ public: FORCE_INLINE void Occupy(const ItemType& item) { Memory::ConstructItems(&Item, &item, 1); - _state = Occupied; + _state = BucketState::Occupied; } template FORCE_INLINE void Occupy(ItemType&& item) { Memory::MoveItems(&Item, &item, 1); - _state = Occupied; + _state = BucketState::Occupied; } FORCE_INLINE bool IsEmpty() const { - return _state == Empty; + return _state == BucketState::Empty; } FORCE_INLINE bool IsDeleted() const { - return _state == Deleted; + return _state == BucketState::Deleted; } FORCE_INLINE bool IsOccupied() const { - return _state == Occupied; + return _state == BucketState::Occupied; } FORCE_INLINE bool IsNotOccupied() const { - return _state != Occupied; + return _state != BucketState::Occupied; } }; @@ -109,9 +103,9 @@ private: { Bucket& toBucket = toData[i]; Memory::MoveItems(&toBucket.Item, &fromBucket.Item, 1); - toBucket._state = Bucket::Occupied; + toBucket._state = BucketState::Occupied; Memory::DestructItem(&fromBucket.Item); - fromBucket._state = Bucket::Empty; + fromBucket._state = BucketState::Empty; } } from.Free(); @@ -443,7 +437,7 @@ public: _allocation.Allocate(capacity); Bucket* data = _allocation.Get(); for (int32 i = 0; i < capacity; i++) - data[i]._state = Bucket::Empty; + data[i]._state = BucketState::Empty; } _size = capacity; Bucket* oldData = oldAllocation.Get(); @@ -459,7 +453,7 @@ public: ASSERT(pos.FreeSlotIndex != -1); Bucket* bucket = &_allocation.Get()[pos.FreeSlotIndex]; Memory::MoveItems(&bucket->Item, &oldBucket.Item, 1); - bucket->_state = Bucket::Occupied; + bucket->_state = BucketState::Occupied; _elementsCount++; } } @@ -763,7 +757,7 @@ private: // Fast path if it's empty Bucket* data = _allocation.Get(); for (int32 i = 0; i < _size; ++i) - data[i]._state = Bucket::Empty; + data[i]._state = BucketState::Empty; } else { @@ -773,7 +767,7 @@ private: _allocation.Allocate(_size); Bucket* data = _allocation.Get(); for (int32 i = 0; i < _size; ++i) - data[i]._state = Bucket::Empty; + data[i]._state = BucketState::Empty; Bucket* oldData = oldAllocation.Get(); FindPositionResult pos; for (int32 i = 0; i < _size; ++i) @@ -785,7 +779,7 @@ private: ASSERT(pos.FreeSlotIndex != -1); Bucket* bucket = &_allocation.Get()[pos.FreeSlotIndex]; Memory::MoveItems(&bucket->Item, &oldBucket.Item, 1); - bucket->_state = Bucket::Occupied; + bucket->_state = BucketState::Occupied; } } for (int32 i = 0; i < _size; ++i)