More SparseArray bug fixes and unit tests

src/anki/Config.h.cmake

@@ -142,6 +142,7 @@
 #	define ANKI_RESTRICT __restrict
 #	define ANKI_USE_RESULT __attribute__((warn_unused_result))
 #	define ANKI_FORCE_INLINE __attribute__((always_inline))
+#	define ANKI_DONT_INLINE __attribute__((noinline))
 #	define ANKI_UNUSED __attribute__((__unused__))
 #else
 #	define ANKI_LIKELY(x) ((x) == 1)
@@ -149,6 +150,7 @@
 #	define ANKI_RESTRICT
 #	define ANKI_USE_RESULT
 #	define ANKI_FORCE_INLINE
+#	define ANKI_DONT_INLINE
 #	define ANKI_UNUSED
 #endif
 

src/anki/util/SparseArray.h

@@ -5,11 +5,11 @@
 
 #pragma once
 
+#include <anki/util/StdTypes.h>
 #include <anki/util/Assert.h>
 #include <anki/util/Array.h>
 #include <anki/util/Allocator.h>
 #include <utility>
-#include <ctime>
 
 namespace anki
 {
@@ -185,13 +185,13 @@ public:
 	/// Get begin.
 	Iterator getBegin()
 	{
-		return Iterator(m_bucket.m_elements[m_firstElementModIdx], this);
+		return Iterator(m_bucket.m_elements[findFirstModIdx()], this);
 	}
 
 	/// Get begin.
 	ConstIterator getBegin() const
 	{
-		return ConstIterator(m_bucket.m_elements[m_firstElementModIdx], this);
+		return ConstIterator(m_bucket.m_elements[findFirstModIdx()], this);
 	}
 
 	/// Get end.
@@ -242,9 +242,11 @@ public:
 		return m_elementCount != 0;
 	}
 
+	/// Check the validity of the array.
+	void validate() const;
+
 protected:
 	SparseArrayBucket<Node, BUCKET_SIZE> m_bucket;
-	Index m_firstElementModIdx = ~Index(0); ///< To start iterating without searching. Points to m_bucket.
 	U32 m_elementCount = 0;
 
 	/// Default constructor.
@@ -303,12 +305,15 @@ protected:
 	const Node* tryGetNode(Index idx) const;
 
 	/// For iterating.
-	const Node* getNextNode(const Node* const node) const;
+	const Node* getNextNode(const Node* const node) const
+	{
+		return getNextNodeInternal(node);
+	}
 
 	/// For iterating.
 	Node* getNextNode(const Node* node)
 	{
-		const Node* out = getNextNode(node);
+		const Node* out = getNextNodeInternal(node);
 		return const_cast<Node*>(out);
 	}
 
@@ -323,6 +328,14 @@ private:
 	/// Remove a node from the tree.
 	/// @return The new root node.
 	static Node* removeFromTree(Node* root, Node* del);
+
+	/// See validate().
+	void validateInternal(Index modIdx, const Node* node, PtrSize& count) const;
+
+	/// For iterating.
+	Index findFirstModIdx() const;
+
+	const Node* getNextNodeInternal(const Node* node) const;
 };
 
 /// Sparse array.

src/anki/util/SparseArray.inl.h

