Add equivalent operation for g_hash_table_insert_replace

Rename simdhash replace operation to make it clear that it only replaces the value

src/mono/mono/mini/interp/tiering.c

@@ -107,7 +107,7 @@ register_imethod_patch_site (InterpMethod *imethod, gpointer *ptr)
 	guint8 found = dn_simdhash_ptr_ptr_try_get_value (patch_sites_table, imethod, (void **)&sites);
 	sites = g_slist_prepend (sites, ptr);
 	if (found)
-		dn_simdhash_ptr_ptr_try_replace (patch_sites_table, imethod, sites);
+		dn_simdhash_ptr_ptr_try_replace_value (patch_sites_table, imethod, sites);
 	else
 		dn_simdhash_ptr_ptr_try_add (patch_sites_table, imethod, sites);
 }

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

@@ -1136,7 +1136,7 @@ get_data_item_wide_index (TransformData *td, void *ptr, gboolean *new_slot)
 	td->data_items [index] = ptr;
 	++td->n_data_items;
 	if (found)
-		dn_simdhash_ptr_ptr_try_replace (td->data_hash, ptr, GUINT_TO_POINTER (index + 1));
+		dn_simdhash_ptr_ptr_try_replace_value (td->data_hash, ptr, GUINT_TO_POINTER (index + 1));
 	else
 		dn_simdhash_ptr_ptr_try_add (td->data_hash, ptr, GUINT_TO_POINTER (index + 1));
 	if (new_slot)
@@ -9386,13 +9386,9 @@ 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;
-		uint8_t match = dn_simdhash_ght_try_get_value (jit_mm->seq_points, imethod->method, (void **)&seq_points);
-		if (!seq_points || seq_points != imethod->jinfo->seq_points) {
-			if (match)
-				dn_simdhash_ght_try_replace (jit_mm->seq_points, imethod->method, imethod->jinfo->seq_points);
-			else
-				dn_simdhash_ght_try_add (jit_mm->seq_points, imethod->method, imethod->jinfo->seq_points);
-		}
+		dn_simdhash_ght_try_get_value (jit_mm->seq_points, imethod->method, (void **)&seq_points);
+		if (!seq_points || seq_points != imethod->jinfo->seq_points)
+			dn_simdhash_ght_insert_replace (jit_mm->seq_points, imethod->method, imethod->jinfo->seq_points, 0);
 	}
 	jit_mm_unlock (jit_mm);
 

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

@@ -55,14 +55,19 @@ dn_simdhash_ght_removed (dn_simdhash_ght_data data, gpointer key, gpointer value
 }
 
 static inline void
-dn_simdhash_ght_replaced (dn_simdhash_ght_data data, gpointer key, gpointer old_value, gpointer new_value)
+dn_simdhash_ght_replaced (dn_simdhash_ght_data data, gpointer old_key, gpointer new_key, gpointer old_value, gpointer new_value)
 {
-	if (old_value == new_value)
-		return;
+	if (old_key != new_key) {
+		GDestroyNotify key_destroy_func = data.key_destroy_func;
+		if (key_destroy_func)
+			key_destroy_func((gpointer)old_key);
+	}
 
-	GDestroyNotify value_destroy_func = data.value_destroy_func;
-	if (value_destroy_func)
-		value_destroy_func((gpointer)old_value);
+	if (old_value != new_value) {
+		GDestroyNotify value_destroy_func = data.value_destroy_func;
+		if (value_destroy_func)
+			value_destroy_func((gpointer)old_value);
+	}
 }
 
 #define DN_SIMDHASH_T dn_simdhash_ght
@@ -109,3 +114,41 @@ dn_simdhash_ght_new_full (
 	dn_simdhash_instance_data(dn_simdhash_ght_data, hash).value_destroy_func = value_destroy_func;
 	return hash;
 }
