SparseArray: More bug fixes

src/anki/util/SparseArray.h

@@ -69,11 +69,7 @@ public:
 	SparseArrayIterator& operator++()
 	{
 		check();
-		++m_elementIdx;
-		if(m_elementIdx >= m_array->m_capacity)
-		{
-			m_elementIdx = MAX_U32;
-		}
+		m_elementIdx = m_array->iterate(m_elementIdx, 1);
 		return *this;
 	}
 
@@ -85,25 +81,17 @@ public:
 		return out;
 	}
 
-	SparseArrayIterator operator+(U n) const
+	SparseArrayIterator operator+(U32 n) const
 	{
 		check();
-		U32 pos = m_elementIdx + n;
-		if(pos >= m_array->m_capacity)
-		{
-			pos = MAX_U32;
-		}
+		U32 pos = m_array->iterate(m_elementIdx, n);
 		return SparseArrayIterator(m_array, pos);
 	}
 
-	SparseArrayIterator& operator+=(U n)
+	SparseArrayIterator& operator+=(U32 n)
 	{
 		check();
-		m_elementIdx += n;
-		if(m_elementIdx >= m_array->m_capacity)
-		{
-			m_elementIdx = MAX_U32;
-		}
+		m_elementIdx = m_array->iterate(m_elementIdx, n);
 		return *this;
 	}
 
@@ -228,13 +216,13 @@ public:
 	/// Get end.
 	Iterator getEnd()
 	{
-		return Iterator();
+		return Iterator(this, MAX_U32);
 	}
 
 	/// Get end.
 	ConstIterator getEnd() const
 	{
-		return ConstIterator();
+		return ConstIterator(this, MAX_U32);
 	}
 
 	/// Get begin.
@@ -360,6 +348,7 @@ protected:
 		return mod(crntPos + m_capacity - desiredPos);
 	}
 
+	/// Find the first alive element.
 	Index findFirstAlive() const
 	{
 		if(m_elementCount == 0)
@@ -367,7 +356,7 @@ protected:
 			return MAX_U32;
 		}
 