@@ -12,13 +12,7 @@ template<typename T, typename TNode, typename TIndex, PtrSize TBucketSize, PtrSi
 TNode** SparseArrayBase<T, TNode, TIndex, TBucketSize, TLinearProbingSize>::findPlace(Index idx, Node*& parent)
 {
 	parent = nullptr;
-
-	// Update the first node
 	const Index modIdx = mod(idx);
-	if(modIdx < m_firstElementModIdx)
-	{
-		m_firstElementModIdx = modIdx;
-	}
 
 	if(m_bucket.m_elements[modIdx] == nullptr || m_bucket.m_elements[modIdx]->m_saIdx == idx)
 	{
@@ -70,7 +64,6 @@ TNode** SparseArrayBase<T, TNode, TIndex, TBucketSize, TLinearProbingSize>::find
 	// Last thing we can do, need to append to a tree
 	ANKI_ASSERT(m_bucket.m_elements[modIdx]);
 	Node* it = m_bucket.m_elements[modIdx];
-	Node** pit = &m_bucket.m_elements[modIdx];
 	while(true)
 	{
 		if(idx > it->m_saIdx)
@@ -80,7 +73,6 @@ TNode** SparseArrayBase<T, TNode, TIndex, TBucketSize, TLinearProbingSize>::find
 			if(right)
 			{
 				it = right;
-				pit = &it->m_saRight;
 			}
 			else
 			{
@@ -95,7 +87,6 @@ TNode** SparseArrayBase<T, TNode, TIndex, TBucketSize, TLinearProbingSize>::find
 			if(left)
 			{
 				it = left;
-				pit = &it->m_saLeft;
 			}
 			else
 			{
@@ -107,7 +98,23 @@ TNode** SparseArrayBase<T, TNode, TIndex, TBucketSize, TLinearProbingSize>::find
 		{
 			// Equal
 			parent = it->m_saParent;
-			return pit;
+
+			if(parent)
+			{
+				if(parent->m_saLeft == it)
+				{
+					return &parent->m_saLeft;
+				}
+				else
+				{
+					ANKI_ASSERT(parent->m_saRight == it);
+					return &parent->m_saRight;
+				}
+			}
+			else
+			{
+				return &m_bucket.m_elements[modIdx];
+			}
 		}
 	}
 
@@ -128,7 +135,7 @@ const TNode* SparseArrayBase<T, TNode, TIndex, TBucketSize, TLinearProbingSize>:
 		{
 			if(it->m_saIdx == idx)
 			{
-				break;
+				return it;
 			}
 			else if(idx > it->m_saIdx)
 			{
@@ -139,8 +146,6 @@ const TNode* SparseArrayBase<T, TNode, TIndex, TBucketSize, TLinearProbingSize>:
 				it = it->m_saLeft;
 			}
 		} while(it);
-
-		return it;
 	}
 
 	// Search for linear probing
@@ -284,20 +289,6 @@ void SparseArrayBase<T, TNode, TIndex, TBucketSize, TLinearProbingSize>::remove(
 
 	const Index modIdx = mod(node->m_saIdx);
 
-	// Update the first node
-	ANKI_ASSERT(m_firstElementModIdx <= modIdx);
-	if(ANKI_UNLIKELY(m_firstElementModIdx == modIdx))
-	{
-		for(Index i = 0; i < BUCKET_SIZE; ++i)
-		{
-			if(m_bucket.m_elements[i])
-			{
-				m_firstElementModIdx = i;
-				break;
-			}
-		}
-	}
-
 	if(parent || left || right)
 	{
 		// In a tree, remove
@@ -328,54 +319,149 @@ void SparseArrayBase<T, TNode, TIndex, TBucketSize, TLinearProbingSize>::remove(
 }
 
 template<typename T, typename TNode, typename TIndex, PtrSize TBucketSize, PtrSize TLinearProbingSize>
-const TNode* SparseArrayBase<T, TNode, TIndex, TBucketSize, TLinearProbingSize>::getNextNode(
-	const Node* const node) const
+const TNode* SparseArrayBase<T, TNode, TIndex, TBucketSize, TLinearProbingSize>::getNextNodeInternal(
+	const Node* node) const
 {
 	ANKI_ASSERT(node);
-	const Node* out = nullptr;
 
 	if(node->m_saLeft)
 	{
-		out = node->m_saLeft;
+		return node->m_saLeft;
 	}
-	else if(node->m_saRight)
+
+	if(node->m_saRight)
 	{
-		out = node->m_saRight;
+		return node->m_saRight;
 	}
-	else if(node->m_saParent)
-	{
-		// Node without children but with a parent, move up the tree
 
-		out = node;
-		const Node* prevNode;
+	// Node without children but with a parent, move up the tree
+	const Node* out = nullptr;
+	if(node->m_saParent)
+	{
+		const Node* prevNode = node;
+		out = node->m_saParent;
 		do
 		{
-			prevNode = out;
-			out = out->m_saParent;
-
 			if(out->m_saRight && out->m_saRight != prevNode)
 			{
-				out = out->m_saRight;
-				break;
+				return out->m_saRight;
 			}
-		} while(out->m_saParent);
+
+			prevNode = out;
+			out = out->m_saParent;
+		} while(out);
+
+		// Apparently the node is the rightmost leaf
+		node = prevNode;
+		ANKI_ASSERT(node);
+		ANKI_ASSERT(m_bucket.m_elements[mod(node->m_saIdx)] == node);
 	}
-	else
+
+	// Base of the tree, move to the next bucket element
 	{
-		// Base of the tree, move to the next bucket element
+		const Index modIdx = mod(node->m_saIdx);
+
+		// Find where the node is
+		Index i = modIdx;
+		do
+		{
+		} while(i < BUCKET_SIZE && node != m_bucket.m_elements[i++]);
+
+		ANKI_ASSERT(i <= BUCKET_SIZE);
 
-		const Index nextIdx = node->m_saIdx + 1u;
-		for(Index i = mod(nextIdx); i < BUCKET_SIZE; ++i)
+		// Now move to the next
+		for(; i < BUCKET_SIZE; ++i)
 		{
 			if(m_bucket.m_elements[i])
 			{
-				out = m_bucket.m_elements[i];
-				break;
+				return m_bucket.m_elements[i];
 			}
 		}
 	}
 
-	return out;
+	return nullptr;
+}
+
+template<typename T, typename TNode, typename TIndex, PtrSize TBucketSize, PtrSize TLinearProbingSize>
+void SparseArrayBase<T, TNode, TIndex, TBucketSize, TLinearProbingSize>::validate() const
+{
+	PtrSize count = 0;
+
+	for(Index i = 0; i < BUCKET_SIZE; ++i)
+	{
+		const Node* it = m_bucket.m_elements[i];
+		if(it)
+		{
+			ANKI_ASSERT(it->m_saParent == nullptr);
+
+			if(it->m_saLeft || it->m_saRight)
+			{
+				// It's a tree
+
+				validateInternal(i, it, count);
+			}
+			else
+			{
+				// Check if it's linear probed
+
+				const Index modIdx = mod(it->m_saIdx);
+				if(modIdx != i)
+				{
+					ANKI_ASSERT(i > modIdx);
+					ANKI_ASSERT(i - modIdx < LINEAR_PROBING_SIZE);
+				}
+
+				++count;
+			}
+		}
+	}
+
+	ANKI_ASSERT(count == m_elementCount);
+}
+
+template<typename T, typename TNode, typename TIndex, PtrSize TBucketSize, PtrSize TLinearProbingSize>
+void SparseArrayBase<T, TNode, TIndex, TBucketSize, TLinearProbingSize>::validateInternal(
+	Index modIdx, const Node* node, PtrSize& count) const
+{
+	ANKI_ASSERT(node);
+	ANKI_ASSERT(mod(node->m_saIdx) == modIdx);
+
+	++count;
+
+	const Node* parent = node->m_saParent;
+	(void)parent;
+	const Node* left = node->m_saLeft;
+	const Node* right = node->m_saRight;
+
+	if(left)
+	{
+		ANKI_ASSERT(left->m_saParent == node);
+		validateInternal(modIdx, left, count);
+	}
+
+	if(right)
+	{
+		ANKI_ASSERT(right->m_saParent == node);
+		validateInternal(modIdx, right, count);
+	}
+}
+
+template<typename T, typename TNode, typename TIndex, PtrSize TBucketSize, PtrSize TLinearProbingSize>
+TIndex SparseArrayBase<T, TNode, TIndex, TBucketSize, TLinearProbingSize>::findFirstModIdx() const
+{
+	for(Index i = 0; i < BUCKET_SIZE; i += 2)
+	{
+		if(m_bucket.m_elements[i])
+		{
+			return i;
+		}
+		else if(m_bucket.m_elements[i + 1])
+		{
+			return i + 1;
+		}
+	}
+
+	return 0;
 }
 
 template<typename T, typename TIndex, PtrSize TBucketSize, PtrSize TLinearProbingSize>

tests/util/SparseArray.cpp

@@ -3,9 +3,11 @@
 // Code licensed under the BSD License.
 // http://www.anki3d.org/LICENSE
 
-#include "tests/framework/Framework.h"
+#include <tests/framework/Framework.h>
 #include <anki/util/SparseArray.h>
+#include <anki/util/HighRezTimer.h>
 #include <unordered_map>
+#include <ctime>
 
 ANKI_TEST(Util, SparseArray)
 {
@@ -106,7 +108,7 @@ ANKI_TEST(Util, SparseArray)
 
 	// Fuzzy test #2: Do random insertions and removals
 	{
-		const U MAX = 1000;
+		const U MAX = 10000;
 		SparseArray<int, U32, 64, 4> arr;
 		using StlMap =
 			std::unordered_map<int, int, std::hash<int>, std::equal_to<int>, HeapAllocator<std::pair<int, int>>>;
@@ -114,26 +116,201 @@ ANKI_TEST(Util, SparseArray)
 
 		for(U i = 0; i < MAX; ++i)
 		{
-			Bool insert = (rand() & 1) || arr.getSize() == 0;
+			const Bool insert = (rand() & 1) || arr.getSize() == 0;
 
 			if(insert)
 			{
-				I idx = rand();
+				const I idx = rand();
+
+				if(map.find(idx) != map.end())
+				{
+					continue;
+				}
+
 				arr.setAt(alloc, idx, idx);
 				map[idx] = idx;
+
+				arr.validate();
 			}
 			else
 			{
-				auto it = std::next(std::begin(map), rand() % map.size());
+				const U idx = U(rand()) % map.size();
+				auto it = std::next(std::begin(map), idx);
 
 				auto it2 = arr.getAt(it->second);
 				ANKI_TEST_EXPECT_NEQ(it2, arr.getEnd());
 
 				map.erase(it);
 				arr.erase(alloc, it2);
+
+				arr.validate();
+			}
+
+			// Iterate and check
+			{
+				StlMap bMap = map;
+
+				auto it = arr.getBegin();
+				while(it != arr.getEnd())
+				{
+					I val = *it;
+
+					auto it2 = bMap.find(val);
+					ANKI_TEST_EXPECT_NEQ(it2, bMap.end());
+
+					bMap.erase(it2);
+
+					++it;
+				}
 			}
 		}
 
 		arr.destroy(alloc);
 	}
 }
+
+static ANKI_DONT_INLINE void* allocAlignedAk(void* userData, void* ptr, PtrSize size, PtrSize alignment)
+{
+	return allocAligned(userData, ptr, size, alignment);
+}
+
+static ANKI_DONT_INLINE void* allocAlignedStl(void* userData, void* ptr, PtrSize size, PtrSize alignment)
+{
+	return allocAligned(userData, ptr, size, alignment);
+}
+
+ANKI_TEST(Util, SparseArrayBench)
+{
+	HeapAllocator<U8> allocAk(allocAlignedAk, nullptr);
+	HeapAllocator<U8> allocStl(allocAlignedStl, nullptr);
+	HeapAllocator<U8> allocTml(allocAligned, nullptr);
+
+	using StlMap = std::unordered_map<int, int, std::hash<int>, std::equal_to<int>, HeapAllocator<std::pair<int, int>>>;
+	StlMap stdMap(10, std::hash<int>(), std::equal_to<int>(), allocStl);
+
+	using AkMap = SparseArray<int, U32, 256, 16>;
+	AkMap akMap;
+
+	HighRezTimer timer;
+
+	const U COUNT = 1024 * 1024 * 5;
+
+	// Create a huge set
+	DynamicArrayAuto<int> vals(allocTml);
+
+	{
+		std::unordered_map<int, int> tmpMap;
+		vals.create(COUNT);
+
+		for(U i = 0; i < COUNT; ++i)
+		{
+			// Put unique keys
+			int v;
+			do
+			{
+				v = rand();
+			} while(tmpMap.find(v) != tmpMap.end());
+			tmpMap[v] = 1;
+
+			vals[i] = v;
+		}
+	}
+
+	// Insertion
+	{
+		// AnkI
+		timer.start();
+		for(U i = 0; i < COUNT; ++i)
+		{
+			akMap.setAt(allocAk, vals[i], vals[i]);
+		}
+		timer.stop();
+		HighRezTimer::Scalar akTime = timer.getElapsedTime();
+
+		// STL
+		timer.start();
+		for(U i = 0; i < COUNT; ++i)
+		{
+			stdMap[vals[i]] = vals[i];
+		}
+		timer.stop();
+		HighRezTimer::Scalar stlTime = timer.getElapsedTime();
+
+		ANKI_TEST_LOGI("Inserting bench: STL %f AnKi %f | %f%%", stlTime, akTime, stlTime / akTime * 100.0);
+	}
+
+// Search
+#if 1
+	{
+		int count = 0;
+
+		// Find values AnKi
+		timer.start();
+		for(U i = 0; i < COUNT; ++i)
+		{
+			auto it = akMap.getAt(vals[i]);
+			count += *it;
+		}
+		timer.stop();
+		HighRezTimer::Scalar akTime = timer.getElapsedTime();
+
+		// Find values STL
+		timer.start();
+		for(U i = 0; i < COUNT; ++i)
+		{
+			count += stdMap[vals[i]];
+		}
+		timer.stop();
+		HighRezTimer::Scalar stlTime = timer.getElapsedTime();
+
+		ANKI_TEST_LOGI("Find bench: STL %f AnKi %f | %f%%", stlTime, akTime, stlTime / akTime * 100.0);
+	}
+#endif
+
+	// Deletes
+	{
+		const U DEL_COUNT = COUNT / 2;
+		DynamicArrayAuto<AkMap::Iterator> delValsAk(allocTml);
+		delValsAk.create(DEL_COUNT);
+
+		DynamicArrayAuto<StlMap::iterator> delValsStl(allocTml);
+		delValsStl.create(DEL_COUNT);
+
+		std::unordered_map<int, int> tmpMap;
+		for(U i = 0; i < DEL_COUNT; ++i)
+		{
+			// Put unique keys
+			int v;
+			do
+			{
+				v = vals[rand() % COUNT];
+			} while(tmpMap.find(v) != tmpMap.end());
+			tmpMap[v] = 1;
+
+			delValsAk[i] = akMap.getAt(v);
+			delValsStl[i] = stdMap.find(v);
+		}
+
+		// Random delete AnKi
+		timer.start();
+		for(U i = 0; i < DEL_COUNT; ++i)
+		{
+			akMap.erase(allocAk, delValsAk[i]);
+		}
+		timer.stop();
+		HighRezTimer::Scalar akTime = timer.getElapsedTime();
+
+		// Random delete STL
+		timer.start();
+		for(U i = 0; i < DEL_COUNT; ++i)
+		{
+			stdMap.erase(delValsStl[i]);
+		}
+		timer.stop();
+		HighRezTimer::Scalar stlTime = timer.getElapsedTime();
+
+		ANKI_TEST_LOGI("Deleting bench: STL %f AnKi %f | %f%%\n", stlTime, akTime, stlTime / akTime * 100.0);
+	}
+
+	akMap.destroy(allocAk);
+}