[mono] Migrate various GHashTables to dn_simdhash

src/mono/mono/component/debugger-engine.c

@@ -364,29 +364,42 @@ typedef struct {
 	GPtrArray *methods;
 	GPtrArray *method_domains;
 	GPtrArray *method_seq_points;
+
+	MonoDomain *domain;
 } CollectDomainData;
 
+static void
+collect_domain_bp_inner (gpointer key, gpointer value, gpointer user_data)
+{
+	MonoMethod *m = (MonoMethod *)key;
+	MonoSeqPointInfo *seq_points = (MonoSeqPointInfo *)value;
+	CollectDomainData *ud = (CollectDomainData*)user_data;
+
+	if (!bp_matches_method (ud->bp, m))
+		return;
+
+	/* Save the info locally to simplify the code inside the domain lock */
+	g_ptr_array_add (ud->methods, m);
+	g_ptr_array_add (ud->method_domains, ud->domain);
+	g_ptr_array_add (ud->method_seq_points, seq_points);
+}
+
+// HACK: Address some sort of linker problem on arm64
+void
+mono_jit_memory_manager_foreach_seq_point (dn_simdhash_ght_t *seq_points, dn_simdhash_ght_foreach_func func, gpointer user_data);
+
 static void
 collect_domain_bp (gpointer key, gpointer value, gpointer user_data)
 {
-	GHashTableIter iter;
-	MonoSeqPointInfo *seq_points;
-	MonoDomain *domain = (MonoDomain*)key;
 	CollectDomainData *ud = (CollectDomainData*)user_data;
-	MonoMethod *m;
+	ud->domain = (MonoDomain*)key;
 
 	// FIXME:
 	MonoJitMemoryManager *jit_mm = get_default_jit_mm ();
 	jit_mm_lock (jit_mm);
-	g_hash_table_iter_init (&iter, jit_mm->seq_points);
-	while (g_hash_table_iter_next (&iter, (void**)&m, (void**)&seq_points)) {
-		if (bp_matches_method (ud->bp, m)) {
-			/* Save the info locally to simplify the code inside the domain lock */
-			g_ptr_array_add (ud->methods, m);
-			g_ptr_array_add (ud->method_domains, domain);
-			g_ptr_array_add (ud->method_seq_points, seq_points);
-		}
-	}
+	// FIXME: This previously used an iterator instead of foreach, so introducing
+	//  an iterator API to simdhash might make it faster.
+	mono_jit_memory_manager_foreach_seq_point (jit_mm->seq_points, collect_domain_bp_inner, ud);
 	jit_mm_unlock (jit_mm);
 }
 

src/mono/mono/metadata/CMakeLists.txt

