[mono] Switch simdhash to prime bucket counts; increase load factor

Add some measurements for simdhash ght
Introduce scan data that's fetched once per bucket
Targeted simdhash optimizations via new dn_simdhash_ght_get_value_or_default
Switch to spaced prime bucket counts instead of power of two bucket counts for better collision resistance
Fix cache alignment of ght compatible and ptr ptr simdhash variants on 64-bit
Higher simdhash load factor (closer to ghashtable's; improves performance for bad hashes)
Add sequential keys measurements
Increase load factor to 30%

Author: kg

src/mono/mono/metadata/bundled-resources.c

@@ -204,9 +204,7 @@ bundled_resources_get (const char *id)
 	char key_buffer[1024];
 	key_from_id(id, key_buffer, sizeof(key_buffer));
 
-	MonoBundledResource *result = NULL;
-	dn_simdhash_ght_try_get_value (bundled_resources, key_buffer, (void **)&result);
-	return result;
+	return (MonoBundledResource *)dn_simdhash_ght_get_value_or_default (bundled_resources, key_buffer);
 }
 
 //---------------------------------------------------------------------------------------

src/mono/mono/metadata/class.c

@@ -1252,7 +1252,7 @@ mono_class_inflate_generic_method_full_checked (MonoMethod *method, MonoClass *k
 	mono_mem_manager_lock (mm);
 	if (!mm->gmethod_cache)
 		mm->gmethod_cache = dn_simdhash_ght_new_full (inflated_method_hash, inflated_method_equal, NULL, (GDestroyNotify)free_inflated_method, 0, NULL);
-	dn_simdhash_ght_try_get_value (mm->gmethod_cache, iresult, (void **)&cached);
+	cached = dn_simdhash_ght_get_value_or_default (mm->gmethod_cache, iresult);
 	mono_mem_manager_unlock (mm);
 
 	if (cached) {
@@ -1358,8 +1358,7 @@ mono_class_inflate_generic_method_full_checked (MonoMethod *method, MonoClass *k
 
 	// check cache
 	mono_mem_manager_lock (mm);
-	cached = NULL;
-	dn_simdhash_ght_try_get_value (mm->gmethod_cache, iresult, (void **)&cached);
+	cached = dn_simdhash_ght_get_value_or_default (mm->gmethod_cache, iresult);
 	if (!cached) {
 		dn_simdhash_ght_insert (mm->gmethod_cache, iresult, iresult);
 		iresult->owner = mm;

src/mono/mono/metadata/metadata.c

@@ -3359,7 +3359,7 @@ mono_metadata_get_inflated_signature (MonoMethodSignature *sig, MonoGenericConte
 		mm->gsignature_cache = dn_simdhash_ght_new_full (inflated_signature_hash, inflated_signature_equal, NULL, (GDestroyNotify)free_inflated_signature, 256, NULL);
 
 	// FIXME: The lookup is done on the newly allocated sig so it always fails
-	dn_simdhash_ght_try_get_value (mm->gsignature_cache, &helper, (gpointer *)&res);
+	res = dn_simdhash_ght_get_value_or_default (mm->gsignature_cache, &helper);
 	if (!res) {
 		res = mono_mem_manager_alloc0 (mm, sizeof (MonoInflatedMethodSignature));
 		// FIXME: sig is an inflated signature not owned by the mem manager

src/mono/mono/mini/interp/transform.c

@@ -2736,7 +2736,7 @@ interp_handle_intrinsics (TransformData *td, MonoMethod *target_method, MonoClas
 				default: return FALSE;
 			}
 		}
-	} 
+	}
 
 	return FALSE;
 }
@@ -10036,8 +10036,7 @@ mono_interp_transform_method (InterpMethod *imethod, ThreadContext *context, Mon
 
 		// FIXME Publishing of seq points seems to be racy with tiereing. We can have both tiered and untiered method
 		// running at the same time. We could therefore get the optimized imethod seq points for the unoptimized method.
-		gpointer seq_points = NULL;
-		dn_simdhash_ght_try_get_value (jit_mm->seq_points, imethod->method, (void **)&seq_points);
+		gpointer seq_points = dn_simdhash_ght_get_value_or_default (jit_mm->seq_points, imethod->method);
 		if (!seq_points || seq_points != imethod->jinfo->seq_points)
 			dn_simdhash_ght_replace (jit_mm->seq_points, imethod->method, imethod->jinfo->seq_points);
 	}

src/mono/mono/mini/seq-points.c

@@ -272,12 +272,12 @@ mono_get_seq_points (MonoMethod *method)
 	// FIXME:
 	jit_mm = get_default_jit_mm ();
 	jit_mm_lock (jit_mm);
-	dn_simdhash_ght_try_get_value (jit_mm->seq_points, method, (void **)&seq_points);
+	seq_points = dn_simdhash_ght_get_value_or_default (jit_mm->seq_points, method);
 	if (!seq_points && method->is_inflated) {
 		/* generic sharing + aot */
-		dn_simdhash_ght_try_get_value (jit_mm->seq_points, declaring_generic_method, (void **)&seq_points);
+		seq_points = dn_simdhash_ght_get_value_or_default (jit_mm->seq_points, declaring_generic_method);
 		if (!seq_points)
-			dn_simdhash_ght_try_get_value (jit_mm->seq_points, shared_method, (void **)&seq_points);
+			seq_points = dn_simdhash_ght_get_value_or_default (jit_mm->seq_points, shared_method);
 	}
 	jit_mm_unlock (jit_mm);
 

src/native/containers/dn-simdhash-ght-compatible.c

@@ -16,27 +16,6 @@ typedef struct dn_simdhash_ght_data {
 	dn_simdhash_ght_destroy_func value_destroy_func;
 } dn_simdhash_ght_data;
 
-static inline uint32_t
-dn_simdhash_ght_hash (dn_simdhash_ght_data data, void * key)
-{
-	dn_simdhash_ght_hash_func hash_func = data.hash_func;
-	if (hash_func)
-		return (uint32_t)hash_func(key);
-	else
-		// FIXME: Seed
-		return MurmurHash3_32_ptr(key, 0);
-}
-
-static inline int32_t
-dn_simdhash_ght_equals (dn_simdhash_ght_data data, void * lhs, void * rhs)
-{
-	dn_simdhash_ght_equal_func equal_func = data.key_equal_func;
-	if (equal_func)
-		return equal_func(lhs, rhs);
-	else
-		return lhs == rhs;
-}
-
 static inline void
 dn_simdhash_ght_removed (dn_simdhash_ght_data data, void * key, void * value)
 {
@@ -68,26 +47,53 @@ dn_simdhash_ght_replaced (dn_simdhash_ght_data data, void * old_key, void * new_
 #define DN_SIMDHASH_KEY_T void *
 #define DN_SIMDHASH_VALUE_T void *
 #define DN_SIMDHASH_INSTANCE_DATA_T dn_simdhash_ght_data
-#define DN_SIMDHASH_KEY_HASHER dn_simdhash_ght_hash
-#define DN_SIMDHASH_KEY_EQUALS dn_simdhash_ght_equals
+
+#define DN_SIMDHASH_SCAN_DATA_T dn_simdhash_ght_equal_func
+#define DN_SIMDHASH_GET_SCAN_DATA(data) data.key_equal_func
+
+#define DN_SIMDHASH_KEY_HASHER(data, key) (uint32_t)data.hash_func(key)
+#define DN_SIMDHASH_KEY_EQUALS(scan_data, lhs, rhs) scan_data(lhs, rhs)
+
 #define DN_SIMDHASH_ON_REMOVE dn_simdhash_ght_removed
 #define DN_SIMDHASH_ON_REPLACE dn_simdhash_ght_replaced
+// perfect cache alignment. 128-byte buckets for 64-bit pointers, 64-byte buckets for 32-bit pointers
 #if SIZEOF_VOID_P == 8
-#define DN_SIMDHASH_BUCKET_CAPACITY 11
+#define DN_SIMDHASH_BUCKET_CAPACITY 14
 #else
 #define DN_SIMDHASH_BUCKET_CAPACITY 12
 #endif
 #define DN_SIMDHASH_NO_DEFAULT_NEW 1
 
 #include "dn-simdhash-specialization.h"
 
+static unsigned int
+dn_simdhash_ght_default_hash (const void * key)
+{
+    // You might think we should avalanche the key bits but in my testing, it doesn't help.
+    // Right now the default hash function is rarely used anyway
+    return (unsigned int)(size_t)key;
+}
+
+static int32_t
+dn_simdhash_ght_default_comparer (const void * a, const void * b)
+{
+	return a == b;
+}
+
 dn_simdhash_ght_t *
 dn_simdhash_ght_new (
 	dn_simdhash_ght_hash_func hash_func, dn_simdhash_ght_equal_func key_equal_func,
 	uint32_t capacity, dn_allocator_t *allocator
 )
 {
 	dn_simdhash_ght_t *hash = dn_simdhash_new_internal(&DN_SIMDHASH_T_META, DN_SIMDHASH_T_VTABLE, capacity, allocator);
+	// Most users of dn_simdhash_ght are passing a custom comparer, and always doing an indirect call ends up being faster
+	//  than conditionally doing a fast inline check when there's no comparer set. Somewhat counter-intuitive, but true
+	//  on both x64 and arm64. Probably due to the smaller code size and reduced branch predictor pressure.
+	if (!hash_func)
+		hash_func = dn_simdhash_ght_default_hash;
+	if (!key_equal_func)
+		key_equal_func = dn_simdhash_ght_default_comparer;
 	dn_simdhash_instance_data(dn_simdhash_ght_data, hash).hash_func = hash_func;
 	dn_simdhash_instance_data(dn_simdhash_ght_data, hash).key_equal_func = key_equal_func;
 	return hash;
@@ -101,6 +107,10 @@ dn_simdhash_ght_new_full (
 )
 {
 	dn_simdhash_ght_t *hash = dn_simdhash_new_internal(&DN_SIMDHASH_T_META, DN_SIMDHASH_T_VTABLE, capacity, allocator);
+	if (!hash_func)
+		hash_func = dn_simdhash_ght_default_hash;
+	if (!key_equal_func)
+		key_equal_func = dn_simdhash_ght_default_comparer;
 	dn_simdhash_instance_data(dn_simdhash_ght_data, hash).hash_func = hash_func;
 	dn_simdhash_instance_data(dn_simdhash_ght_data, hash).key_equal_func = key_equal_func;
 	dn_simdhash_instance_data(dn_simdhash_ght_data, hash).key_destroy_func = key_destroy_func;
@@ -145,3 +155,17 @@ dn_simdhash_ght_insert_replace (
 			return;
 	}
 }
+
+void *
+dn_simdhash_ght_get_value_or_default (
+	dn_simdhash_ght_t *hash, void * key
+)
+{
+	check_self(hash);
+	uint32_t key_hash = DN_SIMDHASH_KEY_HASHER(DN_SIMDHASH_GET_DATA(hash), key);
+	DN_SIMDHASH_VALUE_T *value_ptr = DN_SIMDHASH_FIND_VALUE_INTERNAL(hash, key, key_hash);
+	if (value_ptr)
+		return *value_ptr;
+	else
+		return NULL;
+}

src/native/containers/dn-simdhash-ght-compatible.h

@@ -28,6 +28,12 @@ dn_simdhash_ght_insert_replace (
 	int32_t overwrite_key
 );
 
+// faster and simpler to use than dn_simdhash_ght_try_get_value, use it wisely
+void *
+dn_simdhash_ght_get_value_or_default (
+	dn_simdhash_ght_t *hash, void * key
+);
+
 // compatibility shims for the g_hash_table_ versions in glib.h
 #define dn_simdhash_ght_insert(h,k,v)  dn_simdhash_ght_insert_replace ((h),(k),(v),FALSE)
 #define dn_simdhash_ght_replace(h,k,v) dn_simdhash_ght_insert_replace ((h),(k),(v),TRUE)

src/native/containers/dn-simdhash-ptr-ptr.c

@@ -11,10 +11,11 @@
 #define DN_SIMDHASH_T dn_simdhash_ptr_ptr
 #define DN_SIMDHASH_KEY_T void *
 #define DN_SIMDHASH_VALUE_T void *
-#define DN_SIMDHASH_KEY_HASHER(hash, key) (MurmurHash3_32_ptr(key, 0))
-#define DN_SIMDHASH_KEY_EQUALS(hash, lhs, rhs) (lhs == rhs)
+#define DN_SIMDHASH_KEY_HASHER(data, key) (MurmurHash3_32_ptr(key, 0))
+#define DN_SIMDHASH_KEY_EQUALS(data, lhs, rhs) (lhs == rhs)
+// perfect cache alignment. 128-byte buckets for 64-bit pointers, 64-byte buckets for 32-bit pointers
 #if SIZEOF_VOID_P == 8
-#define DN_SIMDHASH_BUCKET_CAPACITY 11
+#define DN_SIMDHASH_BUCKET_CAPACITY 14
 #else
 #define DN_SIMDHASH_BUCKET_CAPACITY 12
 #endif

src/native/containers/dn-simdhash-ptrpair-ptr.c

@@ -26,8 +26,8 @@ dn_ptrpair_t_equals (dn_ptrpair_t lhs, dn_ptrpair_t rhs)
 #define DN_SIMDHASH_T dn_simdhash_ptrpair_ptr
 #define DN_SIMDHASH_KEY_T dn_ptrpair_t
 #define DN_SIMDHASH_VALUE_T void *
-#define DN_SIMDHASH_KEY_HASHER(hash, key) dn_ptrpair_t_hash(key)
-#define DN_SIMDHASH_KEY_EQUALS(hash, lhs, rhs) dn_ptrpair_t_equals(lhs, rhs)
+#define DN_SIMDHASH_KEY_HASHER(data, key) dn_ptrpair_t_hash(key)
+#define DN_SIMDHASH_KEY_EQUALS(data, lhs, rhs) dn_ptrpair_t_equals(lhs, rhs)
 #if SIZEOF_VOID_P == 8
 // 192 bytes holds 12 16-byte blocks, so 11 keys and one suffix table
 #define DN_SIMDHASH_BUCKET_CAPACITY 11

src/native/containers/dn-simdhash-specialization.h

@@ -23,6 +23,13 @@
 #error Expected DN_SIMDHASH_VALUE_T definition i.e. int
 #endif
 
+// If specified, this data will be precalculated/prefetched at the start of scans
+//  and passed to your KEY_EQUALS macro as its first parameter
+#ifndef DN_SIMDHASH_SCAN_DATA_T
+#define DN_SIMDHASH_SCAN_DATA_T uint8_t
+#define DN_SIMDHASH_GET_SCAN_DATA(data) 0
+#endif
+
 // If specified, we pass instance data to the handlers by-value, otherwise we
 //  pass the pointer to the hash itself by-value. This is enough to allow clang
 //  to hoist the load of the instance data out of the key scan loop, though it
@@ -204,11 +211,13 @@ DN_SIMDHASH_SCAN_BUCKET_INTERNAL (DN_SIMDHASH_T_PTR hash, bucket_t *restrict buc
 #undef bucket_suffixes
 
 	if (DN_LIKELY(index < count)) {
+		DN_SIMDHASH_SCAN_DATA_T scan_data = DN_SIMDHASH_GET_SCAN_DATA(DN_SIMDHASH_GET_DATA(hash));
+		// HACK: Suppress unused variable warning for specializations that don't use scan_data
+		(void)(scan_data);
+
 		DN_SIMDHASH_KEY_T *key = &bucket->keys[index];
 		do {
-			// FIXME: Could be profitable to manually hoist the data load outside of the loop,
-			//  if not out of SCAN_BUCKET_INTERNAL entirely. Clang appears to do LICM on it.
-			if (DN_SIMDHASH_KEY_EQUALS(DN_SIMDHASH_GET_DATA(hash), needle, *key))
+			if (DN_SIMDHASH_KEY_EQUALS(scan_data, needle, *key))
 				return index;
 			key++;
 			index++;

src/native/containers/dn-simdhash-string-ptr.c

@@ -35,8 +35,8 @@ dn_simdhash_str_hash (dn_simdhash_str_key v1)
 #define DN_SIMDHASH_T dn_simdhash_string_ptr
 #define DN_SIMDHASH_KEY_T dn_simdhash_str_key
 #define DN_SIMDHASH_VALUE_T void *
-#define DN_SIMDHASH_KEY_HASHER(hash, key) dn_simdhash_str_hash(key)
-#define DN_SIMDHASH_KEY_EQUALS(hash, lhs, rhs) dn_simdhash_str_equal(lhs, rhs)
+#define DN_SIMDHASH_KEY_HASHER(data, key) dn_simdhash_str_hash(key)
+#define DN_SIMDHASH_KEY_EQUALS(data, lhs, rhs) dn_simdhash_str_equal(lhs, rhs)
 #define DN_SIMDHASH_ACCESSOR_SUFFIX _raw
 
 // perfect cache alignment. 32-bit ptrs: 8-byte keys. 64-bit: 16-byte keys.

src/native/containers/dn-simdhash-u32-ptr.c

@@ -7,7 +7,7 @@
 #define DN_SIMDHASH_T dn_simdhash_u32_ptr
 #define DN_SIMDHASH_KEY_T uint32_t
 #define DN_SIMDHASH_VALUE_T void *
-#define DN_SIMDHASH_KEY_HASHER(hash, key) murmur3_fmix32(key)
-#define DN_SIMDHASH_KEY_EQUALS(hash, lhs, rhs) (lhs == rhs)
+#define DN_SIMDHASH_KEY_HASHER(data, key) murmur3_fmix32(key)
+#define DN_SIMDHASH_KEY_EQUALS(data, lhs, rhs) (lhs == rhs)
 
 #include "dn-simdhash-specialization.h"

src/native/containers/dn-simdhash-utils.h

@@ -176,6 +176,35 @@ MurmurHash3_32_streaming (const uint8_t *key, uint32_t seed)
 	return h1;
 }
 
+static inline uint32_t
+MurmurHash3_32 (const uint8_t *key, uint32_t key_length, uint32_t seed)
+{
+	uint32_t h1 = seed;
+
+	while (key_length >= sizeof(uint32_t)) {
+		uint32_t block = *(uint32_t *)key;
+		MURMUR3_HASH_BLOCK(block);
+		key += sizeof(uint32_t);
+		key_length -= sizeof(uint32_t);
+	}
+
+	if (key_length) {
+		uint32_t last_block = 0;
+		// This is incorrect but easier to write
+		while (key_length > 0) {
+			last_block |= *key;
+			last_block <<= 8;
+			key++;
+		}
+		MURMUR3_HASH_BLOCK(last_block);
+	}
+
+	// finalize. note this is not the same as 32_streaming
+	h1 ^= key_length;
+	h1 = murmur3_fmix32(h1);
+	return h1;
+}
+
 // end of reformulated murmur3-32
 
 void