From 16bd574b4efbbb4306e46de3c2769c162e742758 Mon Sep 17 00:00:00 2001 From: Katelyn Gadd Date: Wed, 3 Apr 2024 20:35:36 -0700 Subject: [PATCH] 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 | 2 +- src/mono/mono/mini/interp/transform.c | 12 +-- .../containers/dn-simdhash-ght-compatible.c | 55 +++++++++++-- .../containers/dn-simdhash-ght-compatible.h | 13 ++++ .../dn-simdhash-specialization-declarations.h | 8 +- .../containers/dn-simdhash-specialization.h | 77 +++++++++++++++---- src/native/containers/dn-simdhash.h | 3 +- 7 files changed, 134 insertions(+), 36 deletions(-) diff --git a/src/mono/mono/mini/interp/tiering.c b/src/mono/mono/mini/interp/tiering.c index 35665d44e2521a..a49b591e9d090e 100644 --- a/src/mono/mono/mini/interp/tiering.c +++ b/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); } diff --git a/src/mono/mono/mini/interp/transform.c b/src/mono/mono/mini/interp/transform.c index f1cef6afb5424d..c0922dfa46a62b 100644 --- a/src/mono/mono/mini/interp/transform.c +++ b/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); diff --git a/src/native/containers/dn-simdhash-ght-compatible.c b/src/native/containers/dn-simdhash-ght-compatible.c index 85a925433f0968..850a3606a299fe 100644 --- a/src/native/containers/dn-simdhash-ght-compatible.c +++ b/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; + } +} diff --git a/src/native/containers/dn-simdhash-ght-compatible.h b/src/native/containers/dn-simdhash-ght-compatible.h index 3611a3a3c4a4a5..d70193e792cb9f 100644 --- a/src/native/containers/dn-simdhash-ght-compatible.h +++ b/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) diff --git a/src/native/containers/dn-simdhash-specialization-declarations.h b/src/native/containers/dn-simdhash-specialization-declarations.h index 84f49c00c13441..585f2094ee58c8 100644 --- a/src/native/containers/dn-simdhash-specialization-declarations.h +++ b/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); diff --git a/src/native/containers/dn-simdhash-specialization.h b/src/native/containers/dn-simdhash-specialization.h index 0cbbb8e8a6a637..a5069c2e66905b 100644 --- a/src/native/containers/dn-simdhash-specialization.h +++ b/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; } diff --git a/src/native/containers/dn-simdhash.h b/src/native/containers/dn-simdhash.h index e2245479efe9f9..65f2e825ca4f9e 100644 --- a/src/native/containers/dn-simdhash.h +++ b/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;