@@ -45,7 +45,9 @@ endif()
 set(imported_native_sources
     ../../../native/containers/dn-simdhash.c
     ../../../native/containers/dn-simdhash-string-ptr.c
-    ../../../native/containers/dn-simdhash-u32-ptr.c)
+    ../../../native/containers/dn-simdhash-u32-ptr.c
+    ../../../native/containers/dn-simdhash-ptr-ptr.c
+    ../../../native/containers/dn-simdhash-ght-compatible.c)
 
 set(metadata_common_sources
     appdomain.c

src/mono/mono/metadata/bundled-resources-internals.h

@@ -17,6 +17,7 @@ typedef enum {
 
 typedef void (*free_bundled_resource_func)(void *, void*);
 
+// WARNING: The layout of these structs cannot change because EmitBundleBase.cs depends on it!
 typedef struct _MonoBundledResource {
 	MonoBundledResourceType type;
 	const char *id;

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

@@ -8,8 +8,11 @@
 #include <mono/metadata/appdomain.h>
 #include <mono/metadata/bundled-resources-internals.h>
 #include <mono/metadata/webcil-loader.h>
+#include "../native/containers/dn-simdhash-specializations.h"
+#include "../native/containers/dn-simdhash-utils.h"
 
-static GHashTable *bundled_resources = NULL;
+static dn_simdhash_ght_t *bundled_resources = NULL;
+static dn_simdhash_ptr_ptr_t *bundled_resource_key_lookup_table = NULL;
 static bool bundled_resources_contains_assemblies = false;
 static bool bundled_resources_contains_satellite_assemblies = false;
 
@@ -31,8 +34,10 @@ mono_bundled_resources_free (void)
 {
 	g_assert (mono_runtime_is_shutting_down ());
 
-	g_hash_table_destroy (bundled_resources);
+	dn_simdhash_free (bundled_resources);
+	dn_simdhash_free (bundled_resource_key_lookup_table);
 	bundled_resources = NULL;
+	bundled_resource_key_lookup_table = NULL;
 
 	bundled_resources_contains_assemblies = false;
 	bundled_resources_contains_satellite_assemblies = false;
@@ -50,6 +55,12 @@ bundled_resources_value_destroy_func (void *resource)
 	MonoBundledResource *value = (MonoBundledResource *)resource;
 	if (value->free_func)
 		value->free_func (resource, value->free_data);
+
+	char *key;
+	if (dn_simdhash_ptr_ptr_try_get_value (bundled_resource_key_lookup_table, (void *)value->id, (void **)&key)) {
+		dn_simdhash_ptr_ptr_try_remove (bundled_resource_key_lookup_table, (void *)value->id);
+		g_free (key);
+	}
 }
 
 static bool
@@ -62,48 +73,58 @@ bundled_resources_is_known_assembly_extension (const char *ext)
 #endif
 }
 
-static gboolean
-bundled_resources_resource_id_equal (const char *id_one, const char *id_two)
+// strrchr calls strlen, so we need to do a search with known length instead
+// for some reason memrchr is defined in a header but the build fails when we try to use it
+static const char *
+g_memrchr (const char *s, char c, size_t n)
 {
-	const char *extension_one = strrchr (id_one, '.');
-	const char *extension_two = strrchr (id_two, '.');
-	if (extension_one && extension_two && bundled_resources_is_known_assembly_extension (extension_one) && bundled_resources_is_known_assembly_extension (extension_two)) {
-		size_t len_one = extension_one - id_one;
-		size_t len_two = extension_two - id_two;
-		return (len_one == len_two) && !strncmp (id_one, id_two, len_one);
-	}
-
-	return !strcmp (id_one, id_two);
+	while (n--)
+		if (s[n] == c)
+			return (void *)(s + n);
+	return NULL;
 }
 
-static guint
-bundled_resources_resource_id_hash (const char *id)
+// If a bundled resource has a known assembly extension, we strip the extension from its name
+// This ensures that lookups for foo.dll will work even if the assembly is in a webcil container
+static char *
+key_from_id (const char *id, char *buffer, guint buffer_len)
 {
-	const char *current = id;
-	const char *extension = NULL;
-	guint previous_hash = 0;
-	guint hash = 0;
-
-	while (*current) {
-		hash = (hash << 5) - (hash + *current);
-		if (*current == '.') {
-			extension = current;
-			previous_hash = hash;
-		}
-		current++;
+	size_t id_length = strlen (id),
+		extension_offset = -1;
+	const char *extension = g_memrchr (id, '.', id_length);
+	if (extension)
+		extension_offset = extension - id;
+	if (!buffer) {
+		buffer_len = (guint)(id_length + 1);
+		buffer = g_malloc (buffer_len);
 	}
+	buffer[0] = 0;
 
-	// alias all extensions to .dll
-	if (extension && bundled_resources_is_known_assembly_extension (extension)) {
-		hash = previous_hash;
-		hash = (hash << 5) - (hash + 'd');
-		hash = (hash << 5) - (hash + 'l');
-		hash = (hash << 5) - (hash + 'l');
-	}
+	if (extension_offset && bundled_resources_is_known_assembly_extension (extension))
+		g_strlcpy(buffer, id, MIN(buffer_len, extension_offset + 1));
+	else
+		g_strlcpy(buffer, id, MIN(buffer_len, id_length + 1));
+
+	return buffer;
+}
+
+static gboolean
+bundled_resources_resource_id_equal (const char *key_one, const char *key_two)
+{
+	return strcmp (key_one, key_two) == 0;
+}
 
-	return hash;
+static guint32
+bundled_resources_resource_id_hash (const char *key)
+{
+	// FIXME: Seed
+	// FIXME: We should cache the hash code so rehashes are cheaper
+	return MurmurHash3_32_streaming ((const uint8_t *)key, 0);
 }
 
+static MonoBundledResource *
+bundled_resources_get (const char *id);
+
 //---------------------------------------------------------------------------------------
 //
 // mono_bundled_resources_add handles bundling of many types of resources to circumvent
@@ -130,7 +151,11 @@ mono_bundled_resources_add (MonoBundledResource **resources_to_bundle, uint32_t
 	g_assert (!domain);
 
 	if (!bundled_resources)
-		bundled_resources = g_hash_table_new_full ((GHashFunc)bundled_resources_resource_id_hash, (GEqualFunc)bundled_resources_resource_id_equal, NULL, bundled_resources_value_destroy_func);
+		// FIXME: Choose a good initial capacity to avoid rehashes during startup. I picked one at random
+		bundled_resources = dn_simdhash_ght_new_full ((GHashFunc)bundled_resources_resource_id_hash, (GEqualFunc)bundled_resources_resource_id_equal, NULL, bundled_resources_value_destroy_func, 2048, NULL);
+
+	if (!bundled_resource_key_lookup_table)
+		bundled_resource_key_lookup_table = dn_simdhash_ptr_ptr_new (2048, NULL);
 
 	bool assemblyAdded = false;
 	bool satelliteAssemblyAdded = false;
@@ -143,7 +168,13 @@ mono_bundled_resources_add (MonoBundledResource **resources_to_bundle, uint32_t
 		if (resource_to_bundle->type == MONO_BUNDLED_SATELLITE_ASSEMBLY)
 			satelliteAssemblyAdded = true;
 
-		g_hash_table_insert (bundled_resources, (gpointer) resource_to_bundle->id, resource_to_bundle);
+		// Generate the hash key for the id (strip certain extensions) and store it
+		//  so that we can free it later when freeing the bundled data
+		char *key = key_from_id (resource_to_bundle->id, NULL, 0);
+		dn_simdhash_ptr_ptr_try_add (bundled_resource_key_lookup_table, (void *)resource_to_bundle->id, key);
+
+		g_assert (dn_simdhash_ght_try_add (bundled_resources, (gpointer) key, resource_to_bundle));
+		// g_assert (bundled_resources_get (resource_to_bundle->id) == resource_to_bundle);
 	}
 
 	if (assemblyAdded)
@@ -172,7 +203,12 @@ bundled_resources_get (const char *id)
 	if (!bundled_resources)
 		return NULL;
 
-	return g_hash_table_lookup (bundled_resources, id);
+	char key_buffer[1024];
+	key_from_id(id, key_buffer, 1024);
+
+	MonoBundledResource *result = NULL;
+	dn_simdhash_ght_try_get_value (bundled_resources, key_buffer, (void **)&result);
+	return result;
 }
 
 //---------------------------------------------------------------------------------------
@@ -364,9 +400,7 @@ bool
 mono_bundled_resources_get_data_resource_values (const char *id, const uint8_t **data_out, uint32_t *size_out)
 {
 	MonoBundledDataResource *bundled_data_resource = bundled_resources_get_data_resource (id);
-	if (!bundled_data_resource ||
-		!bundled_data_resource->data.data ||
-		bundled_data_resource->data.size == 0)
+	if (!bundled_data_resource || !bundled_data_resource->data.data)
 		return false;
 
 	if (data_out)

src/mono/mono/metadata/class.c

@@ -49,6 +49,8 @@
 #include <mono/utils/unlocked.h>
 #include <mono/utils/bsearch.h>
 #include <mono/utils/checked-build.h>
+// for dn_simdhash_ght_t
+#include "../native/containers/dn-simdhash-specializations.h"
 
 MonoStats mono_stats;
 
@@ -1168,7 +1170,7 @@ MonoMethod*
 mono_class_inflate_generic_method_full_checked (MonoMethod *method, MonoClass *klass_hint, MonoGenericContext *context, MonoError *error)
 {
 	MonoMethod *result;
-	MonoMethodInflated *iresult, *cached;
+	MonoMethodInflated *iresult, *cached = NULL;
 	MonoMethodSignature *sig;
 	MonoGenericContext tmp_context;
 
@@ -1223,8 +1225,10 @@ mono_class_inflate_generic_method_full_checked (MonoMethod *method, MonoClass *k
 	// check cache
 	mono_mem_manager_lock (mm);
 	if (!mm->gmethod_cache)
-		mm->gmethod_cache = g_hash_table_new_full (inflated_method_hash, inflated_method_equal, NULL, (GDestroyNotify)free_inflated_method);
-	cached = (MonoMethodInflated *)g_hash_table_lookup (mm->gmethod_cache, iresult);
+		// FIXME: Pick a better pre-reserved size
+		mm->gmethod_cache = dn_simdhash_ght_new_full (inflated_method_hash, inflated_method_equal, NULL, (GDestroyNotify)free_inflated_method, 256, NULL);
+
+	dn_simdhash_ght_try_get_value (mm->gmethod_cache, iresult, (gpointer *)&cached);
 	mono_mem_manager_unlock (mm);
 
 	if (cached) {
@@ -1319,9 +1323,9 @@ mono_class_inflate_generic_method_full_checked (MonoMethod *method, MonoClass *k
 
 	// check cache
 	mono_mem_manager_lock (mm);
-	cached = (MonoMethodInflated *)g_hash_table_lookup (mm->gmethod_cache, iresult);
-	if (!cached) {
-		g_hash_table_insert (mm->gmethod_cache, iresult, iresult);
+	if (!dn_simdhash_ght_try_get_value (mm->gmethod_cache, iresult, (gpointer *)&cached)) {
+		// FIXME: We're wasting memory and cpu by storing key and value redundantly for this table
+		dn_simdhash_ght_try_add (mm->gmethod_cache, iresult, iresult);
 		iresult->owner = mm;
 		cached = iresult;
 	}

src/mono/mono/metadata/image.c

@@ -2120,9 +2120,9 @@ mono_image_close_except_pools (MonoImage *image)
 	}
 
 	if (image->method_cache)
-		g_hash_table_destroy (image->method_cache);
+		dn_simdhash_free (image->method_cache);
 	if (image->methodref_cache)
-		g_hash_table_destroy (image->methodref_cache);
+		dn_simdhash_free (image->methodref_cache);
 	mono_internal_hash_table_destroy (&image->class_cache);
 	mono_conc_hashtable_destroy (image->field_cache);
 	if (image->array_cache) {

src/mono/mono/metadata/loader-internals.h

@@ -18,6 +18,8 @@
 #include <mono/utils/mono-error.h>
 #include <mono/utils/mono-forward.h>
 #include <mono/utils/mono-conc-hashtable.h>
+// for dn_simdhash_ght_t
+#include "../native/containers/dn-simdhash-specializations.h"
 
 #if defined(TARGET_OSX)
 #define MONO_LOADER_LIBRARY_NAME "libcoreclr.dylib"
@@ -174,7 +176,7 @@ struct _MonoMemoryManager {
 	MonoAssemblyLoadContext **alcs;
 
 	// Generic-specific caches
-	GHashTable *ginst_cache, *gmethod_cache, *gsignature_cache;
+	dn_simdhash_ght_t *ginst_cache, *gmethod_cache, *gsignature_cache;
 	MonoConcurrentHashTable *gclass_cache;
 
 	/* mirror caches of ones already on MonoImage. These ones contain generics */

src/mono/mono/metadata/loader.c

@@ -1182,12 +1182,14 @@ mono_get_method_checked (MonoImage *image, guint32 token, MonoClass *klass, Mono
 
 	if (mono_metadata_token_table (token) == MONO_TABLE_METHOD) {
 		if (!image->method_cache)
-			image->method_cache = g_hash_table_new (NULL, NULL);
-		result = (MonoMethod *)g_hash_table_lookup (image->method_cache, GINT_TO_POINTER (mono_metadata_token_index (token)));
+			// FIXME: Pick a better initial size
+			image->method_cache = dn_simdhash_u32_ptr_new (512, NULL);
+		dn_simdhash_u32_ptr_try_get_value (image->method_cache, mono_metadata_token_index (token), (void **)&result);
 	} else if (!image_is_dynamic (image)) {
 		if (!image->methodref_cache)
-			image->methodref_cache = g_hash_table_new (NULL, NULL);
-		result = (MonoMethod *)g_hash_table_lookup (image->methodref_cache, GINT_TO_POINTER (token));
+			// FIXME: Pick a better initial size
+			image->methodref_cache = dn_simdhash_u32_ptr_new (512, NULL);
+		dn_simdhash_u32_ptr_try_get_value (image->methodref_cache, token, (void **)&result);
 	}
 	mono_image_unlock (image);
 
@@ -1204,19 +1206,19 @@ mono_get_method_checked (MonoImage *image, guint32 token, MonoClass *klass, Mono
 		MonoMethod *result2 = NULL;
 
 		if (mono_metadata_token_table (token) == MONO_TABLE_METHOD)
-			result2 = (MonoMethod *)g_hash_table_lookup (image->method_cache, GINT_TO_POINTER (mono_metadata_token_index (token)));
+			dn_simdhash_u32_ptr_try_get_value (image->method_cache, mono_metadata_token_index (token), (void **)&result2);
 		else if (!image_is_dynamic (image))
-			result2 = (MonoMethod *)g_hash_table_lookup (image->methodref_cache, GINT_TO_POINTER (token));
+			dn_simdhash_u32_ptr_try_get_value (image->methodref_cache, token, (void **)&result2);
 
 		if (result2) {
 			mono_image_unlock (image);
 			return result2;
 		}
 
 		if (mono_metadata_token_table (token) == MONO_TABLE_METHOD)
-			g_hash_table_insert (image->method_cache, GINT_TO_POINTER (mono_metadata_token_index (token)), result);
+			dn_simdhash_u32_ptr_try_add (image->method_cache, mono_metadata_token_index (token), result);
 		else if (!image_is_dynamic (image))
-			g_hash_table_insert (image->methodref_cache, GINT_TO_POINTER (token), result);
+			dn_simdhash_u32_ptr_try_add (image->methodref_cache, token, result);
 	}
 
 	mono_image_unlock (image);
@@ -1514,7 +1516,7 @@ mono_method_get_param_token (MonoMethod *method, int index)
 	idx = mono_method_get_index (method);
 	if (idx > 0) {
 		guint param_index = mono_metadata_get_method_params (klass_image, idx, NULL);
-		
+
 		if (index == -1)
 			/* Return value */
 			return mono_metadata_make_token (MONO_TABLE_PARAM, 0);