Work on the HashMap. Finding is 3 times slower than STL. Need to optimize

include/anki/util/HashMap.h

@@ -15,18 +15,16 @@ namespace anki {
 /// @{
 
 /// HashMap node. It's not a traditional bucket because it doesn't contain more
-/// than one node.
-template<typename T>
+/// than one values.
+template<typename TKey, typename TValue>
 class HashMapNode
 {
 public:
-	using Value = T;
-
-	Value m_value;
 	U64 m_hash;
-	HashMapNode* m_parent = nullptr;
 	HashMapNode* m_left = nullptr;
 	HashMapNode* m_right = nullptr;
+	TValue m_value;
+	HashMapNode* m_parent = nullptr;
 
 	template<typename... TArgs>
 	HashMapNode(TArgs&&... args)
@@ -153,12 +151,13 @@ public:
 /// @{
 
 /// Hash map template.
-template<typename T, typename THasher, typename TCompare,
-	typename TNode = HashMapNode<T>>
+template<typename TKey, typename TValue, typename THasher, typename TCompare,
+	typename TNode = HashMapNode<TKey, TValue>>
 class HashMap: public NonCopyable
 {
 public:
-	using Value = T;
+	using Key = TKey;
+	using Value = TValue;
 	using Node = TNode;
 	using Reference = Value&;
 	using ConstReference = const Value&;
@@ -254,10 +253,10 @@ public:
 
 	/// Copy an element in the map.
 	template<typename TAllocator>
-	void pushBack(TAllocator alloc, ConstReference x)
+	void pushBack(TAllocator alloc, const Key& key, ConstReference x)
 	{
 		Node* node = alloc.template newInstance<Node>(x);
-		node->m_hash = THasher()(node->m_value);
+		node->m_hash = THasher()(key);
 		pushBackNode(node);
 	}
 
@@ -266,17 +265,16 @@ public:
 	void pushBack(Pointer x)
 	{
 		ANKI_ASSERT(x);
-		//static_assert(typeid(x) == );
-		pushBackNode(x);
+		// TODO
 	}
 
 	/// Construct an element inside the map.
 	template<typename TAllocator, typename... TArgs>
-	void emplaceBack(TAllocator alloc, TArgs&&... args)
+	void emplaceBack(TAllocator alloc, const Key& key, TArgs&&... args)
 	{
 		Node* node = alloc.template newInstance<Node>(
 			std::forward<TArgs>(args)...);
-		node->m_hash = THasher()(node->m_value);
+		node->m_hash = THasher()(key);
 		pushBackNode(node);
 	}
 
@@ -285,7 +283,7 @@ public:
 	void erase(TAllocator alloc, Iterator it);
 
 	/// Find item.
-	Iterator find(const Value& a);
+	Iterator find(const Key& key);
 
 private:
 	Node* m_root = nullptr;
@@ -299,8 +297,6 @@ private:
 
 	void pushBackNode(Node* node);
 
-	Node* findInternal(Node* node, const Value& a, U64 aHash);
-
 	template<typename TAllocator>
 	void destroyInternal(TAllocator alloc, Node* node);
 };

include/anki/util/HashMap.inl.h

@@ -6,9 +6,10 @@
 namespace anki {
 
 //==============================================================================
-template<typename T, typename THasher, typename TCompare, typename TNode>
+template<typename TKey, typename TValue, typename THasher, typename TCompare,
+	typename TNode>
 template<typename TAllocator>
-void HashMap<T, THasher, TCompare, TNode>::destroy(TAllocator alloc)
+void HashMap<TKey, TValue, THasher, TCompare, TNode>::destroy(TAllocator alloc)
 {
 	if(m_root)
 	{
@@ -18,10 +19,11 @@ void HashMap<T, THasher, TCompare, TNode>::destroy(TAllocator alloc)
 }
 
 //==============================================================================
-template<typename T, typename THasher, typename TCompare, typename TNode>
+template<typename TKey, typename TValue, typename THasher, typename TCompare,
+	typename TNode>
 template<typename TAllocator>
-void HashMap<T, THasher, TCompare, TNode>::destroyInternal(
-	TAllocator alloc, Node* node)
+void HashMap<TKey, TValue, THasher, TCompare, TNode>
+	::destroyInternal(TAllocator alloc, Node* node)
 {
 	ANKI_ASSERT(node);
 
@@ -39,8 +41,9 @@ void HashMap<T, THasher, TCompare, TNode>::destroyInternal(
 }
 
 //==============================================================================
-template<typename T, typename THasher, typename TCompare, typename TNode>
-void HashMap<T, THasher, TCompare, TNode>::pushBackNode(Node* node)
+template<typename TKey, typename TValue, typename THasher, typename TCompare,
+	typename TNode>
+void HashMap<TKey, TValue, THasher, TCompare, TNode>::pushBackNode(Node* node)
 {
 	if(ANKI_UNLIKELY(!m_root))
 	{
@@ -50,90 +53,82 @@ void HashMap<T, THasher, TCompare, TNode>::pushBackNode(Node* node)
 
 	const U64 hash = node->m_hash;
 	Node* it = m_root;
-	while(1)
+	Bool done = false;
+	do
 	{
 		const U64 nhash = it->m_hash;
 		if(hash > nhash)
 		{
 			// Go to right
-			if(it->m_right)
+			Node* right = it->m_right;
+			if(right)
 			{
-				it = it->m_right;
+				it = right;
 			}
 			else
 			{
-				it->m_right = node;
 				node->m_parent = it;
-				break;
+				it->m_right = node;
+				done = true;
 			}
 		}
 		else
 		{
 			ANKI_ASSERT(hash != nhash && "Not supported");
 			// Go to left
-			if(it->m_left)
+			Node* left = it->m_left;
+			if(left)
 			{
-				it = it->m_left;
+				it = left;
 			}
 			else
 			{
-				it->m_left = node;
 				node->m_parent = it;
-				break;
+				it->m_left = node;
+				done = true;
 			}
 		}
-	}
+	} while(!done);
 }
 
 //==============================================================================
-template<typename T, typename THasher, typename TCompare, typename TNode>
-typename HashMap<T, THasher, TCompare, TNode>::Iterator
-	HashMap<T, THasher, TCompare, TNode>::find(const Value& a)
+template<typename TKey, typename TValue, typename THasher, typename TCompare,
+	typename TNode>
+typename HashMap<TKey, TValue, THasher, TCompare, TNode>::Iterator
+	HashMap<TKey, TValue, THasher, TCompare, TNode>
+	::find(const Key& key)
 {
-	if(ANKI_UNLIKELY(!m_root))
-	{
-		Iterator(nullptr);
-	}
-
-	U64 hash = THasher()(a);
-	return Iterator(findInternal(m_root, a, hash));
-}
+	const U64 hash = THasher()(key);
 
-//==============================================================================
-template<typename T, typename THasher, typename TCompare, typename TNode>
-TNode* HashMap<T, THasher, TCompare, TNode>::findInternal(Node* node,
-	const Value& a, U64 aHash)
-{
-	ANKI_ASSERT(node);
-	if(node->m_hash == aHash)
+	Node* node = m_root;
+	while(node)
 	{
-		// Found
-		ANKI_ASSERT(TCompare()(node->m_value, a)
-			&& "Doesn't accept collisions");
-		return node;
-	}
+		const U64 bhash = node->m_hash;
 
-	if(aHash < node->m_hash)
-	{
-		node = node->m_left;
-	}
-	else
-	{
-		node = node->m_right;
-	}
-
-	if(node)
-	{
-		return findInternal(node, a, aHash);
+		if(hash < bhash)
+		{
+			node = node->m_left;
+		}
+		else if(hash > bhash)
+		{
+			node = node->m_right;
+		}
+		else
+		{
+			// Found
+			break;
+		}
 	}
 
-	return node;
+	return Iterator(node);
 }
 
 //==============================================================================
-template<typename T, typename THasher, typename TCompare, typename TNode>
+template<typename TKey, typename TValue, typename THasher, typename TCompare,
+	typename TNode>
 template<typename TAllocator>
-void HashMap<T, THasher, TCompare, TNode>::erase(TAllocator alloc, Iterator it)
+void HashMap<TKey, TValue, THasher, TCompare, TNode>::erase(TAllocator alloc,
+	Iterator it)
 {
 	Node* del = it.m_node;
 	ANKI_ASSERT(del);

src/util/HighRezTimer.cpp

@@ -11,8 +11,6 @@ namespace anki {
 //==============================================================================
 void HighRezTimer::start()
 {
-	ANKI_ASSERT(m_startTime == 0);
-	ANKI_ASSERT(m_stopTime == 0);
 	m_startTime = getCurrentTime();
 	m_stopTime = 0.0;
 }

tests/util/HashMap.cpp

@@ -6,6 +6,7 @@
 #include "tests/framework/Framework.h"
 #include "tests/util/Foo.h"
 #include "anki/util/HashMap.h"
+#include "anki/util/DArray.h"
 #include "anki/util/HighRezTimer.h"
 #include <unordered_map>
 
@@ -32,23 +33,24 @@ public:
 ANKI_TEST(Util, HashMap)
 {
 	HeapAllocator<U8> alloc(allocAligned, nullptr);
-	int arr[] = {20, 15, 5, 1, 10, 0, 18, 6, 7, 11, 13, 3};
-	U arrSize = sizeof(arr) / sizeof(arr[0]);
+	int vals[] = {20, 15, 5, 1, 10, 0, 18, 6, 7, 11, 13, 3};
+	U valsSize = sizeof(vals) / sizeof(vals[0]);
 
 	// Simple
 	{
-		HashMap<int, Hasher, Compare> map;
-		map.pushBack(alloc, 20);
+		HashMap<int, int, Hasher, Compare> map;
+		map.pushBack(alloc, 20, 1);
+		map.pushBack(alloc, 21, 1);
 		map.destroy(alloc);
 	}
 
 	// Add more and iterate
 	{
-		HashMap<int, Hasher, Compare> map;
+		HashMap<int, int, Hasher, Compare> map;
 
-		for(U i = 0; i < arrSize; ++i)
+		for(U i = 0; i < valsSize; ++i)
 		{
-			map.pushBack(alloc, arr[i]);
+			map.pushBack(alloc, vals[i], vals[i] * 10);
 		}
 
 		U count = 0;
@@ -57,29 +59,124 @@ ANKI_TEST(Util, HashMap)
 		{
 			++count;
 		}
-		ANKI_TEST_EXPECT_EQ(count, arrSize);
+		ANKI_TEST_EXPECT_EQ(count, valsSize);
 
 		map.destroy(alloc);
 	}
 
 	// Erase
 	{
-		HashMap<int, Hasher, Compare> map;
+		HashMap<int, int, Hasher, Compare> map;
 
-		for(U i = 0; i < arrSize; ++i)
+		for(U i = 0; i < valsSize; ++i)
 		{
-			map.pushBack(alloc, arr[i]);
+			map.pushBack(alloc, vals[i], vals[i] * 10);
 		}
 
-		for(U i = arrSize - 1; i != 0; --i)
+		for(U i = valsSize - 1; i != 0; --i)
 		{
-			HashMap<int, Hasher, Compare>::Iterator it = map.find(arr[i]);
+			HashMap<int, int, Hasher, Compare>::Iterator it = map.find(vals[i]);
 			ANKI_TEST_EXPECT_NEQ(it, map.getEnd());
 
 			map.erase(alloc, it);
-			map.pushBack(alloc, arr[i]);
+			map.pushBack(alloc, vals[i], vals[i] * 10);
 		}
 
 		map.destroy(alloc);
 	}
+
+	// Find
+	{
+		HashMap<int, int, Hasher, Compare> map;
+
+		for(U i = 0; i < valsSize; ++i)
+		{
+			map.pushBack(alloc, vals[i], vals[i] * 10);
+		}
+
+		for(U i = valsSize - 1; i != 0; --i)
+		{
+			HashMap<int, int, Hasher, Compare>::Iterator it = map.find(vals[i]);
+			ANKI_TEST_EXPECT_NEQ(it, map.getEnd());
+			ANKI_TEST_EXPECT_EQ(*it, vals[i] * 10);
+		}
+
+		map.destroy(alloc);
+	}
+
+	// Bench it
+	{
+		HashMap<int, int, Hasher, Compare> akMap;
+		std::unordered_map<int, int, std::hash<int>, std::equal_to<int>,
+			HeapAllocator<std::pair<int, int>>> stdMap(10, std::hash<int>(),
+			std::equal_to<int>(), alloc);
+
+		std::unordered_map<int, int> tmpMap;
+
+		HighRezTimer timer;
+		I64 count = 0;
+
+		// Create a huge set
+		const U COUNT = 1024 * 100;
+		DArrayAuto<int> vals(alloc);
+		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;
+		}
+
+		// Put the vals AnKi
+		timer.start();
+		for(U i = 0; i < COUNT; ++i)
+		{
+			akMap.pushBack(alloc, vals[i], vals[i]);
+		}
+		timer.stop();
+		HighRezTimer::Scalar akTime = timer.getElapsedTime();
+
+		// Put the vals STL
+		timer.start();
+		for(U i = 0; i < COUNT; ++i)
+		{
+			stdMap[vals[i]] = vals[i];
+		}
+		timer.stop();
+		HighRezTimer::Scalar stlTime = timer.getElapsedTime();
+
+		printf("Inserting bench: STL %f AnKi %f | %f%%\n", stlTime, akTime,
+			stlTime / akTime * 100.0);
+
+		// Find values AnKi
+		timer.start();
+		for(U i = 0; i < COUNT; ++i)
+		{
+			auto it = akMap.find(vals[i]);
+			count += *it;
+		}
+		timer.stop();
+		akTime = timer.getElapsedTime();
+
+		// Find values STL
+		timer.start();
+		for(U i = 0; i < COUNT; ++i)
+		{
+			count += stdMap[vals[i]];
+		}
+		timer.stop();
+		stlTime = timer.getElapsedTime();
+
+		printf("Find bench: STL %f AnKi %f | %f%%\n", stlTime, akTime,
+			stlTime / akTime * 100.0);
+
+		akMap.destroy(alloc);
+	}
 }