+
+void
+dn_simdhash_ght_insert_replace (
+	dn_simdhash_ght_t *hash,
+	gpointer key, gpointer value,
+	gboolean overwrite_key
+)
+{
+	check_self(hash);
+	uint32_t key_hash = DN_SIMDHASH_KEY_HASHER(DN_SIMDHASH_GET_DATA(hash), key);
+	dn_simdhash_insert_mode imode = overwrite_key
+		? DN_SIMDHASH_INSERT_MODE_OVERWRITE_KEY_AND_VALUE
+		: DN_SIMDHASH_INSERT_MODE_OVERWRITE_VALUE;
+
+	dn_simdhash_insert_result ok = DN_SIMDHASH_TRY_INSERT_INTERNAL(hash, key, key_hash, value, imode);
+	if (ok == DN_SIMDHASH_INSERT_NEED_TO_GROW) {
+		dn_simdhash_buffers_t old_buffers = dn_simdhash_ensure_capacity_internal(hash, dn_simdhash_capacity(hash) + 1);
+		if (old_buffers.buckets) {
+			DN_SIMDHASH_REHASH_INTERNAL(hash, old_buffers);
+			dn_simdhash_free_buffers(old_buffers);
+		}
+		ok = DN_SIMDHASH_TRY_INSERT_INTERNAL(hash, key, key_hash, value, imode);
+	}
+
+	switch (ok) {
+		case DN_SIMDHASH_INSERT_OK_ADDED_NEW:
+			hash->count++;
+			return;
+		case DN_SIMDHASH_INSERT_OK_OVERWROTE_EXISTING:
+			return;
+		// We should always return one of the first two
+		case DN_SIMDHASH_INSERT_KEY_ALREADY_PRESENT:
+		case DN_SIMDHASH_INSERT_NEED_TO_GROW:
+		default:
+			assert(0);
+			return;
+	}
+}

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

@@ -10,3 +10,16 @@ dn_simdhash_ght_new_full (
 	GDestroyNotify key_destroy_func, GDestroyNotify value_destroy_func,
 	uint32_t capacity, dn_allocator_t *allocator
 );
+
+// compatible with g_hash_table_insert_replace
+void
+dn_simdhash_ght_insert_replace (
+    dn_simdhash_ght_t *hash,
+    gpointer key, gpointer value,
+    gboolean overwrite_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)
+#define dn_simdhash_ght_add(h,k)       dn_simdhash_ght_insert_replace ((h),(k),(k),TRUE)

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

@@ -32,8 +32,8 @@
 #define DN_SIMDHASH_TRY_GET_VALUE_WITH_HASH DN_SIMDHASH_GLUE_3(DN_SIMDHASH_T,_try_get_value_with_hash,DN_SIMDHASH_ACCESSOR_SUFFIX)
 #define DN_SIMDHASH_TRY_REMOVE DN_SIMDHASH_GLUE_3(DN_SIMDHASH_T,_try_remove,DN_SIMDHASH_ACCESSOR_SUFFIX)
 #define DN_SIMDHASH_TRY_REMOVE_WITH_HASH DN_SIMDHASH_GLUE_3(DN_SIMDHASH_T,_try_remove_with_hash,DN_SIMDHASH_ACCESSOR_SUFFIX)
-#define DN_SIMDHASH_TRY_REPLACE DN_SIMDHASH_GLUE_3(DN_SIMDHASH_T,_try_replace,DN_SIMDHASH_ACCESSOR_SUFFIX)
-#define DN_SIMDHASH_TRY_REPLACE_WITH_HASH DN_SIMDHASH_GLUE_3(DN_SIMDHASH_T,_try_replace_with_hash,DN_SIMDHASH_ACCESSOR_SUFFIX)
+#define DN_SIMDHASH_TRY_REPLACE_VALUE DN_SIMDHASH_GLUE_3(DN_SIMDHASH_T,_try_replace_value,DN_SIMDHASH_ACCESSOR_SUFFIX)
+#define DN_SIMDHASH_TRY_REPLACE_VALUE_WITH_HASH DN_SIMDHASH_GLUE_3(DN_SIMDHASH_T,_try_replace_value_with_hash,DN_SIMDHASH_ACCESSOR_SUFFIX)
 #define DN_SIMDHASH_FOREACH DN_SIMDHASH_GLUE_3(DN_SIMDHASH_T,_foreach,DN_SIMDHASH_ACCESSOR_SUFFIX)
 #define DN_SIMDHASH_FOREACH_FUNC DN_SIMDHASH_GLUE_3(DN_SIMDHASH_T,_foreach_func,DN_SIMDHASH_ACCESSOR_SUFFIX)
 #define DN_SIMDHASH_DESTROY_ALL DN_SIMDHASH_GLUE(DN_SIMDHASH_T,_destroy_all)
@@ -67,10 +67,10 @@ uint8_t
 DN_SIMDHASH_TRY_REMOVE_WITH_HASH (DN_SIMDHASH_T_PTR hash, DN_SIMDHASH_KEY_T key, uint32_t key_hash);
 
 uint8_t
