diff --git a/src/mono/mono/metadata/bundled-resources.c b/src/mono/mono/metadata/bundled-resources.c index 56a3153f0c35c0..bdc32e795fb035 100644 --- a/src/mono/mono/metadata/bundled-resources.c +++ b/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); } //--------------------------------------------------------------------------------------- diff --git a/src/mono/mono/metadata/class.c b/src/mono/mono/metadata/class.c index 1e71f0a2b9dde3..5600f3dc75dd45 100644 --- a/src/mono/mono/metadata/class.c +++ b/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; diff --git a/src/mono/mono/metadata/metadata.c b/src/mono/mono/metadata/metadata.c index d194af42eacdc9..25d77e10d4dcb3 100644 --- a/src/mono/mono/metadata/metadata.c +++ b/src/mono/mono/metadata/metadata.c @@ -3355,7 +3355,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 @@ -3498,8 +3498,7 @@ mono_metadata_get_canonical_generic_inst (MonoGenericInst *candidate) if (!mm->ginst_cache) mm->ginst_cache = dn_simdhash_ght_new_full (mono_metadata_generic_inst_hash, mono_metadata_generic_inst_equal, NULL, (GDestroyNotify)free_generic_inst, 0, NULL); - MonoGenericInst *ginst = NULL; - dn_simdhash_ght_try_get_value (mm->ginst_cache, candidate, (void **)&ginst); + MonoGenericInst *ginst = dn_simdhash_ght_get_value_or_default (mm->ginst_cache, candidate); if (!ginst) { int size = MONO_SIZEOF_GENERIC_INST + type_argc * sizeof (MonoType *); ginst = (MonoGenericInst *)mono_mem_manager_alloc0 (mm, size); diff --git a/src/mono/mono/mini/interp/transform.c b/src/mono/mono/mini/interp/transform.c index 573408c1e85e05..01a53164cc2489 100644 --- a/src/mono/mono/mini/interp/transform.c +++ b/src/mono/mono/mini/interp/transform.c @@ -2735,7 +2735,7 @@ interp_handle_intrinsics (TransformData *td, MonoMethod *target_method, MonoClas default: return FALSE; } } - } + } return FALSE; } @@ -10035,8 +10035,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); } diff --git a/src/mono/mono/mini/seq-points.c b/src/mono/mono/mini/seq-points.c index 0bb7beef2474da..1565c597596480 100644 --- a/src/mono/mono/mini/seq-points.c +++ b/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); diff --git a/src/native/containers/dn-simdhash-ght-compatible.c b/src/native/containers/dn-simdhash-ght-compatible.c index af0cf96365ecfc..78612fd4c675ff 100644 --- a/src/native/containers/dn-simdhash-ght-compatible.c +++ b/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,8 +47,13 @@ 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 #if SIZEOF_VOID_P == 8 @@ -81,6 +65,18 @@ dn_simdhash_ght_replaced (dn_simdhash_ght_data data, void * old_key, void * new_ #include "dn-simdhash-specialization.h" +static unsigned int +dn_simdhash_ght_default_hash (const void * key) +{ + 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, @@ -88,6 +84,13 @@ dn_simdhash_ght_new ( ) { 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 +104,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 +152,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; +} diff --git a/src/native/containers/dn-simdhash-ght-compatible.h b/src/native/containers/dn-simdhash-ght-compatible.h index 4474f02346d3b6..2b87f84b03363d 100644 --- a/src/native/containers/dn-simdhash-ght-compatible.h +++ b/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) diff --git a/src/native/containers/dn-simdhash-ptr-ptr.c b/src/native/containers/dn-simdhash-ptr-ptr.c index 80501ffc3fa992..9f4528ca1f64f6 100644 --- a/src/native/containers/dn-simdhash-ptr-ptr.c +++ b/src/native/containers/dn-simdhash-ptr-ptr.c @@ -11,8 +11,8 @@ #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) #if SIZEOF_VOID_P == 8 #define DN_SIMDHASH_BUCKET_CAPACITY 11 #else diff --git a/src/native/containers/dn-simdhash-ptrpair-ptr.c b/src/native/containers/dn-simdhash-ptrpair-ptr.c index a427c7e76ef9c1..091b047f7dee00 100644 --- a/src/native/containers/dn-simdhash-ptrpair-ptr.c +++ b/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 diff --git a/src/native/containers/dn-simdhash-specialization.h b/src/native/containers/dn-simdhash-specialization.h index 8005d3f2dd54b7..7614a690f5c080 100644 --- a/src/native/containers/dn-simdhash-specialization.h +++ b/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++; diff --git a/src/native/containers/dn-simdhash-string-ptr.c b/src/native/containers/dn-simdhash-string-ptr.c index 4bd03896506657..c3a93ceac9cdba 100644 --- a/src/native/containers/dn-simdhash-string-ptr.c +++ b/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. diff --git a/src/native/containers/dn-simdhash-u32-ptr.c b/src/native/containers/dn-simdhash-u32-ptr.c index a3e1a77a92c2a6..de4d62bf644a00 100644 --- a/src/native/containers/dn-simdhash-u32-ptr.c +++ b/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" diff --git a/src/native/containers/simdhash-benchmark/Makefile b/src/native/containers/simdhash-benchmark/Makefile index 6349d1c96538b5..1ba9e3564ac1f0 100644 --- a/src/native/containers/simdhash-benchmark/Makefile +++ b/src/native/containers/simdhash-benchmark/Makefile @@ -5,7 +5,7 @@ benchmark_deps := $(wildcard ./*.c) $(wildcard ./*.h) # I don't know why this is necessary nodejs_path := $(shell which node) -benchmark_sources := ../dn-simdhash.c ../dn-vector.c ./benchmark.c ../dn-simdhash-u32-ptr.c ../dn-simdhash-string-ptr.c ./ghashtable.c ./all-measurements.c +benchmark_sources := ../dn-simdhash.c ../dn-vector.c ./benchmark.c ../dn-simdhash-u32-ptr.c ../dn-simdhash-string-ptr.c ../dn-simdhash-ght-compatible.c ./ghashtable.c ./all-measurements.c common_options := -g -O3 -DNO_CONFIG_H -lm -DNDEBUG ifeq ($(SIMD), 0) wasm_options := -mbulk-memory @@ -15,12 +15,10 @@ endif benchmark-native: $(dn_deps) $(benchmark_deps) clang $(benchmark_sources) $(common_options) -DSIZEOF_VOID_P=8 + objdump -S -d --no-show-raw-insn ./a.out > ./a.dis benchmark-wasm: $(dn_deps) $(benchmark_deps) ~/Projects/emscripten/emcc $(benchmark_sources) $(common_options) $(wasm_options) -DSIZEOF_VOID_P=4 - -disassemble-benchmark: benchmark-native benchmark-wasm - objdump -d ./a.out > ./a.dis ~/wabt/bin/wasm2wat ./a.out.wasm > ./a.wat run-native: benchmark-native @@ -29,5 +27,5 @@ run-native: benchmark-native run-wasm: benchmark-wasm $(nodejs_path) ./a.out.js $(FILTER) -default_target: disassemble-benchmark +default_target: benchmark-native diff --git a/src/native/containers/simdhash-benchmark/all-measurements.h b/src/native/containers/simdhash-benchmark/all-measurements.h index e6646f203e49e9..416f3c519ac492 100644 --- a/src/native/containers/simdhash-benchmark/all-measurements.h +++ b/src/native/containers/simdhash-benchmark/all-measurements.h @@ -132,6 +132,30 @@ static void destroy_instance_ght (void *data) { g_hash_table_destroy((GHashTable *)data); } + +static void * create_instance_dnght () { + if (!random_u32s) + init_data(); + + return dn_simdhash_ght_new(NULL, NULL, 0, NULL); +} + +static void * create_instance_dnght_random_values () { + if (!random_u32s) + init_data(); + + dn_simdhash_ght_t *result = dn_simdhash_ght_new(NULL, NULL, INNER_COUNT, NULL); + for (int i = 0; i < INNER_COUNT; i++) { + uint32_t key = *dn_vector_index_t(random_u32s, uint32_t, i); + dn_simdhash_ght_try_add(result, (gpointer)(size_t)key, (gpointer)(size_t)i); + } + return result; +} + +static void destroy_instance_dnght (void *data) { + dn_simdhash_free((dn_simdhash_ght_t *)data); +} + #endif // MEASUREMENTS_IMPLEMENTATION // These go outside the guard because we include this file multiple times. @@ -217,3 +241,17 @@ MEASUREMENT(ght_find_missing_key, GHashTable *, create_instance_ght_random_value dn_simdhash_assert(g_hash_table_lookup(data, (gpointer)(size_t)key) == NULL); } }) + +MEASUREMENT(dnght_find_random_keys, dn_simdhash_ght_t *, create_instance_dnght_random_values, destroy_instance_dnght, { + for (int i = 0; i < INNER_COUNT; i++) { + uint32_t key = *dn_vector_index_t(random_u32s, uint32_t, i); + dn_simdhash_assert(dn_simdhash_ght_get_value_or_default(data, (gpointer)(size_t)key) == (gpointer)(size_t)i); + } +}) + +MEASUREMENT(dnght_find_missing_key, dn_simdhash_ght_t *, create_instance_dnght_random_values, destroy_instance_dnght, { + for (int i = 0; i < INNER_COUNT; i++) { + uint32_t key = *dn_vector_index_t(random_unused_u32s, uint32_t, i); + dn_simdhash_assert(!dn_simdhash_ght_get_value_or_default(data, (gpointer)(size_t)key)); + } +})