-		for(Index i = 0; i < m_capacity; ++i)
+		for(U32 i = 0; i < m_capacity; ++i)
 		{
 			if(m_elements[i].m_alive)
 			{
@@ -382,14 +371,28 @@ protected:
 	/// Find an element and return its position inside m_elements.
 	Index findInternal(Index idx) const;
 
+	/// Reset the class.
 	void resetMembers()
 	{
 		m_elements = nullptr;
 		m_elementCount = 0;
 		m_capacity = 0;
-		m_initialStorageSize = 0;
-		m_probeCount = 0;
-		m_maxLoadFactor = 0;
+	}
+
+	/// Iterate a number of elements.
+	U32 iterate(U32 pos, U32 n) const
+	{
+		ANKI_ASSERT(pos < m_capacity);
+		ANKI_ASSERT(n > 0);
+		ANKI_ASSERT(m_elements[pos].m_alive);
+
+		while(n > 0 && ++pos < m_capacity)
+		{
+			ANKI_ASSERT(m_elements[pos].m_alive == 1 || m_elements[pos].m_alive == 0);
+			n -= U32(m_elements[pos].m_alive);
+		}
+
+		return (pos >= m_capacity) ? MAX_U32 : pos;
 	}
 };
 /// @}

src/anki/util/SparseArray.inl.h

@@ -45,56 +45,53 @@ template<typename T, typename TIndex>
 template<typename TAlloc>
 U32 SparseArray<T, TIndex>::insert(TAlloc& alloc, Index idx, Value& val)
 {
+start:
 	Index desiredPos = mod(idx);
 	Index endPos = mod(desiredPos + m_probeCount);
+	Index pos = desiredPos;
 
-	while(true)
+	while(pos != endPos)
 	{
-		Index pos = desiredPos;
+		Element& el = m_elements[pos];
 
-		while(pos != endPos)
+		if(!el.m_alive)
 		{
-			Element& el = m_elements[pos];
-
-			if(!el.m_alive)
-			{
-				// Empty slot was found, construct in-place
-
-				el.m_alive = true;
-				el.m_idx = idx;
-				::new(&el.m_value) Value(std::move(val));
+			// Empty slot was found, construct in-place
 
-				return 1;
-			}
-			else if(el.m_idx == idx)
-			{
-				// Same index was found, replace
+			el.m_alive = true;
+			el.m_idx = idx;
+			::new(&el.m_value) Value(std::move(val));
 
-				el.m_idx = idx;
-				el.m_value.~Value();
-				::new(&el.m_value) Value(std::move(val));
+			return 1;
+		}
+		else if(el.m_idx == idx)
+		{
+			// Same index was found, replace
 
-				return 0;
-			}
+			el.m_idx = idx;
+			el.m_value.~Value();
+			::new(&el.m_value) Value(std::move(val));
 
-			// Do the robin-hood
-			const Index otherDesiredPos = mod(el.m_idx);
-			if(distanceFromDesired(pos, otherDesiredPos) < distanceFromDesired(pos, desiredPos))
-			{
-				// Swap
-				std::swap(val, el.m_value);
-				std::swap(idx, el.m_idx);
-				desiredPos = otherDesiredPos;
-				endPos = mod(desiredPos + m_probeCount);
-			}
+			return 0;
+		}
 
-			pos = mod(pos + 1u);
+		// Do the robin-hood
+		const Index otherDesiredPos = mod(el.m_idx);
+		if(distanceFromDesired(pos, otherDesiredPos) < distanceFromDesired(pos, desiredPos))
+		{
+			// Swap
+			std::swap(val, el.m_value);
+			std::swap(idx, el.m_idx);
+			goto start;
 		}
 
-		// Didn't found an empty place, need to grow and try again
-		grow(alloc);
+		pos = mod(pos + 1u);
 	}
 
+	// Didn't found an empty place, need to grow and try again
+	grow(alloc);
+	goto start;
+
 	ANKI_ASSERT(0);
 	return 0;
 }
@@ -133,6 +130,7 @@ void SparseArray<T, TIndex>::grow(TAlloc& alloc)
 	Element* oldElements = m_elements;
 	const U32 oldCapacity = m_capacity;
 	const U32 oldElementCount = m_elementCount;
+	(void)oldElementCount;
 
 	m_capacity *= 2;
 	m_elements = static_cast<Element*>(alloc.getMemoryPool().allocate(m_capacity * sizeof(Element), alignof(Element)));
@@ -140,7 +138,7 @@ void SparseArray<T, TIndex>::grow(TAlloc& alloc)
 	m_elementCount = 0;
 
 	// Start re-inserting
-	U count = oldElementCount;
+	U count = oldCapacity;
 	Index pos = startPos;
 	while(count--)
 	{
@@ -154,6 +152,11 @@ void SparseArray<T, TIndex>::grow(TAlloc& alloc)
 
 		pos = mod(pos + 1, oldCapacity);
 	}
+
+	ANKI_ASSERT(oldElementCount == m_elementCount);
+
+	// Finalize
+	alloc.getMemoryPool().free(oldElements);
 }
 
 template<typename T, typename TIndex>
