diff --git a/src/coreclr/interpreter/compiler.h b/src/coreclr/interpreter/compiler.h index 6a534f1152c0dd..c483c6fba51d66 100644 --- a/src/coreclr/interpreter/compiler.h +++ b/src/coreclr/interpreter/compiler.h @@ -9,52 +9,8 @@ #include "datastructs.h" #include "enum_class_flags.h" #include - -#include "../../native/containers/dn-simdhash.h" -#include "../../native/containers/dn-simdhash-specializations.h" -#include "../../native/containers/dn-simdhash-utils.h" - -class dn_simdhash_ptr_ptr_holder -{ - dn_simdhash_ptr_ptr_t *Value; -public: - dn_simdhash_ptr_ptr_holder() : - Value(nullptr) - { - } - - dn_simdhash_ptr_ptr_t* GetValue() - { - if (!Value) - Value = dn_simdhash_ptr_ptr_new(0, nullptr); - return Value; - } - - dn_simdhash_ptr_ptr_holder(const dn_simdhash_ptr_ptr_holder&) = delete; - dn_simdhash_ptr_ptr_holder& operator=(const dn_simdhash_ptr_ptr_holder&) = delete; - dn_simdhash_ptr_ptr_holder(dn_simdhash_ptr_ptr_holder&& other) - { - Value = other.Value; - other.Value = nullptr; - } - dn_simdhash_ptr_ptr_holder& operator=(dn_simdhash_ptr_ptr_holder&& other) - { - if (this != &other) - { - if (Value != nullptr) - dn_simdhash_free(Value); - Value = other.Value; - other.Value = nullptr; - } - return *this; - } - - ~dn_simdhash_ptr_ptr_holder() - { - if (Value != nullptr) - dn_simdhash_free(Value); - } -}; +#include "failures.h" +#include "simdhash.h" struct InterpException { @@ -67,16 +23,6 @@ struct InterpException const CorJitResult m_result; }; -#if defined(__GNUC__) || defined(__clang__) -#define INTERPRETER_NORETURN __attribute__((noreturn)) -#else -#define INTERPRETER_NORETURN __declspec(noreturn) -#endif - -INTERPRETER_NORETURN void NO_WAY(const char* message); -INTERPRETER_NORETURN void BADCODE(const char* message); -INTERPRETER_NORETURN void NOMEM(); - class InterpCompiler; class InterpDataItemIndexMap @@ -540,15 +486,11 @@ class InterpCompiler dn_simdhash_ptr_ptr_holder m_pointerToNameMap; bool PointerInNameMap(void* ptr) { - if (dn_simdhash_ptr_ptr_try_get_value(m_pointerToNameMap.GetValue(), ptr, NULL)) - { - return true; - } - return false; + return dn_simdhash_ptr_ptr_try_get_value(m_pointerToNameMap.GetValue(), ptr, NULL) != 0; } void AddPointerToNameMap(void* ptr, const char* name) { - dn_simdhash_ptr_ptr_try_add(m_pointerToNameMap.GetValue(), ptr, (void*)name); + checkNoError(dn_simdhash_ptr_ptr_try_add(m_pointerToNameMap.GetValue(), ptr, (void*)name)); } void PrintNameInPointerMap(void* ptr); #endif @@ -865,7 +807,10 @@ int32_t InterpDataItemIndexMap::GetDataItemIndexForT(const T& lookup) VarSizedDataWithPayload* pLookup = new(hashItemPayload) VarSizedDataWithPayload(); memcpy(&pLookup->payload, &lookup, sizeof(T)); - dn_simdhash_ght_insert(hash, (void*)pLookup, (void*)(size_t)dataItemIndex); + checkAddedNew(dn_simdhash_ght_try_insert( + hash, (void*)pLookup, (void*)(size_t)dataItemIndex, DN_SIMDHASH_INSERT_MODE_ENSURE_UNIQUE + )); + return dataItemIndex; } diff --git a/src/coreclr/interpreter/failures.h b/src/coreclr/interpreter/failures.h new file mode 100644 index 00000000000000..9532b7bfba0c45 --- /dev/null +++ b/src/coreclr/interpreter/failures.h @@ -0,0 +1,17 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +#ifndef _FAILURES_H_ +#define _FAILURES_H_ + +#if defined(__GNUC__) || defined(__clang__) +#define INTERPRETER_NORETURN __attribute__((noreturn)) +#else +#define INTERPRETER_NORETURN __declspec(noreturn) +#endif + +INTERPRETER_NORETURN void NO_WAY(const char* message); +INTERPRETER_NORETURN void BADCODE(const char* message); +INTERPRETER_NORETURN void NOMEM(); + +#endif diff --git a/src/coreclr/interpreter/interpreter.h b/src/coreclr/interpreter/interpreter.h index 90c62b98af6dfe..969a3c3c73ee0c 100644 --- a/src/coreclr/interpreter/interpreter.h +++ b/src/coreclr/interpreter/interpreter.h @@ -27,6 +27,7 @@ #ifdef DEBUG extern "C" void assertAbort(const char* why, const char* file, unsigned line); + #undef assert #define assert(p) (void)((p) || (assertAbort(#p, __FILE__, __LINE__), 0)) #endif // DEBUG diff --git a/src/coreclr/interpreter/simdhash.h b/src/coreclr/interpreter/simdhash.h new file mode 100644 index 00000000000000..6396e4fb325f2b --- /dev/null +++ b/src/coreclr/interpreter/simdhash.h @@ -0,0 +1,72 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +#ifndef _SIMDHASH_H_ +#define _SIMDHASH_H_ + +#include "failures.h" +#include "../../native/containers/dn-simdhash.h" +#include "../../native/containers/dn-simdhash-specializations.h" +#include "../../native/containers/dn-simdhash-utils.h" + +class dn_simdhash_ptr_ptr_holder +{ + dn_simdhash_ptr_ptr_t *Value; +public: + dn_simdhash_ptr_ptr_holder() : + Value(nullptr) + { + } + + dn_simdhash_ptr_ptr_t* GetValue() + { + if (!Value) + Value = dn_simdhash_ptr_ptr_new(0, nullptr); + return Value; + } + + dn_simdhash_ptr_ptr_holder(const dn_simdhash_ptr_ptr_holder&) = delete; + dn_simdhash_ptr_ptr_holder& operator=(const dn_simdhash_ptr_ptr_holder&) = delete; + dn_simdhash_ptr_ptr_holder(dn_simdhash_ptr_ptr_holder&& other) + { + Value = other.Value; + other.Value = nullptr; + } + dn_simdhash_ptr_ptr_holder& operator=(dn_simdhash_ptr_ptr_holder&& other) + { + if (this != &other) + { + if (Value != nullptr) + dn_simdhash_free(Value); + Value = other.Value; + other.Value = nullptr; + } + return *this; + } + + ~dn_simdhash_ptr_ptr_holder() + { + if (Value != nullptr) + dn_simdhash_free(Value); + } +}; + +// Asserts that no error occurred during a simdhash add, but does not mind if no new item was inserted +inline void checkNoError(dn_simdhash_add_result result) +{ + if (result == DN_SIMDHASH_OUT_OF_MEMORY) + NOMEM(); + else if (result < 0) + NO_WAY("Internal error in simdhash"); +} + +// Asserts that a new item was successfully inserted into the simdhash +inline void checkAddedNew(dn_simdhash_add_result result) +{ + if (result == DN_SIMDHASH_OUT_OF_MEMORY) + NOMEM(); + else if (result != DN_SIMDHASH_ADD_INSERTED) + NO_WAY("Failed to add new item into simdhash"); +} + +#endif // _SIMDHASH_H_ diff --git a/src/coreclr/interpreter/stackmap.cpp b/src/coreclr/interpreter/stackmap.cpp index 340b5dbb7fce76..525115f3ca6e79 100644 --- a/src/coreclr/interpreter/stackmap.cpp +++ b/src/coreclr/interpreter/stackmap.cpp @@ -6,15 +6,16 @@ #include "interpreter.h" #include "stackmap.h" -#include "../../native/containers/dn-simdhash.h" -#include "../../native/containers/dn-simdhash-specializations.h" -#include "../../native/containers/dn-simdhash-utils.h" - -extern "C" void assertAbort(const char* why, const char* file, unsigned line); +#include "failures.h" +#include "simdhash.h" void dn_simdhash_assert_fail (const char* file, int line, const char* condition) { +#if DEBUG assertAbort(condition, file, line); +#else + NO_WAY(condition); +#endif } thread_local dn_simdhash_ptr_ptr_t *t_sharedStackMapLookup = nullptr; @@ -24,10 +25,13 @@ InterpreterStackMap* GetInterpreterStackMap(ICorJitInfo* jitInfo, CORINFO_CLASS_ InterpreterStackMap* result = nullptr; if (!t_sharedStackMapLookup) t_sharedStackMapLookup = dn_simdhash_ptr_ptr_new(0, nullptr); + if (!t_sharedStackMapLookup) + NOMEM(); + if (!dn_simdhash_ptr_ptr_try_get_value(t_sharedStackMapLookup, classHandle, (void **)&result)) { result = new InterpreterStackMap(jitInfo, classHandle); - dn_simdhash_ptr_ptr_try_add(t_sharedStackMapLookup, classHandle, result); + checkAddedNew(dn_simdhash_ptr_ptr_try_add(t_sharedStackMapLookup, classHandle, result)); } return result; } diff --git a/src/native/containers/dn-simdhash-ght-compatible.c b/src/native/containers/dn-simdhash-ght-compatible.c index 41b6f3d5403430..1a3cd566f55c1e 100644 --- a/src/native/containers/dn-simdhash-ght-compatible.c +++ b/src/native/containers/dn-simdhash-ght-compatible.c @@ -69,9 +69,9 @@ dn_simdhash_ght_replaced (dn_simdhash_ght_data data, void * old_key, void * new_ 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; + // 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 @@ -87,6 +87,8 @@ dn_simdhash_ght_new ( ) { dn_simdhash_ght_t *hash = dn_simdhash_new_internal(&DN_SIMDHASH_T_META, DN_SIMDHASH_T_VTABLE, capacity, allocator); + if (!hash) + return NULL; // 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. @@ -107,6 +109,8 @@ 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) + return NULL; if (!hash_func) hash_func = dn_simdhash_ght_default_hash; if (!key_equal_func) @@ -133,7 +137,14 @@ dn_simdhash_ght_insert_replace ( 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); + uint8_t grow_ok; + dn_simdhash_buffers_t old_buffers = dn_simdhash_ensure_capacity_internal(hash, dn_simdhash_capacity(hash) + 1, &grow_ok); + // The ghashtable API doesn't have a way to signal failure, so just assert that we didn't fail to grow + dn_simdhash_assert(grow_ok); + // Don't attempt to insert after a failed grow (it's possible to get here because dn_simdhash_assert is configurable) + if (!grow_ok) + return; + if (old_buffers.buckets) { DN_SIMDHASH_REHASH_INTERNAL(hash, old_buffers); dn_simdhash_free_buffers(old_buffers); @@ -151,11 +162,51 @@ dn_simdhash_ght_insert_replace ( case DN_SIMDHASH_INSERT_KEY_ALREADY_PRESENT: case DN_SIMDHASH_INSERT_NEED_TO_GROW: default: - assert(0); + dn_simdhash_assert(!"Internal error in dn_simdhash_ght_insert_replace"); return; } } +dn_simdhash_add_result +dn_simdhash_ght_try_insert ( + dn_simdhash_ght_t *hash, + void *key, void *value, + dn_simdhash_insert_mode mode +) +{ + check_self(hash); + uint32_t key_hash = DN_SIMDHASH_KEY_HASHER(DN_SIMDHASH_GET_DATA(hash), key); + + dn_simdhash_insert_result ok = DN_SIMDHASH_TRY_INSERT_INTERNAL(hash, key, key_hash, value, mode); + if (ok == DN_SIMDHASH_INSERT_NEED_TO_GROW) { + uint8_t grow_ok; + dn_simdhash_buffers_t old_buffers = dn_simdhash_ensure_capacity_internal(hash, dn_simdhash_capacity(hash) + 1, &grow_ok); + if (!grow_ok) + return DN_SIMDHASH_OUT_OF_MEMORY; + + 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, mode); + } + + switch (ok) { + case DN_SIMDHASH_INSERT_OK_ADDED_NEW: + hash->count++; + return DN_SIMDHASH_ADD_INSERTED; + case DN_SIMDHASH_INSERT_OK_OVERWROTE_EXISTING: + return DN_SIMDHASH_ADD_OVERWROTE; + case DN_SIMDHASH_INSERT_KEY_ALREADY_PRESENT: + return DN_SIMDHASH_ADD_FAILED; + + case DN_SIMDHASH_INSERT_NEED_TO_GROW: + default: + dn_simdhash_assert(!"Internal error in dn_simdhash_ght_insert_replace"); + return DN_SIMDHASH_INTERNAL_ERROR; + } +} + void * dn_simdhash_ght_get_value_or_default ( dn_simdhash_ght_t *hash, void * key diff --git a/src/native/containers/dn-simdhash-ght-compatible.h b/src/native/containers/dn-simdhash-ght-compatible.h index 76f25ae09946c1..c11147bd35c2b4 100644 --- a/src/native/containers/dn-simdhash-ght-compatible.h +++ b/src/native/containers/dn-simdhash-ght-compatible.h @@ -38,6 +38,13 @@ dn_simdhash_ght_get_value_or_default ( dn_simdhash_ght_t *hash, void * key ); +dn_simdhash_add_result +dn_simdhash_ght_try_insert ( + dn_simdhash_ght_t *hash, + void *key, void *value, + dn_simdhash_insert_mode mode +); + #ifdef __cplusplus } #endif // __cplusplus diff --git a/src/native/containers/dn-simdhash-specialization-declarations.h b/src/native/containers/dn-simdhash-specialization-declarations.h index 97dc6fe32d2310..bde49893337d29 100644 --- a/src/native/containers/dn-simdhash-specialization-declarations.h +++ b/src/native/containers/dn-simdhash-specialization-declarations.h @@ -52,10 +52,10 @@ DN_SIMDHASH_T_PTR DN_SIMDHASH_NEW (uint32_t capacity, dn_allocator_t *allocator); #endif -uint8_t +dn_simdhash_add_result DN_SIMDHASH_TRY_ADD (DN_SIMDHASH_T_PTR hash, DN_SIMDHASH_KEY_T key, DN_SIMDHASH_VALUE_T value); -uint8_t +dn_simdhash_add_result DN_SIMDHASH_TRY_ADD_WITH_HASH (DN_SIMDHASH_T_PTR hash, DN_SIMDHASH_KEY_T key, uint32_t key_hash, DN_SIMDHASH_VALUE_T value); // result is an optional parameter that will be set to the value of the item if it was found. diff --git a/src/native/containers/dn-simdhash-specialization.h b/src/native/containers/dn-simdhash-specialization.h index 7614a690f5c080..408a70a713ee26 100644 --- a/src/native/containers/dn-simdhash-specialization.h +++ b/src/native/containers/dn-simdhash-specialization.h @@ -305,17 +305,6 @@ 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, @@ -444,7 +433,7 @@ DN_SIMDHASH_NEW (uint32_t capacity, dn_allocator_t *allocator) } #endif -uint8_t +dn_simdhash_add_result DN_SIMDHASH_TRY_ADD (DN_SIMDHASH_T_PTR hash, DN_SIMDHASH_KEY_T key, DN_SIMDHASH_VALUE_T value) { check_self(hash); @@ -453,14 +442,18 @@ DN_SIMDHASH_TRY_ADD (DN_SIMDHASH_T_PTR hash, DN_SIMDHASH_KEY_T key, DN_SIMDHASH_ return DN_SIMDHASH_TRY_ADD_WITH_HASH(hash, key, key_hash, value); } -uint8_t +dn_simdhash_add_result DN_SIMDHASH_TRY_ADD_WITH_HASH (DN_SIMDHASH_T_PTR hash, DN_SIMDHASH_KEY_T key, uint32_t key_hash, DN_SIMDHASH_VALUE_T value) { check_self(hash); 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); + uint8_t grow_ok; + dn_simdhash_buffers_t old_buffers = dn_simdhash_ensure_capacity_internal(hash, dn_simdhash_capacity(hash) + 1, &grow_ok); + if (!grow_ok) + return DN_SIMDHASH_OUT_OF_MEMORY; + if (old_buffers.buckets) { DN_SIMDHASH_REHASH_INTERNAL(hash, old_buffers); dn_simdhash_free_buffers(old_buffers); @@ -471,18 +464,19 @@ DN_SIMDHASH_TRY_ADD_WITH_HASH (DN_SIMDHASH_T_PTR hash, DN_SIMDHASH_KEY_T key, ui switch (ok) { case DN_SIMDHASH_INSERT_OK_ADDED_NEW: hash->count++; - return 1; + return DN_SIMDHASH_ADD_INSERTED; case DN_SIMDHASH_INSERT_OK_OVERWROTE_EXISTING: // This shouldn't happen dn_simdhash_assert(!"Overwrote an existing item while adding"); - return 1; + return DN_SIMDHASH_INTERNAL_ERROR; case DN_SIMDHASH_INSERT_KEY_ALREADY_PRESENT: - return 0; + return DN_SIMDHASH_ADD_FAILED; case DN_SIMDHASH_INSERT_NEED_TO_GROW: - // We should always have enough space after growing once. + dn_simdhash_assert(!"After growing there was no space for a new item"); + return DN_SIMDHASH_INTERNAL_ERROR; default: dn_simdhash_assert(!"Failed to add a new item but there was no existing item"); - return 0; + return DN_SIMDHASH_INTERNAL_ERROR; } } diff --git a/src/native/containers/dn-simdhash-specializations.h b/src/native/containers/dn-simdhash-specializations.h index 9533edfc5f3d1f..e899b9ed38270e 100644 --- a/src/native/containers/dn-simdhash-specializations.h +++ b/src/native/containers/dn-simdhash-specializations.h @@ -61,7 +61,7 @@ typedef struct dn_simdhash_str_key dn_simdhash_str_key; typedef struct dn_ptrpair_t { - void *first, *second; + void *first, *second; } dn_ptrpair_t; #define DN_SIMDHASH_T dn_simdhash_ptrpair_ptr diff --git a/src/native/containers/dn-simdhash-string-ptr.c b/src/native/containers/dn-simdhash-string-ptr.c index c3a93ceac9cdba..d917f3386c440e 100644 --- a/src/native/containers/dn-simdhash-string-ptr.c +++ b/src/native/containers/dn-simdhash-string-ptr.c @@ -83,8 +83,8 @@ dn_simdhash_string_ptr_try_remove (dn_simdhash_string_ptr_t *hash, const char *k void dn_simdhash_string_ptr_foreach (dn_simdhash_string_ptr_t *hash, dn_simdhash_string_ptr_foreach_func func, void *user_data) { - assert(hash); - assert(func); + dn_simdhash_assert(hash); + dn_simdhash_assert(func); dn_simdhash_buffers_t buffers = hash->buffers; BEGIN_SCAN_PAIRS(buffers, key_address, value_address) diff --git a/src/native/containers/dn-simdhash.c b/src/native/containers/dn-simdhash.c index 24ea919f8ae365..f29d42b81e9ca8 100644 --- a/src/native/containers/dn-simdhash.c +++ b/src/native/containers/dn-simdhash.c @@ -167,7 +167,9 @@ dn_simdhash_new_internal (dn_simdhash_meta_t *meta, dn_simdhash_vtable_t vtable, { const size_t size = sizeof(dn_simdhash_t) + meta->data_size; dn_simdhash_t *result = (dn_simdhash_t *)dn_allocator_alloc(allocator, size); - dn_simdhash_assert(result); + if (!result) + return NULL; + memset(result, 0, size); dn_simdhash_assert(meta); @@ -178,7 +180,12 @@ dn_simdhash_new_internal (dn_simdhash_meta_t *meta, dn_simdhash_vtable_t vtable, result->vtable = vtable; result->buffers.allocator = allocator; - dn_simdhash_ensure_capacity_internal(result, compute_adjusted_capacity(capacity)); + uint8_t alloc_ok; + dn_simdhash_ensure_capacity_internal(result, compute_adjusted_capacity(capacity), &alloc_ok); + if (!alloc_ok) { + dn_allocator_free(allocator, result); + return NULL; + } return result; } @@ -205,9 +212,12 @@ dn_simdhash_free_buffers (dn_simdhash_buffers_t buffers) } dn_simdhash_buffers_t -dn_simdhash_ensure_capacity_internal (dn_simdhash_t *hash, uint32_t capacity) +dn_simdhash_ensure_capacity_internal (dn_simdhash_t *hash, uint32_t capacity, uint8_t *ok) { dn_simdhash_assert(hash); + dn_simdhash_assert(ok); + *ok = 0; + size_t bucket_count = (capacity + hash->meta->bucket_capacity - 1) / hash->meta->bucket_capacity; // FIXME: Only apply this when capacity == 0? if (bucket_count < DN_SIMDHASH_MIN_BUCKET_COUNT) @@ -225,6 +235,8 @@ dn_simdhash_ensure_capacity_internal (dn_simdhash_t *hash, uint32_t capacity) dn_simdhash_buffers_t result = { 0, }; if (bucket_count <= hash->buffers.buckets_length) { dn_simdhash_assert(value_count <= hash->buffers.values_length); + // We didn't grow but we also didn't fail, so we set ok to 1. + *ok = 1; return result; } @@ -235,9 +247,26 @@ dn_simdhash_ensure_capacity_internal (dn_simdhash_t *hash, uint32_t capacity) capacity, value_count ); */ + + // pad buckets allocation by the width of one vector so we can align it + size_t buckets_size_bytes = (bucket_count * hash->meta->bucket_size_bytes) + DN_SIMDHASH_VECTOR_WIDTH, + values_size_bytes = value_count * hash->meta->value_size; + + // If either of these allocations fail all we can do is return a default-initialized buffers_t, which will + // result in the caller not freeing anything and seeing an ok of 0, so they can tell the grow failed. + // This should leave the hash in a well-formed state and if they try to grow later it might work. + void *new_buckets = dn_allocator_alloc(hash->buffers.allocator, buckets_size_bytes); + if (!new_buckets) + return result; + + void *new_values = dn_allocator_alloc(hash->buffers.allocator, values_size_bytes); + if (!new_values) { + dn_allocator_free(hash->buffers.allocator, new_buckets); + return result; + } + // Store old buffers so caller can rehash and then free them result = hash->buffers; - size_t grow_at_count = value_count; grow_at_count *= 100; grow_at_count /= DN_SIMDHASH_SIZING_PERCENTAGE; @@ -245,13 +274,6 @@ dn_simdhash_ensure_capacity_internal (dn_simdhash_t *hash, uint32_t capacity) hash->buffers.buckets_length = (uint32_t)bucket_count; hash->buffers.values_length = (uint32_t)value_count; - // pad buckets allocation by the width of one vector so we can align it - size_t buckets_size_bytes = (bucket_count * hash->meta->bucket_size_bytes) + DN_SIMDHASH_VECTOR_WIDTH, - values_size_bytes = value_count * hash->meta->value_size; - - void *new_buckets = dn_allocator_alloc(hash->buffers.allocator, buckets_size_bytes), - *new_values = dn_allocator_alloc(hash->buffers.allocator, values_size_bytes); - dn_simdhash_assert(new_buckets); dn_simdhash_assert(new_values); @@ -268,6 +290,8 @@ dn_simdhash_ensure_capacity_internal (dn_simdhash_t *hash, uint32_t capacity) // Skip this for performance; memset is especially slow in wasm // memset(hash->buffers.values, 0, values_size_bytes); + *ok = 1; + return result; } @@ -301,7 +325,7 @@ dn_simdhash_count (dn_simdhash_t *hash) uint32_t dn_simdhash_overflow_count (dn_simdhash_t *hash) { - assert(hash); + dn_simdhash_assert(hash); uint32_t result = 0; for (uint32_t bucket_index = 0; bucket_index < hash->buffers.buckets_length; bucket_index++) { uint8_t *suffixes = ((uint8_t *)hash->buffers.buckets) + (bucket_index * hash->meta->bucket_size_bytes); @@ -311,14 +335,16 @@ dn_simdhash_overflow_count (dn_simdhash_t *hash) return result; } -void +uint8_t dn_simdhash_ensure_capacity (dn_simdhash_t *hash, uint32_t capacity) { dn_simdhash_assert(hash); capacity = compute_adjusted_capacity(capacity); - dn_simdhash_buffers_t old_buffers = dn_simdhash_ensure_capacity_internal(hash, capacity); + uint8_t result; + dn_simdhash_buffers_t old_buffers = dn_simdhash_ensure_capacity_internal(hash, capacity, &result); if (old_buffers.buckets) { hash->vtable.rehash(hash, old_buffers); dn_simdhash_free_buffers(old_buffers); } + return result; } diff --git a/src/native/containers/dn-simdhash.h b/src/native/containers/dn-simdhash.h index af1afd8a910c7f..13bcb9ec828e4b 100644 --- a/src/native/containers/dn-simdhash.h +++ b/src/native/containers/dn-simdhash.h @@ -52,11 +52,32 @@ typedef struct dn_simdhash_t dn_simdhash_t; typedef struct dn_simdhash_meta_t { // type metadata for generic implementation + // NOTE: key_size and value_size are not used consistently by every part of the implementation, + // a specialization is still strongly typed based on its KEY_T and VALUE_T. But they need to match. uint32_t bucket_capacity, bucket_size_bytes, key_size, value_size, // Allocate this many bytes of extra data inside the dn_simdhash_t data_size; } dn_simdhash_meta_t; +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; + +typedef enum dn_simdhash_add_result { + DN_SIMDHASH_INTERNAL_ERROR = -2, + DN_SIMDHASH_OUT_OF_MEMORY = -1, + DN_SIMDHASH_ADD_FAILED = 0, + DN_SIMDHASH_ADD_INSERTED = 1, + DN_SIMDHASH_ADD_OVERWROTE = 2, +} dn_simdhash_add_result; + typedef enum dn_simdhash_insert_result { DN_SIMDHASH_INSERT_OK_ADDED_NEW, DN_SIMDHASH_INSERT_OK_OVERWROTE_EXISTING, @@ -139,8 +160,9 @@ dn_simdhash_free_buffers (dn_simdhash_buffers_t buffers); // If a resize happens, this will allocate new buffers and return the old ones. // It is your responsibility to rehash and then free the old buffers. +// If growing failed due to an out of memory condition *ok will be 0, otherwise it will be 1. dn_simdhash_buffers_t -dn_simdhash_ensure_capacity_internal (dn_simdhash_t *hash, uint32_t capacity); +dn_simdhash_ensure_capacity_internal (dn_simdhash_t *hash, uint32_t capacity, uint8_t *ok); // Erases the contents of the table, but does not shrink it. void @@ -161,8 +183,9 @@ uint32_t dn_simdhash_overflow_count (dn_simdhash_t *hash); // Automatically resizes the table if it is too small to hold the requested number -// of items. Will not shrink the table if it is already bigger. -void +// of items. Will not shrink the table if it is already bigger. Returns whether +// the operation was successful - 0 indicates allocation failure. +uint8_t dn_simdhash_ensure_capacity (dn_simdhash_t *hash, uint32_t capacity); #ifdef __cplusplus diff --git a/src/native/containers/simdhash-benchmark/run-benchmark.ps1 b/src/native/containers/simdhash-benchmark/run-benchmark.ps1 index 9417d16c505cbf..1a4221b9ad0b75 100755 --- a/src/native/containers/simdhash-benchmark/run-benchmark.ps1 +++ b/src/native/containers/simdhash-benchmark/run-benchmark.ps1 @@ -1,2 +1,2 @@ -cl /GS- /O2 /std:c17 ./*.c ../dn-simdhash-u32-ptr.c ../dn-simdhash.c ../dn-vector.c ../dn-simdhash-string-ptr.c /DNO_CONFIG_H /DSIZEOF_VOID_P=8 /DNDEBUG /Fe:all-measurements.exe +cl /GS- /O2 /std:c17 ./*.c ../dn-simdhash-u32-ptr.c ../dn-simdhash.c ../dn-vector.c ../dn-simdhash-string-ptr.c ../dn-simdhash-ght-compatible.c /DNO_CONFIG_H /DSIZEOF_VOID_P=8 /DNDEBUG /Fe:all-measurements.exe ./all-measurements.exe