-DN_SIMDHASH_TRY_REPLACE (DN_SIMDHASH_T_PTR hash, DN_SIMDHASH_KEY_T key, DN_SIMDHASH_VALUE_T new_value);
+DN_SIMDHASH_TRY_REPLACE_VALUE (DN_SIMDHASH_T_PTR hash, DN_SIMDHASH_KEY_T key, DN_SIMDHASH_VALUE_T new_value);
 
 uint8_t
-DN_SIMDHASH_TRY_REPLACE_WITH_HASH (DN_SIMDHASH_T_PTR hash, DN_SIMDHASH_KEY_T key, uint32_t key_hash, DN_SIMDHASH_VALUE_T new_value);
+DN_SIMDHASH_TRY_REPLACE_VALUE_WITH_HASH (DN_SIMDHASH_T_PTR hash, DN_SIMDHASH_KEY_T key, uint32_t key_hash, DN_SIMDHASH_VALUE_T new_value);
 
 void
 DN_SIMDHASH_FOREACH (DN_SIMDHASH_T_PTR hash, DN_SIMDHASH_FOREACH_FUNC func, void *user_data);

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

@@ -43,7 +43,7 @@
 
 #ifndef DN_SIMDHASH_ON_REPLACE
 #define DN_SIMDHASH_HAS_REPLACE_HANDLER 0
-#define DN_SIMDHASH_ON_REPLACE(data, key, old_value, new_value)
+#define DN_SIMDHASH_ON_REPLACE(data, old_key, new_key, old_value, new_value)
 #else // DN_SIMDHASH_ON_REPLACE
 #define DN_SIMDHASH_HAS_REPLACE_HANDLER 1
 #ifndef DN_SIMDHASH_ON_REMOVE
@@ -57,7 +57,7 @@
 #else // DN_SIMDHASH_ON_REMOVE
 #define DN_SIMDHASH_HAS_REMOVE_HANDLER 1
 #ifndef DN_SIMDHASH_ON_REPLACE
-#error Expected DN_SIMDHASH_ON_REPLACE(data, key, old_value, new_value) to be defined.
+#error Expected DN_SIMDHASH_ON_REPLACE(data, old_key, new_key, old_value, new_value) to be defined.
 #endif
 #endif // DN_SIMDHASH_ON_REMOVE
 
@@ -232,8 +232,38 @@ DN_SIMDHASH_FIND_VALUE_INTERNAL (DN_SIMDHASH_T_PTR hash, DN_SIMDHASH_KEY_T key,
 	return NULL;
 }
 
+typedef enum dn_simdhash_insert_mode {
+	// Ensures that no matching key exists in the hash, then adds the key/value pair
+	DN_SIMDHASH_INSERT_MODE_ENSURE_UNIQUE,
+	// If a matching key exists in the hash, overwrite its value but leave the key alone
+	DN_SIMDHASH_INSERT_MODE_OVERWRITE_VALUE,
+	// If a matching key exists in the hash, overwrite both the key and the value
+	DN_SIMDHASH_INSERT_MODE_OVERWRITE_KEY_AND_VALUE,
+	// Do not scan for existing matches before adding the new key/value pair.
+	DN_SIMDHASH_INSERT_MODE_REHASHING,
+} dn_simdhash_insert_mode;
+
+static void
+do_overwrite (
+	DN_SIMDHASH_T_PTR hash, uint32_t bucket_index, bucket_t *bucket_address, int index_in_bucket,
+	DN_SIMDHASH_KEY_T key, DN_SIMDHASH_VALUE_T value, uint8_t overwrite_key
+) {
+	DN_SIMDHASH_KEY_T *key_ptr = &bucket_address->keys[index_in_bucket];
+	DN_SIMDHASH_VALUE_T *value_ptr = address_of_value(hash->buffers, (bucket_index * DN_SIMDHASH_BUCKET_CAPACITY) + index_in_bucket);
+#if DN_SIMDHASH_HAS_REPLACE_HANDLER
+	DN_SIMDHASH_KEY_T old_key = *key_ptr;
+	DN_SIMDHASH_VALUE_T old_value = *value_ptr;
+#endif
+	if (overwrite_key)
+		*key_ptr = key;
+	*value_ptr = value;
+#if DN_SIMDHASH_HAS_REPLACE_HANDLER
+	DN_SIMDHASH_ON_REPLACE(DN_SIMDHASH_GET_DATA(hash), overwrite_key ? old_key : key, key, old_value, value);
+#endif
+}
+
 static dn_simdhash_insert_result