@@ -217,6 +220,7 @@ void SparseArray<T, TIndex>::validate() const
 {
 	if(m_capacity == 0)
 	{
+		ANKI_ASSERT(m_elementCount == 0 && m_elements == 0);
 		return;
 	}
 
@@ -239,24 +243,29 @@ void SparseArray<T, TIndex>::validate() const
 
 	// Start iterating
 	U elementCount = 0;
-	const Index endPos = mod(startPos + m_capacity - 1);
+	U count = m_capacity;
 	Index pos = startPos;
-	while(pos != endPos)
+	Index prevPos = ~Index(0);
+	while(count--)
 	{
 		if(m_elements[pos].m_alive)
 		{
-			if(elementCount > 0)
-			{
-				// There is a prev, check the distances
-				const Index prevPos = mod(pos + m_capacity - 1);
-
-				ANKI_ASSERT(distanceFromDesired(pos, mod(m_elements[prevPos].m_idx))
-					<= distanceFromDesired(pos, mod(m_elements[pos].m_idx)));
+			const Index myDesiredPos = mod(m_elements[pos].m_idx);
+			const Index myDistanceFromDesired = distanceFromDesired(pos, myDesiredPos);
+			ANKI_ASSERT(myDistanceFromDesired < m_probeCount);
 
-				ANKI_ASSERT(distanceFromDesired(pos, mod(m_elements[pos].m_idx)) < m_probeCount);
+			if(prevPos != ~Index(0))
+			{
+				Index prevDesiredPos = mod(m_elements[prevPos].m_idx);
+				ANKI_ASSERT(myDesiredPos >= prevDesiredPos);
 			}
 
 			++elementCount;
+			prevPos = pos;
+		}
+		else
+		{
+			prevPos = ~Index(0);
 		}
 
 		pos = mod(pos + 1);
@@ -268,7 +277,11 @@ void SparseArray<T, TIndex>::validate() const
 template<typename T, typename TIndex>
 TIndex SparseArray<T, TIndex>::findInternal(Index idx) const
 {
-	ANKI_ASSERT(m_elementCount > 0);
+	if(ANKI_UNLIKELY(m_elementCount == 0))
+	{
+		return MAX_U32;
+	}
+
 	const Index desiredPos = mod(idx);
 	const Index endPos = mod(desiredPos + m_probeCount);
 	Index pos = desiredPos;

tests/util/SparseArray.cpp

@@ -39,23 +39,21 @@ ANKI_TEST(Util, SparseArray)
 		arr.destroy(alloc);
 	}
 
-#if 0
 	// Do complex insertions
 	{
-		SparseArray<PtrSize, U32, 32, 3> arr;
+		SparseArray<PtrSize, U32> arr(64, 3);
 
-		arr.setAt(alloc, 32, 1);
-		// Linear probing
-		arr.setAt(alloc, 32 * 2, 2);
-		arr.setAt(alloc, 32 * 3, 3);
-		// Append to a tree
-		arr.setAt(alloc, 32 * 4, 4);
-		// Linear probing
-		arr.setAt(alloc, 32 + 1, 5);
-		// Evict node
-		arr.setAt(alloc, 32 * 2 + 1, 5);
+		arr.emplace(alloc, 64 * 0 - 1, 1);
+		// Linear probing to 0
+		arr.emplace(alloc, 64 * 1 - 1, 2);
+		// Linear probing to 1
+		arr.emplace(alloc, 64 * 2 - 1, 3);
+		// Linear probing to 2
+		arr.emplace(alloc, 1, 3);
+		// Swap
+		arr.emplace(alloc, 64 * 1, 3);
 
-		ANKI_TEST_EXPECT_EQ(arr.getSize(), 6);
+		ANKI_TEST_EXPECT_EQ(arr.getSize(), 5);
 
 		arr.destroy(alloc);
 	}
@@ -63,7 +61,7 @@ ANKI_TEST(Util, SparseArray)
 	// Fuzzy test
 	{
 		const U MAX = 1000;
-		SparseArray<int, U32, 32, 3> arr;
+		SparseArray<int, U32> arr;
 		std::vector<int> numbers;
 
 		srand(time(nullptr));
@@ -78,17 +76,21 @@ ANKI_TEST(Util, SparseArray)
 				if(std::find(numbers.begin(), numbers.end(), int(num)) == numbers.end())
 				{
 					// Not found
-					ANKI_TEST_EXPECT_EQ(arr.getAt(num), arr.getEnd());
-					arr.setAt(alloc, num, num);
+					ANKI_TEST_EXPECT_EQ(arr.find(num), arr.getEnd());
+					arr.emplace(alloc, num, num);
+					ANKI_TEST_EXPECT_EQ(arr.getSize(), i + 1);
+
 					numbers.push_back(num);
 					break;
 				}
 				else
 				{
 					// Found
-					ANKI_TEST_EXPECT_NEQ(arr.getAt(num), arr.getEnd());
+					ANKI_TEST_EXPECT_NEQ(arr.find(num), arr.getEnd());
 				}
 			}
+
+			arr.validate();
 		}
 
 		ANKI_TEST_EXPECT_EQ(arr.getSize(), MAX);
@@ -101,17 +103,19 @@ ANKI_TEST(Util, SparseArray)
 			int num = numbers[idx];
 			numbers.erase(numbers.begin() + idx);
 
-			auto it = arr.getAt(num);
+			auto it = arr.find(num);
 			ANKI_TEST_EXPECT_NEQ(it, arr.getEnd());
 			ANKI_TEST_EXPECT_EQ(*it, num);
 			arr.erase(alloc, it);
+
+			arr.validate();
 		}
 	}
 
 	// Fuzzy test #2: Do random insertions and removals
 	{
 		const U MAX = 10000;
-		SparseArray<int, U32, 64, 4> arr;
+		SparseArray<int, U32> arr;
 		using StlMap =
 			std::unordered_map<int, int, std::hash<int>, std::equal_to<int>, HeapAllocator<std::pair<int, int>>>;
 		StlMap map(10, std::hash<int>(), std::equal_to<int>(), alloc);
@@ -129,7 +133,7 @@ ANKI_TEST(Util, SparseArray)
 					continue;
 				}
 
