Merge pull request #106020 from Ivorforce/hashmap-duplicate-hash

Optimize `HashMap` insertion by removing duplicate computation of hash and position

core/templates/hash_map.h

@@ -82,7 +82,7 @@ private:
 	uint32_t capacity_index = 0;
 	uint32_t num_elements = 0;
 
-	_FORCE_INLINE_ uint32_t _hash(const TKey &p_key) const {
+	_FORCE_INLINE_ static uint32_t _hash(const TKey &p_key) {
 		uint32_t hash = Hasher::hash(p_key);
 
 		if (unlikely(hash == EMPTY_HASH)) {
@@ -98,14 +98,14 @@ private:
 	}
 
 	bool _lookup_pos(const TKey &p_key, uint32_t &r_pos) const {
-		if (elements == nullptr || num_elements == 0) {
+		return elements != nullptr && num_elements > 0 && _lookup_pos_unchecked(p_key, _hash(p_key), r_pos);
-			return false; // Failed lookups, no elements
 	}
 
+	/// Note: Assumes that elements != nullptr
+	bool _lookup_pos_unchecked(const TKey &p_key, uint32_t p_hash, uint32_t &r_pos) const {
 		const uint32_t capacity = hash_table_size_primes[capacity_index];
 		const uint64_t capacity_inv = hash_table_size_primes_inv[capacity_index];
-		uint32_t hash = _hash(p_key);
+		uint32_t pos = fastmod(p_hash, capacity_inv, capacity);
-		uint32_t pos = fastmod(hash, capacity_inv, capacity);
 		uint32_t distance = 0;
 
 		while (true) {
@@ -117,7 +117,7 @@ private:
 				return false;
 			}
 
-			if (hashes[pos] == hash && Comparator::compare(elements[pos]->data.key, p_key)) {
+			if (hashes[pos] == p_hash && Comparator::compare(elements[pos]->data.key, p_key)) {
 				r_pos = pos;
 				return true;
 			}
@@ -127,7 +127,7 @@ private:
 		}
 	}
 
-	void _insert_with_hash(uint32_t p_hash, HashMapElement<TKey, TValue> *p_value) {
+	void _insert_element(uint32_t p_hash, HashMapElement<TKey, TValue> *p_value) {
 		const uint32_t capacity = hash_table_size_primes[capacity_index];
 		const uint64_t capacity_inv = hash_table_size_primes_inv[capacity_index];
 		uint32_t hash = p_hash;
@@ -188,14 +188,14 @@ private:
 				continue;
 			}
 
-			_insert_with_hash(old_hashes[i], old_elements[i]);
+			_insert_element(old_hashes[i], old_elements[i]);
 		}
 
 		Memory::free_static(old_elements);
 		Memory::free_static(old_hashes);
 	}
 
-	_FORCE_INLINE_ HashMapElement<TKey, TValue> *_insert(const TKey &p_key, const TValue &p_value, bool p_front_insert = false) {
+	_FORCE_INLINE_ HashMapElement<TKey, TValue> *_insert(const TKey &p_key, const TValue &p_value, uint32_t p_hash, bool p_front_insert = false) {
 		uint32_t capacity = hash_table_size_primes[capacity_index];
 		if (unlikely(elements == nullptr)) {
 			// Allocate on demand to save memory.
@@ -209,13 +209,6 @@ private:
 			}
 		}
 
-		uint32_t pos = 0;
-		bool exists = _lookup_pos(p_key, pos);
-
-		if (exists) {
-			elements[pos]->data.value = p_value;
-			return elements[pos];
-		} else {
 		if (num_elements + 1 > MAX_OCCUPANCY * capacity) {
 			ERR_FAIL_COND_V_MSG(capacity_index + 1 == HASH_TABLE_SIZE_MAX, nullptr, "Hash table maximum capacity reached, aborting insertion.");
 			_resize_and_rehash(capacity_index + 1);
@@ -236,11 +229,9 @@ private:
 			tail_element = elem;
 		}
 
-			uint32_t hash = _hash(p_key);
+		_insert_element(p_hash, elem);
-			_insert_with_hash(hash, elem);
 		return elem;
 	}
-	}
 
 public:
 	_FORCE_INLINE_ uint32_t get_capacity() const { return hash_table_size_primes[capacity_index]; }
@@ -398,11 +389,13 @@ public:
 	// Replace the key of an entry in-place, without invalidating iterators or changing the entries position during iteration.
 	// p_old_key must exist in the map and p_new_key must not, unless it is equal to p_old_key.
 	bool replace_key(const TKey &p_old_key, const TKey &p_new_key) {
+		ERR_FAIL_COND_V(elements == nullptr || num_elements == 0, false);
 		if (p_old_key == p_new_key) {
 			return true;
 		}
+		const uint32_t new_hash = _hash(p_new_key);
 		uint32_t pos = 0;
-		ERR_FAIL_COND_V(_lookup_pos(p_new_key, pos), false);
+		ERR_FAIL_COND_V(_lookup_pos_unchecked(p_new_key, new_hash, pos), false);
 		ERR_FAIL_COND_V(!_lookup_pos(p_old_key, pos), false);
 		HashMapElement<TKey, TValue> *element = elements[pos];
 
@@ -418,13 +411,12 @@ public:
 		}
 		hashes[pos] = EMPTY_HASH;
 		elements[pos] = nullptr;
-		// _insert_with_hash will increment this again.
+		// _insert_element will increment this again.
 		num_elements--;
 
 		// Update the HashMapElement with the new key and reinsert it.
 		const_cast<TKey &>(element->data.key) = p_new_key;
-		uint32_t hash = _hash(p_new_key);
+		_insert_element(new_hash, element);
-		_insert_with_hash(hash, element);
 
 		return true;
 	}
@@ -583,10 +575,11 @@ public:
 	}
 
 	TValue &operator[](const TKey &p_key) {
+		const uint32_t hash = _hash(p_key);
 		uint32_t pos = 0;
-		bool exists = _lookup_pos(p_key, pos);
+		bool exists = elements && num_elements > 0 && _lookup_pos_unchecked(p_key, hash, pos);
 		if (!exists) {
-			return _insert(p_key, TValue())->data.value;
+			return _insert(p_key, TValue(), hash)->data.value;
 		} else {
 			return elements[pos]->data.value;
 		}
@@ -595,7 +588,15 @@ public:
 	/* Insert */
 
 	Iterator insert(const TKey &p_key, const TValue &p_value, bool p_front_insert = false) {
-		return Iterator(_insert(p_key, p_value, p_front_insert));
+		const uint32_t hash = _hash(p_key);
+		uint32_t pos = 0;
+		bool exists = elements && num_elements > 0 && _lookup_pos_unchecked(p_key, hash, pos);
+		if (!exists) {
+			return Iterator(_insert(p_key, p_value, hash, p_front_insert));
+		} else {
+			elements[pos]->data.value = p_value;
+			return Iterator(elements[pos]);
+		}
 	}
 
 	/* Constructors */