-DN_SIMDHASH_TRY_INSERT_INTERNAL (DN_SIMDHASH_T_PTR hash, DN_SIMDHASH_KEY_T key, uint32_t key_hash, DN_SIMDHASH_VALUE_T value, uint8_t ensure_not_present)
+DN_SIMDHASH_TRY_INSERT_INTERNAL (DN_SIMDHASH_T_PTR hash, DN_SIMDHASH_KEY_T key, uint32_t key_hash, DN_SIMDHASH_VALUE_T value, dn_simdhash_insert_mode mode)
 {
 	// HACK: Early out. Better to grow without scanning here.
 	// We're comparing with the computed grow_at_count threshold to maintain an appropriate load factor
@@ -249,10 +279,21 @@ DN_SIMDHASH_TRY_INSERT_INTERNAL (DN_SIMDHASH_T_PTR hash, DN_SIMDHASH_KEY_T key,
 
 	BEGIN_SCAN_BUCKETS(first_bucket_index, bucket_index, bucket_address)
 		// If necessary, check the current bucket for the key
-		if (ensure_not_present) {
+		if (mode != DN_SIMDHASH_INSERT_MODE_REHASHING) {
 			int index_in_bucket = DN_SIMDHASH_SCAN_BUCKET_INTERNAL(hash, bucket_address, key, search_vector);
-			if (index_in_bucket >= 0)
-				return DN_SIMDHASH_INSERT_KEY_ALREADY_PRESENT;
+			if (index_in_bucket >= 0) {
+				if (
+					(mode == DN_SIMDHASH_INSERT_MODE_OVERWRITE_KEY_AND_VALUE) ||
+					(mode == DN_SIMDHASH_INSERT_MODE_OVERWRITE_VALUE)
+				) {
+					do_overwrite (
+						hash, bucket_index, bucket_address, index_in_bucket,
+						key, value, (mode == DN_SIMDHASH_INSERT_MODE_OVERWRITE_KEY_AND_VALUE)
+					);
+					return DN_SIMDHASH_INSERT_OK_OVERWROTE_EXISTING;
+				} else
+					return DN_SIMDHASH_INSERT_KEY_ALREADY_PRESENT;
+			}
 		}
 
 		// The current bucket doesn't contain the key, or duplicate checks are disabled (for rehashing),
@@ -272,7 +313,7 @@ DN_SIMDHASH_TRY_INSERT_INTERNAL (DN_SIMDHASH_T_PTR hash, DN_SIMDHASH_KEY_T key,
 			//  during the process of getting here we may end up finding a duplicate, which would
 			//  leave the cascade counters in a corrupted state
 			adjust_cascaded_counts(buffers, first_bucket_index, bucket_index, 1);
-			return DN_SIMDHASH_INSERT_OK;
+			return DN_SIMDHASH_INSERT_OK_ADDED_NEW;
 		}
 
 		// The current bucket is full, so try the next bucket.
@@ -291,10 +332,10 @@ DN_SIMDHASH_REHASH_INTERNAL (DN_SIMDHASH_T_PTR hash, dn_simdhash_buffers_t old_b
 		dn_simdhash_insert_result ok = DN_SIMDHASH_TRY_INSERT_INTERNAL(
 			hash, *key_address, key_hash,
 			*value_address,
-			0
+			DN_SIMDHASH_INSERT_MODE_REHASHING
 		);
 		// FIXME: Why doesn't assert(ok) work here? Clang says it's unused
-		if (ok != DN_SIMDHASH_INSERT_OK)
+		if (ok != DN_SIMDHASH_INSERT_OK_ADDED_NEW)
 			assert(0);
 	END_SCAN_PAIRS(old_buffers, key_address, value_address)
 }
@@ -352,20 +393,24 @@ DN_SIMDHASH_TRY_ADD_WITH_HASH (DN_SIMDHASH_T_PTR hash, DN_SIMDHASH_KEY_T key, ui
 {
 	check_self(hash);
 
-	dn_simdhash_insert_result ok = DN_SIMDHASH_TRY_INSERT_INTERNAL(hash, key, key_hash, value, 1);
+	dn_simdhash_insert_result ok = DN_SIMDHASH_TRY_INSERT_INTERNAL(hash, key, key_hash, value, DN_SIMDHASH_INSERT_MODE_ENSURE_UNIQUE);
 	if (ok == DN_SIMDHASH_INSERT_NEED_TO_GROW) {
 		dn_simdhash_buffers_t old_buffers = dn_simdhash_ensure_capacity_internal(hash, dn_simdhash_capacity(hash) + 1);
 		if (old_buffers.buckets) {
 			DN_SIMDHASH_REHASH_INTERNAL(hash, old_buffers);
 			dn_simdhash_free_buffers(old_buffers);
 		}
-		ok = DN_SIMDHASH_TRY_INSERT_INTERNAL(hash, key, key_hash, value, 1);
+		ok = DN_SIMDHASH_TRY_INSERT_INTERNAL(hash, key, key_hash, value, DN_SIMDHASH_INSERT_MODE_ENSURE_UNIQUE);
 	}
 
 	switch (ok) {
-		case DN_SIMDHASH_INSERT_OK:
+		case DN_SIMDHASH_INSERT_OK_ADDED_NEW:
 			hash->count++;
 			return 1;
+		case DN_SIMDHASH_INSERT_OK_OVERWROTE_EXISTING:
+			// This shouldn't happen
+			assert(0);
+			return 1;
 		case DN_SIMDHASH_INSERT_KEY_ALREADY_PRESENT:
 			return 0;
 		case DN_SIMDHASH_INSERT_NEED_TO_GROW:
@@ -489,16 +534,16 @@ DN_SIMDHASH_TRY_REMOVE_WITH_HASH (DN_SIMDHASH_T_PTR hash, DN_SIMDHASH_KEY_T key,
 }
 
 uint8_t
-DN_SIMDHASH_TRY_REPLACE (DN_SIMDHASH_T_PTR hash, DN_SIMDHASH_KEY_T key, DN_SIMDHASH_VALUE_T new_value)
+DN_SIMDHASH_TRY_REPLACE_VALUE (DN_SIMDHASH_T_PTR hash, DN_SIMDHASH_KEY_T key, DN_SIMDHASH_VALUE_T new_value)
 {
 	check_self(hash);
 
 	uint32_t key_hash = DN_SIMDHASH_KEY_HASHER(DN_SIMDHASH_GET_DATA(hash), key);
-	return DN_SIMDHASH_TRY_REPLACE_WITH_HASH(hash, key, key_hash, new_value);
+	return DN_SIMDHASH_TRY_REPLACE_VALUE_WITH_HASH(hash, key, key_hash, new_value);
 }
 
 uint8_t
-DN_SIMDHASH_TRY_REPLACE_WITH_HASH (DN_SIMDHASH_T_PTR hash, DN_SIMDHASH_KEY_T key, uint32_t key_hash, DN_SIMDHASH_VALUE_T new_value)
+DN_SIMDHASH_TRY_REPLACE_VALUE_WITH_HASH (DN_SIMDHASH_T_PTR hash, DN_SIMDHASH_KEY_T key, uint32_t key_hash, DN_SIMDHASH_VALUE_T new_value)
 {
 	check_self(hash);
 
@@ -511,7 +556,7 @@ DN_SIMDHASH_TRY_REPLACE_WITH_HASH (DN_SIMDHASH_T_PTR hash, DN_SIMDHASH_KEY_T key
 #endif
 	*value_ptr = new_value;
 #if DN_SIMDHASH_HAS_REPLACE_HANDLER
-	DN_SIMDHASH_ON_REPLACE(DN_SIMDHASH_GET_DATA(hash), key, old_value, new_value);
+	DN_SIMDHASH_ON_REPLACE(DN_SIMDHASH_GET_DATA(hash), key, key, old_value, new_value);
 #endif
 	return 1;
 }

src/native/containers/dn-simdhash.h

@@ -59,7 +59,8 @@ typedef struct dn_simdhash_meta_t {
 } dn_simdhash_meta_t;
 
 typedef enum dn_simdhash_insert_result {
-	DN_SIMDHASH_INSERT_OK,
+	DN_SIMDHASH_INSERT_OK_ADDED_NEW,
+	DN_SIMDHASH_INSERT_OK_OVERWROTE_EXISTING,
 	DN_SIMDHASH_INSERT_NEED_TO_GROW,
 	DN_SIMDHASH_INSERT_KEY_ALREADY_PRESENT,
 } dn_simdhash_insert_result;