-				arr.setAt(alloc, idx, idx);
+				arr.emplace(alloc, idx, idx);
 				map[idx] = idx;
 
 				arr.validate();
@@ -139,7 +143,7 @@ ANKI_TEST(Util, SparseArray)
 				const U idx = U(rand()) % map.size();
 				auto it = std::next(std::begin(map), idx);
 
-				auto it2 = arr.getAt(it->second);
+				auto it2 = arr.find(it->second);
 				ANKI_TEST_EXPECT_NEQ(it2, arr.getEnd());
 
 				map.erase(it);
@@ -169,10 +173,8 @@ ANKI_TEST(Util, SparseArray)
 
 		arr.destroy(alloc);
 	}
-#endif
 }
 
-#if 0
 static PtrSize akAllocSize = 0;
 static ANKI_DONT_INLINE void* allocAlignedAk(void* userData, void* ptr, PtrSize size, PtrSize alignment)
 {
@@ -204,8 +206,8 @@ ANKI_TEST(Util, SparseArrayBench)
 	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, 1024, 16>;
-	AkMap akMap;
+	using AkMap = SparseArray<int, U32>;
+	AkMap akMap(512, log2(512), 0.9f);
 
 	HighRezTimer timer;
 
@@ -238,7 +240,7 @@ ANKI_TEST(Util, SparseArrayBench)
 		timer.start();
 		for(U i = 0; i < COUNT; ++i)
 		{
-			akMap.setAt(allocAk, vals[i], vals[i]);
+			akMap.emplace(allocAk, vals[i], vals[i]);
 		}
 		timer.stop();
 		HighRezTimer::Scalar akTime = timer.getElapsedTime();
@@ -263,7 +265,7 @@ ANKI_TEST(Util, SparseArrayBench)
 		timer.start();
 		for(U i = 0; i < COUNT; ++i)
 		{
-			auto it = akMap.getAt(vals[i]);
+			auto it = akMap.find(vals[i]);
 			count += *it;
 		}
 		timer.stop();
@@ -307,7 +309,7 @@ ANKI_TEST(Util, SparseArrayBench)
 			} while(tmpMap.find(v) != tmpMap.end());
 			tmpMap[v] = 1;
 
-			delValsAk[i] = akMap.getAt(v);
+			delValsAk[i] = akMap.find(v);
 			delValsStl[i] = stdMap.find(v);
 		}
 
@@ -334,5 +336,3 @@ ANKI_TEST(Util, SparseArrayBench)
 
 	akMap.destroy(allocAk);
 }
-
-#endif