From f632d7bf5c831bd7f020f794ab58ed4a3c71bf13 Mon Sep 17 00:00:00 2001 From: Jakub Gonet Date: Fri, 10 Jan 2025 17:35:42 +0100 Subject: [PATCH 01/17] ets: add duplicate_bag option Signed-off-by: Jakub Gonet --- libs/estdlib/src/ets.erl | 2 +- src/libAtomVM/defaultatoms.def | 6 +++ src/libAtomVM/nifs.c | 87 ++++++++++++++++++++++++++++----- tests/erlang_tests/test_ets.erl | 68 ++++++++++++++------------ 4 files changed, 118 insertions(+), 45 deletions(-) diff --git a/libs/estdlib/src/ets.erl b/libs/estdlib/src/ets.erl index 5c3144db3..9b608ff4f 100644 --- a/libs/estdlib/src/ets.erl +++ b/libs/estdlib/src/ets.erl @@ -43,7 +43,7 @@ ]). -opaque table() :: atom | reference(). --type table_type() :: set. +-type table_type() :: set | duplicate_bag. -type access_type() :: private | protected | public. -type option() :: table_type() | {keypos, non_neg_integer()} | access_type(). -type options() :: [option()]. diff --git a/src/libAtomVM/defaultatoms.def b/src/libAtomVM/defaultatoms.def index fa5150a07..e9276685f 100644 --- a/src/libAtomVM/defaultatoms.def +++ b/src/libAtomVM/defaultatoms.def @@ -188,3 +188,9 @@ X(BREAK_IGNORED_ATOM, "\xD", "break_ignored") X(SCOPE_ATOM, "\x5", "scope") X(NOMATCH_ATOM, "\x7", "nomatch") +X(NAMED_TABLE_ATOM, "\xB", "named_table") +X(KEYPOS_ATOM, "\x6", "keypos") +X(PRIVATE_ATOM, "\x7", "private") +X(PUBLIC_ATOM, "\x6", "public") +X(DUPLICATE_BAG_ATOM, "\xd", "duplicate_bag") +X(SET_ATOM, "\x3", "set") diff --git a/src/libAtomVM/nifs.c b/src/libAtomVM/nifs.c index 95968d2e7..a4981f2f1 100644 --- a/src/libAtomVM/nifs.c +++ b/src/libAtomVM/nifs.c @@ -3539,35 +3539,96 @@ static term nif_erlang_raise(Context *ctx, int argc, term argv[]) return term_invalid_term(); } +static inline term get_option_key(term option, term *maybe_value) +{ + *maybe_value = term_invalid_term(); + if (LIKELY(term_is_atom(option))) { + return option; + } + + bool is_tuple = term_is_tuple(option) && term_get_tuple_arity(option) == 2; + if (UNLIKELY(!is_tuple)) { + return term_invalid_term(); + } + term first = term_get_tuple_element(option, 0); + term second = term_get_tuple_element(option, 1); + if (UNLIKELY(!term_is_atom(first))) { + return term_invalid_term(); + } + + *maybe_value = second; + return first; +} + static term nif_ets_new(Context *ctx, int argc, term argv[]) { UNUSED(argc); - term name = argv[0]; - VALIDATE_VALUE(name, term_is_atom); - term options = argv[1]; + VALIDATE_VALUE(name, term_is_atom); VALIDATE_VALUE(options, term_is_list); - term is_named = interop_kv_get_value_default(options, ATOM_STR("\xB", "named_table"), FALSE_ATOM, ctx->global); - term keypos = interop_kv_get_value_default(options, ATOM_STR("\x6", "keypos"), term_from_int(1), ctx->global); + bool is_named = false; + bool private = false; + bool public = false; + bool is_duplicate_bag = false; + avm_int_t keypos = 1; - if (term_to_int(keypos) < 1) { - RAISE_ERROR(BADARG_ATOM); - } + while (term_is_nonempty_list(options)) { + term head = term_get_list_head(options); + term value; + term key_atom = get_option_key(head, &value); + VALIDATE_VALUE(key_atom, term_is_atom); - term private = interop_kv_get_value(options, ATOM_STR("\x7", "private"), ctx->global); - term public = interop_kv_get_value(options, ATOM_STR("\x6", "public"), ctx->global); + switch (key_atom) { + case NAMED_TABLE_ATOM: { + is_named = true; + break; + } + case PRIVATE_ATOM: { + private = true; + public = false; + break; + } + case PUBLIC_ATOM: { + private = false; + public = true; + break; + } + case SET_ATOM: { + is_duplicate_bag = false; + break; + } + case DUPLICATE_BAG_ATOM: { + is_duplicate_bag = true; + break; + } + case KEYPOS_ATOM: { + VALIDATE_VALUE(value, term_is_integer); + keypos = term_to_int(value); + if (UNLIKELY(keypos < 1)) { + RAISE_ERROR(BADARG_ATOM); + } + break; + } + default: + RAISE_ERROR(BADARG_ATOM); + } + options = term_get_list_tail(options); + } + VALIDATE_VALUE(options, term_is_nil); EtsAccessType access = EtsAccessProtected; - if (!term_is_invalid_term(private)) { + if (private) { access = EtsAccessPrivate; - } else if (!term_is_invalid_term(public)) { + } else if (public) { access = EtsAccessPublic; } + EtsTableType type = is_duplicate_bag ? EtsTableDuplicateBag : EtsTableSet; + term table = term_invalid_term(); - EtsErrorCode result = ets_create_table_maybe_gc(name, is_named == TRUE_ATOM, EtsTableSet, access, term_to_int(keypos) - 1, &table, ctx); + EtsErrorCode result = ets_create_table_maybe_gc(name, is_named, type, access, keypos - 1, &table, ctx); switch (result) { case EtsOk: return table; diff --git a/tests/erlang_tests/test_ets.erl b/tests/erlang_tests/test_ets.erl index 7909506dd..a975e04cc 100644 --- a/tests/erlang_tests/test_ets.erl +++ b/tests/erlang_tests/test_ets.erl @@ -49,20 +49,26 @@ test_ets_new() -> assert_badarg(fun() -> ets:new(keypos_test, [{keypos, -1}]) end), ets:new(type_test, [set]), - - % Unimplemented - ets:new(type_test, [ordered_set]), - ets:new(type_test, [bag]), ets:new(type_test, [duplicate_bag]), - ets:new(heir_test, [{heir, self(), []}]), - ets:new(heir_test, [{heir, none}]), - ets:new(write_conc_test, [{write_concurrency, true}]), - ets:new(read_conc_test, [{read_concurrency, true}]), - case otp_version() of - OTP when OTP >= 23 -> ets:new(decent_counters_test, [{decentralized_counters, true}]); - _ -> ok + + % Unimplemented in AtomVM + Options = [ + fun() -> ets:new(type_test, [ordered_set]) end, + fun() -> ets:new(type_test, [bag]) end, + fun() -> ets:new(heir_test, [{heir, self(), []}]) end, + fun() -> ets:new(heir_test, [{heir, none}]) end, + fun() -> ets:new(write_conc_test, [{write_concurrency, true}]) end, + fun() -> ets:new(read_conc_test, [{read_concurrency, true}]) end, + fun() -> ets:new(compressed_test, [compressed]) end + ], + Otp23Options = [ + fun() -> ets:new(decent_counters_test, [{decentralized_counters, true}]) end | Options + ], + case vm_version() of + atom -> [assert_badarg(NewF) || NewF <- Otp23Options]; + {otp, V} when V >= 23 -> [NewF() || NewF <- Otp23Options]; + {otp, _V} -> [NewF() || NewF <- Options] end, - ets:new(compressed_test, [compressed]), ok. test_permissions() -> @@ -290,32 +296,32 @@ isolated(Fun) -> end. assert_badarg(Fun) -> - try - Fun(), - erlang:error(no_throw) + try Fun() of + R -> erlang:error({no_throw, R}) catch error:badarg -> ok; - OtherClass:OtherError -> - erlang:error({OtherClass, OtherError}) + C:E -> + erlang:error({C, E}) end. supports_v4_port_encoding() -> + case vm_version() of + % small utf8 atom + atom -> true; + {otp, V} when V < 24 -> false; + % v4 is supported but not the default + {otp, V} when V < 26 -> true; + % small utf8 atom + {otp, _} -> true + end. + +vm_version() -> case erlang:system_info(machine) of "ATOM" -> - % small utf8 atom - true; + atom; "BEAM" -> - OTP = otp_version(), - if - OTP < 24 -> false; - % v4 is supported but not the default - OTP < 26 -> true; - % small utf8 atom - true -> true - end + OTPRelease = erlang:system_info(otp_release), + Version = list_to_integer(OTPRelease), + {otp, Version} end. - -otp_version() -> - OTPRelease = erlang:system_info(otp_release), - list_to_integer(OTPRelease). From 3700a10a80969e006267459555ef4032addf6307 Mon Sep 17 00:00:00 2001 From: Jakub Gonet Date: Mon, 16 Jun 2025 16:07:22 +0200 Subject: [PATCH 02/17] ets: no prefix for internal functions, add missing headers Signed-off-by: Jakub Gonet --- src/libAtomVM/ets.c | 11 ++++++----- src/libAtomVM/ets_hashtable.h | 2 +- src/libAtomVM/memory.h | 1 + 3 files changed, 8 insertions(+), 6 deletions(-) diff --git a/src/libAtomVM/ets.c b/src/libAtomVM/ets.c index 36f51e83b..5a0ff98ec 100644 --- a/src/libAtomVM/ets.c +++ b/src/libAtomVM/ets.c @@ -20,6 +20,7 @@ #include "ets.h" +#include #include "context.h" #include "defaultatoms.h" #include "ets_hashtable.h" @@ -245,7 +246,7 @@ static void ets_delete_all_tables(struct Ets *ets, GlobalContext *global) ets_delete_tables_internal(ets, true_pred, NULL, global); } -static EtsErrorCode ets_table_insert(struct EtsTable *ets_table, term entry, Context *ctx) +static EtsErrorCode insert(struct EtsTable *ets_table, term entry, Context *ctx) { size_t keypos = ets_table->keypos; @@ -266,7 +267,7 @@ static EtsErrorCode ets_table_insert(struct EtsTable *ets_table, term entry, Con return EtsOk; } -static EtsErrorCode ets_table_insert_list(struct EtsTable *ets_table, term list, Context *ctx) +static EtsErrorCode insert_multiple(struct EtsTable *ets_table, term list, Context *ctx) { term iter = list; size_t size = 0; @@ -321,9 +322,9 @@ EtsErrorCode ets_insert(term name_or_ref, term entry, Context *ctx) EtsErrorCode result; if (term_is_tuple(entry)) { - result = ets_table_insert(ets_table, entry, ctx); + result = insert(ets_table, entry, ctx); } else if (term_is_list(entry)) { - result = ets_table_insert_list(ets_table, entry, ctx); + result = insert_multiple(ets_table, entry, ctx); } else { result = EtsBadEntry; } @@ -559,7 +560,7 @@ EtsErrorCode ets_update_counter_maybe_gc(term ref, term key, term operation, ter term final_value = term_from_int(elem_value); term_put_tuple_element(to_insert, position, final_value); - EtsErrorCode insert_result = ets_table_insert(ets_table, to_insert, ctx); + EtsErrorCode insert_result = insert(ets_table, to_insert, ctx); if (insert_result == EtsOk) { *ret = final_value; } diff --git a/src/libAtomVM/ets_hashtable.h b/src/libAtomVM/ets_hashtable.h index cab87a6f6..de4ad75d7 100644 --- a/src/libAtomVM/ets_hashtable.h +++ b/src/libAtomVM/ets_hashtable.h @@ -39,7 +39,7 @@ struct EtsHashTable typedef enum EtsHashtableOptions { - EtsHashtableAllowOverwrite = 1 + EtsHashtableAllowOverwrite = (1 << 0), } EtsHashtableOptions; typedef enum EtsHashtableStatus diff --git a/src/libAtomVM/memory.h b/src/libAtomVM/memory.h index e53ef5c9d..a6edd7b23 100644 --- a/src/libAtomVM/memory.h +++ b/src/libAtomVM/memory.h @@ -23,6 +23,7 @@ // #define DEBUG_HEAP_ALLOC +#include #include #include #ifdef DEBUG_HEAP_ALLOC From fbcf57ff8ceb3ce71620cdde0267b3a0f93287c3 Mon Sep 17 00:00:00 2001 From: Jakub Gonet Date: Mon, 16 Jun 2025 17:47:30 +0200 Subject: [PATCH 03/17] term: improve docs Signed-off-by: Jakub Gonet --- src/libAtomVM/term.h | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/libAtomVM/term.h b/src/libAtomVM/term.h index 6b766d2df..fc3a24d92 100644 --- a/src/libAtomVM/term.h +++ b/src/libAtomVM/term.h @@ -1673,7 +1673,7 @@ static inline term term_alloc_tuple(uint32_t size, Heap *heap) /** * @brief Replaces the content of a tuple element. * - * @details Destructively replaces the nth element of an existing tuple, it should be used only on newly allocated tuples. + * @details Destructively replaces the nth (0-based) element of an existing tuple, it should be used only on newly allocated tuples. * @param t the term pointing to the target tuple, fails if not a tuple. * @param elem_index the index of the element that will be replaced. * @param put_value the term that will be put on the nth tuple element. @@ -1685,12 +1685,11 @@ static inline void term_put_tuple_element(term t, uint32_t elem_index, term put_ term *boxed_value = term_to_term_ptr(t); TERM_DEBUG_ASSERT((size_t) elem_index < term_get_size_from_boxed_header(boxed_value[0])); - boxed_value[elem_index + 1] = put_value; } /** - * @brief Returns the nth tuple element + * @brief Returns the nth (0-based) tuple element * * @details Returns the nth element for a given tuple pointed by a term. * @param t a term that points to a tuple, fails otherwise. @@ -1704,7 +1703,6 @@ static inline term term_get_tuple_element(term t, int elem_index) const term *boxed_value = term_to_const_term_ptr(t); TERM_DEBUG_ASSERT((size_t) elem_index < term_get_size_from_boxed_header(boxed_value[0])); - return boxed_value[elem_index + 1]; } From e87cc6af74acc2702efbd32446886437bd2e085c Mon Sep 17 00:00:00 2001 From: Jakub Gonet Date: Mon, 16 Jun 2025 17:56:45 +0200 Subject: [PATCH 04/17] ets: use index instead of position (NIFs) ETS mixes position (1-based) with indexing (0-based). Tuple are 0-indexed so we should convert to it at earliest possible moment. This commit doesn't introduce any change in behavior. Signed-off-by: Jakub Gonet --- src/libAtomVM/nifs.c | 25 +++++++++++++++---------- 1 file changed, 15 insertions(+), 10 deletions(-) diff --git a/src/libAtomVM/nifs.c b/src/libAtomVM/nifs.c index a4981f2f1..0f8280e4b 100644 --- a/src/libAtomVM/nifs.c +++ b/src/libAtomVM/nifs.c @@ -3572,7 +3572,7 @@ static term nif_ets_new(Context *ctx, int argc, term argv[]) bool private = false; bool public = false; bool is_duplicate_bag = false; - avm_int_t keypos = 1; + size_t key_index = 0; while (term_is_nonempty_list(options)) { term head = term_get_list_head(options); @@ -3605,10 +3605,11 @@ static term nif_ets_new(Context *ctx, int argc, term argv[]) } case KEYPOS_ATOM: { VALIDATE_VALUE(value, term_is_integer); - keypos = term_to_int(value); + avm_int_t keypos = term_to_int(value); if (UNLIKELY(keypos < 1)) { RAISE_ERROR(BADARG_ATOM); } + key_index = keypos - 1; break; } default: @@ -3628,7 +3629,7 @@ static term nif_ets_new(Context *ctx, int argc, term argv[]) EtsTableType type = is_duplicate_bag ? EtsTableDuplicateBag : EtsTableSet; term table = term_invalid_term(); - EtsErrorCode result = ets_create_table_maybe_gc(name, is_named, type, access, keypos - 1, &table, ctx); + EtsErrorCode result = ets_create_table_maybe_gc(name, is_named, type, access, key_index, &table, ctx); switch (result) { case EtsOk: return table; @@ -3699,12 +3700,17 @@ static term nif_ets_lookup_element(Context *ctx, int argc, term argv[]) term ref = argv[0]; VALIDATE_VALUE(ref, is_ets_table_id); - term key = argv[1]; - term pos = argv[2]; - VALIDATE_VALUE(pos, term_is_integer); + term key_term = argv[1]; + term keypos_term = argv[2]; + VALIDATE_VALUE(keypos_term, term_is_integer); + avm_int_t keypos = term_to_int(keypos_term); + if (UNLIKELY(keypos < 1)) { + RAISE_ERROR(BADARG_ATOM); + } + size_t key_index = term_to_int(keypos_term) - 1; term ret = term_invalid_term(); - EtsErrorCode result = ets_lookup_element_maybe_gc(ref, key, term_to_int(pos), &ret, ctx); + EtsErrorCode result = ets_lookup_element_maybe_gc(ref, key_term, key_index + 1, &ret, ctx); switch (result) { case EtsOk: return ret; @@ -3751,13 +3757,12 @@ static term nif_ets_update_counter(Context *ctx, int argc, term argv[]) term key = argv[1]; term operation = argv[2]; - term default_value; + term default_value = term_invalid_term(); if (argc == 4) { default_value = argv[3]; VALIDATE_VALUE(default_value, term_is_tuple); + // FIXME: this should be based on keypos in table term_put_tuple_element(default_value, 0, key); - } else { - default_value = term_invalid_term(); } term ret; EtsErrorCode result = ets_update_counter_maybe_gc(ref, key, operation, default_value, &ret, ctx); From bf77d3bd57840effd5ba0bb3a3f5b611e5d19b1b Mon Sep 17 00:00:00 2001 From: Jakub Gonet Date: Mon, 16 Jun 2025 18:48:39 +0200 Subject: [PATCH 05/17] ets: use 0-indexing for lookup/2 Signed-off-by: Jakub Gonet --- src/libAtomVM/ets.c | 21 ++++++++------------- src/libAtomVM/nifs.c | 4 ++-- 2 files changed, 10 insertions(+), 15 deletions(-) diff --git a/src/libAtomVM/ets.c b/src/libAtomVM/ets.c index 5a0ff98ec..adee9779e 100644 --- a/src/libAtomVM/ets.c +++ b/src/libAtomVM/ets.c @@ -20,7 +20,6 @@ #include "ets.h" -#include #include "context.h" #include "defaultatoms.h" #include "ets_hashtable.h" @@ -28,6 +27,7 @@ #include "memory.h" #include "overflow_helpers.h" #include "term.h" +#include #define ETS_NO_INDEX SIZE_MAX #define ETS_ANY_PROCESS -1 @@ -367,37 +367,32 @@ EtsErrorCode ets_lookup_maybe_gc(term name_or_ref, term key, term *ret, Context return result; } -EtsErrorCode ets_lookup_element_maybe_gc(term name_or_ref, term key, size_t pos, term *ret, Context *ctx) +EtsErrorCode ets_lookup_element_maybe_gc(term name_or_ref, term key, size_t key_index, term *ret, Context *ctx) { - if (UNLIKELY(pos == 0)) { - return EtsBadPosition; - } - struct EtsTable *ets_table = ets_acquire_table(&ctx->global->ets, ctx->process_id, name_or_ref, TableAccessRead); if (IS_NULL_PTR(ets_table)) { return EtsBadAccess; } term entry = ets_hashtable_lookup(ets_table->hashtable, key, ets_table->keypos, ctx->global); - - if (term_is_nil(entry)) { + if (!term_is_tuple(entry)) { SMP_UNLOCK(ets_table); return EtsEntryNotFound; } - if ((size_t) term_get_tuple_arity(entry) < pos) { + size_t arity = term_get_tuple_arity(entry); + if (key_index >= arity) { SMP_UNLOCK(ets_table); return EtsBadPosition; } - term res = term_get_tuple_element(entry, pos - 1); - size_t size = (size_t) memory_estimate_usage(res); - // allocate [object] + term t = term_get_tuple_element(entry, key_index); + size_t size = (size_t) memory_estimate_usage(t); if (UNLIKELY(memory_ensure_free_opt(ctx, size, MEMORY_CAN_SHRINK) != MEMORY_GC_OK)) { SMP_UNLOCK(ets_table); return EtsAllocationFailure; } - *ret = memory_copy_term_tree(&ctx->heap, res); + *ret = memory_copy_term_tree(&ctx->heap, t); SMP_UNLOCK(ets_table); return EtsOk; diff --git a/src/libAtomVM/nifs.c b/src/libAtomVM/nifs.c index 0f8280e4b..e9d50d8b7 100644 --- a/src/libAtomVM/nifs.c +++ b/src/libAtomVM/nifs.c @@ -3707,10 +3707,10 @@ static term nif_ets_lookup_element(Context *ctx, int argc, term argv[]) if (UNLIKELY(keypos < 1)) { RAISE_ERROR(BADARG_ATOM); } - size_t key_index = term_to_int(keypos_term) - 1; + size_t key_index = keypos - 1; term ret = term_invalid_term(); - EtsErrorCode result = ets_lookup_element_maybe_gc(ref, key_term, key_index + 1, &ret, ctx); + EtsErrorCode result = ets_lookup_element_maybe_gc(ref, key_term, key_index, &ret, ctx); switch (result) { case EtsOk: return ret; From d892b24bd7be073ecb5e1bee6afe4484c7b9408a Mon Sep 17 00:00:00 2001 From: Jakub Gonet Date: Mon, 16 Jun 2025 23:17:35 +0200 Subject: [PATCH 06/17] ets: use 0-indexing for update_counter/4 Signed-off-by: Jakub Gonet --- src/libAtomVM/ets.c | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/src/libAtomVM/ets.c b/src/libAtomVM/ets.c index adee9779e..e9935bbc1 100644 --- a/src/libAtomVM/ets.c +++ b/src/libAtomVM/ets.c @@ -524,14 +524,16 @@ EtsErrorCode ets_update_counter_maybe_gc(term ref, term key, term operation, ter SMP_UNLOCK(ets_table); return EtsBadEntry; } - int arity = term_get_tuple_arity(to_insert); - avm_int_t position = term_to_int(position_term) - 1; - if (UNLIKELY(arity <= position || position < 1)) { + size_t arity = term_get_tuple_arity(to_insert); + size_t elem_index = term_to_int(position_term) - 1; + // FIXME: elem_index can be 0 (i.e. first element). + // The correct condition is to error if it's the same as key position in the ETS table + if (UNLIKELY(arity <= elem_index || elem_index < 1)) { SMP_UNLOCK(ets_table); return EtsBadEntry; } - term elem = term_get_tuple_element(to_insert, position); + term elem = term_get_tuple_element(to_insert, elem_index); if (UNLIKELY(!term_is_integer(elem))) { SMP_UNLOCK(ets_table); return EtsBadEntry; @@ -554,7 +556,7 @@ EtsErrorCode ets_update_counter_maybe_gc(term ref, term key, term operation, ter } term final_value = term_from_int(elem_value); - term_put_tuple_element(to_insert, position, final_value); + term_put_tuple_element(to_insert, elem_index, final_value); EtsErrorCode insert_result = insert(ets_table, to_insert, ctx); if (insert_result == EtsOk) { *ret = final_value; From 64f2bb24bcb6a9c485d9920902fc0b0675b0a0de Mon Sep 17 00:00:00 2001 From: Jakub Gonet Date: Mon, 16 Jun 2025 23:21:51 +0200 Subject: [PATCH 07/17] ets: fix update_counter position check ETS supports setting different key position (default: first element in tuple) in ets:new/2. Checking if updated element shouldn't be placed as first element is wrong. Signed-off-by: Jakub Gonet --- src/libAtomVM/ets.c | 7 +++---- tests/erlang_tests/test_ets.erl | 7 +++++++ 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/src/libAtomVM/ets.c b/src/libAtomVM/ets.c index e9935bbc1..7fd52de84 100644 --- a/src/libAtomVM/ets.c +++ b/src/libAtomVM/ets.c @@ -27,6 +27,7 @@ #include "memory.h" #include "overflow_helpers.h" #include "term.h" +#include "utils.h" #include #define ETS_NO_INDEX SIZE_MAX @@ -517,7 +518,7 @@ EtsErrorCode ets_update_counter_maybe_gc(term ref, term key, term operation, ter term increment_term; term threshold_term; term set_value_term; - // +1 to position, +1 to elem after key + // +1 for index -> position, +1 for targeting elem after key size_t default_pos = (ets_table->keypos + 1) + 1; if (UNLIKELY(!operation_to_tuple4(operation, default_pos, &position_term, &increment_term, &threshold_term, &set_value_term))) { @@ -526,9 +527,7 @@ EtsErrorCode ets_update_counter_maybe_gc(term ref, term key, term operation, ter } size_t arity = term_get_tuple_arity(to_insert); size_t elem_index = term_to_int(position_term) - 1; - // FIXME: elem_index can be 0 (i.e. first element). - // The correct condition is to error if it's the same as key position in the ETS table - if (UNLIKELY(arity <= elem_index || elem_index < 1)) { + if (UNLIKELY(arity <= elem_index || elem_index == ets_table->keypos)) { SMP_UNLOCK(ets_table); return EtsBadEntry; } diff --git a/tests/erlang_tests/test_ets.erl b/tests/erlang_tests/test_ets.erl index a975e04cc..907f9e939 100644 --- a/tests/erlang_tests/test_ets.erl +++ b/tests/erlang_tests/test_ets.erl @@ -233,8 +233,14 @@ test_update_counter() -> 31 = ets:update_counter(T, key, {4, 10, 39, 31}), 30 = ets:update_counter(T, key, {4, -10, 30, 30}), + % {Position, Increment} with non-default position + T2 = ets:new(test, [{keypos, 2}]), + true = ets:insert(T2, {100, 200}), + 150 = ets:update_counter(T2, 200, {1, 50}), + TErr = ets:new(test, []), true = ets:insert(TErr, {key, 0, not_number}), + true = ets:insert(TErr, {0}), true = ets:insert(TErr, {not_number, ok}), assert_badarg(fun() -> ets:update_counter(TErr, none, 10) end), assert_badarg(fun() -> ets:update_counter(TErr, not_number, 10) end), @@ -243,6 +249,7 @@ test_update_counter() -> assert_badarg(fun() -> ets:update_counter(TErr, key, {0, 10}) end), assert_badarg(fun() -> ets:update_counter(TErr, key, {-1, 10}) end), assert_badarg(fun() -> ets:update_counter(TErr, key, {1, 10}) end), + assert_badarg(fun() -> ets:update_counter(TErr, 0, {1, 10}) end), assert_badarg(fun() -> ets:update_counter(TErr, key, {3, 10}) end), assert_badarg(fun() -> ets:update_counter(TErr, key, not_number) end), assert_badarg(fun() -> ets:update_counter(TErr, key, {not_number, 10}) end), From d7916406e1c2c6661ba69c5f94ed153964285eb5 Mon Sep 17 00:00:00 2001 From: Jakub Gonet Date: Mon, 16 Jun 2025 23:28:40 +0200 Subject: [PATCH 08/17] ets: rename ets_table->keypos to key_index Additionally, unifies tuple arity checks (no change in behavior). Signed-off-by: Jakub Gonet --- src/libAtomVM/ets.c | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/src/libAtomVM/ets.c b/src/libAtomVM/ets.c index 7fd52de84..637ae8352 100644 --- a/src/libAtomVM/ets.c +++ b/src/libAtomVM/ets.c @@ -57,7 +57,7 @@ struct EtsTable term name; bool is_named; int32_t owner_process_id; - size_t keypos; + size_t key_index; EtsTableType table_type; // In the future, we might support rb-trees for sorted sets // For this MVP, we only support unsorted sets @@ -171,7 +171,7 @@ EtsErrorCode ets_create_table_maybe_gc(term name, bool is_named, EtsTableType ta uint64_t ref_ticks = globalcontext_get_ref_ticks(ctx->global); ets_table->ref_ticks = ref_ticks; - ets_table->keypos = keypos; + ets_table->key_index = keypos; #ifndef AVM_NO_SMP ets_table->lock = smp_rwlock_create(); @@ -249,9 +249,9 @@ static void ets_delete_all_tables(struct Ets *ets, GlobalContext *global) static EtsErrorCode insert(struct EtsTable *ets_table, term entry, Context *ctx) { - size_t keypos = ets_table->keypos; + size_t keypos = ets_table->key_index; - if ((size_t) term_get_tuple_arity(entry) < keypos + 1) { + if ((size_t) term_get_tuple_arity(entry) <= keypos) { return EtsBadEntry; } @@ -276,7 +276,7 @@ static EtsErrorCode insert_multiple(struct EtsTable *ets_table, term list, Conte while (term_is_nonempty_list(iter)) { term tuple = term_get_list_head(iter); iter = term_get_list_tail(iter); - if (!term_is_tuple(tuple) || (size_t) term_get_tuple_arity(tuple) < (ets_table->keypos + 1)) { + if (!term_is_tuple(tuple) || (size_t) term_get_tuple_arity(tuple) <= ets_table->key_index) { return EtsBadEntry; } ++size; @@ -293,7 +293,7 @@ static EtsErrorCode insert_multiple(struct EtsTable *ets_table, term list, Conte size_t i = 0; while (term_is_nonempty_list(list)) { term tuple = term_get_list_head(list); - nodes[i] = ets_hashtable_new_node(tuple, ets_table->keypos); + nodes[i] = ets_hashtable_new_node(tuple, ets_table->key_index); if (IS_NULL_PTR(nodes[i])) { for (size_t it = 0; it < i; ++it) { ets_hashtable_free_node(nodes[it], ctx->global); @@ -337,7 +337,7 @@ EtsErrorCode ets_insert(term name_or_ref, term entry, Context *ctx) static EtsErrorCode ets_table_lookup_maybe_gc(struct EtsTable *ets_table, term key, term *ret, Context *ctx, int num_roots, term *roots) { - term res = ets_hashtable_lookup(ets_table->hashtable, key, ets_table->keypos, ctx->global); + term res = ets_hashtable_lookup(ets_table->hashtable, key, ets_table->key_index, ctx->global); if (term_is_nil(res)) { *ret = term_nil(); @@ -375,7 +375,7 @@ EtsErrorCode ets_lookup_element_maybe_gc(term name_or_ref, term key, size_t key_ return EtsBadAccess; } - term entry = ets_hashtable_lookup(ets_table->hashtable, key, ets_table->keypos, ctx->global); + term entry = ets_hashtable_lookup(ets_table->hashtable, key, ets_table->key_index, ctx->global); if (!term_is_tuple(entry)) { SMP_UNLOCK(ets_table); return EtsEntryNotFound; @@ -424,7 +424,7 @@ EtsErrorCode ets_delete(term name_or_ref, term key, term *ret, Context *ctx) return EtsBadAccess; } - bool _found = ets_hashtable_remove(ets_table->hashtable, key, ets_table->keypos, ctx->global); + bool _found = ets_hashtable_remove(ets_table->hashtable, key, ets_table->key_index, ctx->global); UNUSED(_found); SMP_UNLOCK(ets_table); @@ -519,7 +519,7 @@ EtsErrorCode ets_update_counter_maybe_gc(term ref, term key, term operation, ter term threshold_term; term set_value_term; // +1 for index -> position, +1 for targeting elem after key - size_t default_pos = (ets_table->keypos + 1) + 1; + size_t default_pos = (ets_table->key_index + 1) + 1; if (UNLIKELY(!operation_to_tuple4(operation, default_pos, &position_term, &increment_term, &threshold_term, &set_value_term))) { SMP_UNLOCK(ets_table); @@ -527,7 +527,7 @@ EtsErrorCode ets_update_counter_maybe_gc(term ref, term key, term operation, ter } size_t arity = term_get_tuple_arity(to_insert); size_t elem_index = term_to_int(position_term) - 1; - if (UNLIKELY(arity <= elem_index || elem_index == ets_table->keypos)) { + if (UNLIKELY(arity <= elem_index || elem_index == ets_table->key_index)) { SMP_UNLOCK(ets_table); return EtsBadEntry; } From 72ea5b247a340f84fe39138f7d51599268d5594e Mon Sep 17 00:00:00 2001 From: Jakub Gonet Date: Mon, 16 Jun 2025 23:32:18 +0200 Subject: [PATCH 09/17] ets: rename params Signed-off-by: Jakub Gonet --- src/libAtomVM/ets_hashtable.c | 14 +++++++------- src/libAtomVM/ets_hashtable.h | 9 +++++---- 2 files changed, 12 insertions(+), 11 deletions(-) diff --git a/src/libAtomVM/ets_hashtable.c b/src/libAtomVM/ets_hashtable.c index d162d9c49..f9adfa0d9 100644 --- a/src/libAtomVM/ets_hashtable.c +++ b/src/libAtomVM/ets_hashtable.c @@ -87,7 +87,7 @@ static void print_info(struct EtsHashTable *hash_table) } #endif -struct HNode *ets_hashtable_new_node(term entry, int keypos) +struct HNode *ets_hashtable_new_node(term entry, int key_index) { struct HNode *new_node = malloc(sizeof(struct HNode)); if (IS_NULL_PTR(new_node)) { @@ -101,8 +101,8 @@ struct HNode *ets_hashtable_new_node(term entry, int keypos) term new_entry = memory_copy_term_tree(&new_node->heap, entry); assert(term_is_tuple(new_entry)); - assert(term_get_tuple_arity(new_entry) >= keypos); - term key = term_get_tuple_element(new_entry, keypos); + assert(term_get_tuple_arity(new_entry) >= key_index); + term key = term_get_tuple_element(new_entry, key_index); new_node->next = NULL; new_node->key = key; @@ -167,14 +167,14 @@ EtsHashtableStatus ets_hashtable_insert(struct EtsHashTable *hash_table, struct return EtsHashtableOk; } -term ets_hashtable_lookup(struct EtsHashTable *hash_table, term key, size_t keypos, GlobalContext *global) +term ets_hashtable_lookup(struct EtsHashTable *hash_table, term key, size_t key_index, GlobalContext *global) { uint32_t hash = hash_term(key, global); uint32_t index = hash % hash_table->capacity; const struct HNode *node = hash_table->buckets[index]; while (node) { - term key_to_compare = term_get_tuple_element(node->entry, keypos); + term key_to_compare = term_get_tuple_element(node->entry, key_index); if (term_compare(key, key_to_compare, TermCompareExact, global) == TermEquals) { return node->entry; } @@ -184,7 +184,7 @@ term ets_hashtable_lookup(struct EtsHashTable *hash_table, term key, size_t keyp return term_nil(); } -bool ets_hashtable_remove(struct EtsHashTable *hash_table, term key, size_t keypos, GlobalContext *global) +bool ets_hashtable_remove(struct EtsHashTable *hash_table, term key, size_t key_index, GlobalContext *global) { uint32_t hash = hash_term(key, global); uint32_t index = hash % hash_table->capacity; @@ -192,7 +192,7 @@ bool ets_hashtable_remove(struct EtsHashTable *hash_table, term key, size_t keyp struct HNode *node = hash_table->buckets[index]; struct HNode *prev_node = NULL; while (node) { - term key_to_compare = term_get_tuple_element(node->entry, keypos); + term key_to_compare = term_get_tuple_element(node->entry, key_index); if (term_compare(key, key_to_compare, TermCompareExact, global) == TermEquals) { struct HNode *next_node = node->next; ets_hashtable_free_node(node, global); diff --git a/src/libAtomVM/ets_hashtable.h b/src/libAtomVM/ets_hashtable.h index de4ad75d7..59626521d 100644 --- a/src/libAtomVM/ets_hashtable.h +++ b/src/libAtomVM/ets_hashtable.h @@ -52,12 +52,13 @@ typedef enum EtsHashtableStatus struct EtsHashTable *ets_hashtable_new(); void ets_hashtable_destroy(struct EtsHashTable *hash_table, GlobalContext *global); -EtsHashtableStatus ets_hashtable_insert(struct EtsHashTable *hash_table, struct HNode *new_node, EtsHashtableOptions opts, GlobalContext *global); -term ets_hashtable_lookup(struct EtsHashTable *hash_table, term key, size_t keypos, GlobalContext *global); -bool ets_hashtable_remove(struct EtsHashTable *hash_table, term key, size_t keypos, GlobalContext *global); -struct HNode *ets_hashtable_new_node(term entry, int keypos); +struct HNode *ets_hashtable_new_node(term entry, int key_index); void ets_hashtable_free_node(struct HNode *node, GlobalContext *global); +EtsHashtableStatus ets_hashtable_insert(struct EtsHashTable *hash_table, struct HNode *new_node, EtsHashtableOptions opts, GlobalContext *global); +term ets_hashtable_lookup(struct EtsHashTable *hash_table, term key, size_t key_index, GlobalContext *global); +bool ets_hashtable_remove(struct EtsHashTable *hash_table, term key, size_t key_index, GlobalContext *global); + #ifdef __cplusplus } #endif From c804e42db1685b1d31ed00bec1401178a2e27235 Mon Sep 17 00:00:00 2001 From: Jakub Gonet Date: Tue, 17 Jun 2025 12:00:41 +0200 Subject: [PATCH 10/17] ets: rename entry to tuple in insert Signed-off-by: Jakub Gonet --- src/libAtomVM/ets.c | 11 ++++++----- src/libAtomVM/ets_hashtable.c | 6 +++--- 2 files changed, 9 insertions(+), 8 deletions(-) diff --git a/src/libAtomVM/ets.c b/src/libAtomVM/ets.c index 637ae8352..e60f53bca 100644 --- a/src/libAtomVM/ets.c +++ b/src/libAtomVM/ets.c @@ -247,21 +247,22 @@ static void ets_delete_all_tables(struct Ets *ets, GlobalContext *global) ets_delete_tables_internal(ets, true_pred, NULL, global); } -static EtsErrorCode insert(struct EtsTable *ets_table, term entry, Context *ctx) +static EtsErrorCode insert(struct EtsTable *ets_table, term tuple, Context *ctx) { - size_t keypos = ets_table->key_index; - - if ((size_t) term_get_tuple_arity(entry) <= keypos) { + assert(term_is_tuple(tuple)); + size_t key_index = ets_table->key_index; + if ((size_t) term_get_tuple_arity(tuple) <= key_index) { return EtsBadEntry; } - struct HNode *new_node = ets_hashtable_new_node(entry, keypos); + struct HNode *new_node = ets_hashtable_new_node(tuple, key_index); if (IS_NULL_PTR(new_node)) { return EtsAllocationFailure; } EtsHashtableStatus res = ets_hashtable_insert(ets_table->hashtable, new_node, EtsHashtableAllowOverwrite, ctx->global); if (UNLIKELY(res != EtsHashtableOk)) { + ets_hashtable_free_node(new_node, ctx->global); return EtsAllocationFailure; } diff --git a/src/libAtomVM/ets_hashtable.c b/src/libAtomVM/ets_hashtable.c index f9adfa0d9..b69d36b01 100644 --- a/src/libAtomVM/ets_hashtable.c +++ b/src/libAtomVM/ets_hashtable.c @@ -87,19 +87,19 @@ static void print_info(struct EtsHashTable *hash_table) } #endif -struct HNode *ets_hashtable_new_node(term entry, int key_index) +struct HNode *ets_hashtable_new_node(term tuple, int key_index) { struct HNode *new_node = malloc(sizeof(struct HNode)); if (IS_NULL_PTR(new_node)) { goto cleanup; } - size_t size = memory_estimate_usage(entry); + size_t size = memory_estimate_usage(tuple); if (UNLIKELY(memory_init_heap(&new_node->heap, size) != MEMORY_GC_OK)) { goto cleanup; } - term new_entry = memory_copy_term_tree(&new_node->heap, entry); + term new_entry = memory_copy_term_tree(&new_node->heap, tuple); assert(term_is_tuple(new_entry)); assert(term_get_tuple_arity(new_entry) >= key_index); term key = term_get_tuple_element(new_entry, key_index); From daaabc2629b61f4a50400c9cb807235b1bd16920 Mon Sep 17 00:00:00 2001 From: Jakub Gonet Date: Tue, 17 Jun 2025 13:18:26 +0200 Subject: [PATCH 11/17] ets: rename internal lookup function Signed-off-by: Jakub Gonet --- src/libAtomVM/ets.c | 30 ++++++++++++++++------ src/libAtomVM/ets_hashtable.c | 38 ++++++++++++++++++++++++++-- src/libAtomVM/ets_hashtable.h | 1 + tests/erlang_tests/test_ets.erl | 44 +++++++++++++++++++++++++++++++++ 4 files changed, 103 insertions(+), 10 deletions(-) diff --git a/src/libAtomVM/ets.c b/src/libAtomVM/ets.c index e60f53bca..bbf03ce23 100644 --- a/src/libAtomVM/ets.c +++ b/src/libAtomVM/ets.c @@ -247,22 +247,36 @@ static void ets_delete_all_tables(struct Ets *ets, GlobalContext *global) ets_delete_tables_internal(ets, true_pred, NULL, global); } +static inline bool has_key_at(term tuple, size_t key_index) +{ + return key_index < (size_t) term_get_tuple_arity(tuple); +} + static EtsErrorCode insert(struct EtsTable *ets_table, term tuple, Context *ctx) { assert(term_is_tuple(tuple)); size_t key_index = ets_table->key_index; - if ((size_t) term_get_tuple_arity(tuple) <= key_index) { + if (UNLIKELY(!has_key_at(tuple, key_index))) { return EtsBadEntry; } - struct HNode *new_node = ets_hashtable_new_node(tuple, key_index); - if (IS_NULL_PTR(new_node)) { + bool is_duplicate_bag = ets_table->table_type == EtsTableDuplicateBag; + struct HNode *node = NULL; + if (is_duplicate_bag) { + term key = term_get_tuple_element(tuple, key_index); + term old_tuples = ets_hashtable_lookup(ets_table->hashtable, key, key_index, ctx->global); + bool is_new = term_is_nil(old_tuples); + node = ets_hashtable_new_node_from_list(is_new ? term_nil() : old_tuples, tuple, key_index); + } else { + node = ets_hashtable_new_node(tuple, key_index); + } + if (IS_NULL_PTR(node)) { return EtsAllocationFailure; } - EtsHashtableStatus res = ets_hashtable_insert(ets_table->hashtable, new_node, EtsHashtableAllowOverwrite, ctx->global); + EtsHashtableStatus res = ets_hashtable_insert(ets_table->hashtable, node, EtsHashtableAllowOverwrite, ctx->global); if (UNLIKELY(res != EtsHashtableOk)) { - ets_hashtable_free_node(new_node, ctx->global); + ets_hashtable_free_node(node, ctx->global); return EtsAllocationFailure; } @@ -336,7 +350,7 @@ EtsErrorCode ets_insert(term name_or_ref, term entry, Context *ctx) return result; } -static EtsErrorCode ets_table_lookup_maybe_gc(struct EtsTable *ets_table, term key, term *ret, Context *ctx, int num_roots, term *roots) +static EtsErrorCode lookup_maybe_gc(struct EtsTable *ets_table, term key, term *ret, Context *ctx, int num_roots, term *roots) { term res = ets_hashtable_lookup(ets_table->hashtable, key, ets_table->key_index, ctx->global); @@ -363,7 +377,7 @@ EtsErrorCode ets_lookup_maybe_gc(term name_or_ref, term key, term *ret, Context return EtsBadAccess; } - EtsErrorCode result = ets_table_lookup_maybe_gc(ets_table, key, ret, ctx, 0, NULL); + EtsErrorCode result = lookup_maybe_gc(ets_table, key, ret, ctx, 0, NULL); SMP_UNLOCK(ets_table); return result; @@ -490,7 +504,7 @@ EtsErrorCode ets_update_counter_maybe_gc(term ref, term key, term operation, ter term roots[] = { key, operation, safe_default_value }; term list; - EtsErrorCode result = ets_table_lookup_maybe_gc(ets_table, key, &list, ctx, 3, roots); + EtsErrorCode result = lookup_maybe_gc(ets_table, key, &list, ctx, 3, roots); if (UNLIKELY(result != EtsOk)) { SMP_UNLOCK(ets_table); return result; diff --git a/src/libAtomVM/ets_hashtable.c b/src/libAtomVM/ets_hashtable.c index b69d36b01..04c30173b 100644 --- a/src/libAtomVM/ets_hashtable.c +++ b/src/libAtomVM/ets_hashtable.c @@ -20,6 +20,7 @@ #include "ets_hashtable.h" +#include "memory.h" #include "smp.h" #include "term.h" #include "utils.h" @@ -89,6 +90,8 @@ static void print_info(struct EtsHashTable *hash_table) struct HNode *ets_hashtable_new_node(term tuple, int key_index) { + assert(term_is_tuple(tuple)); + assert(term_get_tuple_arity(tuple) >= key_index); struct HNode *new_node = malloc(sizeof(struct HNode)); if (IS_NULL_PTR(new_node)) { goto cleanup; @@ -100,8 +103,6 @@ struct HNode *ets_hashtable_new_node(term tuple, int key_index) } term new_entry = memory_copy_term_tree(&new_node->heap, tuple); - assert(term_is_tuple(new_entry)); - assert(term_get_tuple_arity(new_entry) >= key_index); term key = term_get_tuple_element(new_entry, key_index); new_node->next = NULL; @@ -115,6 +116,39 @@ struct HNode *ets_hashtable_new_node(term tuple, int key_index) return NULL; } +// TODO: create list elsewhere, by copying terms from orig heap, appending new copied tuple and using ets_hashtable_new_node +struct HNode *ets_hashtable_new_node_from_list(term old_tuples, term tuple, size_t key_index) +{ + assert(term_is_tuple(tuple)); + assert((size_t) term_get_tuple_arity(tuple) >= key_index); + assert(term_is_list(old_tuples)); + + struct HNode *new_node = malloc(sizeof(struct HNode)); + if (IS_NULL_PTR(new_node)) { + goto oom; + } + + size_t old_list_size = memory_estimate_usage(old_tuples); + size_t new_tuple_size = memory_estimate_usage(tuple); + if (UNLIKELY(memory_init_heap(&new_node->heap, old_list_size + new_tuple_size + CONS_SIZE) != MEMORY_GC_OK)) { + goto oom; + } + term ets_list = memory_copy_term_tree(&new_node->heap, old_tuples); + term ets_tuple = memory_copy_term_tree(&new_node->heap, tuple); + + term new_key = term_get_tuple_element(ets_tuple, key_index); + ets_list = term_list_prepend(ets_tuple, ets_list, &new_node->heap); + + new_node->next = NULL; + new_node->key = new_key; + new_node->entry = ets_list; + return new_node; + +oom: + free(new_node); + return NULL; +} + EtsHashtableStatus ets_hashtable_insert(struct EtsHashTable *hash_table, struct HNode *new_node, EtsHashtableOptions opts, GlobalContext *global) { term key = new_node->key; diff --git a/src/libAtomVM/ets_hashtable.h b/src/libAtomVM/ets_hashtable.h index 59626521d..34f63fe8c 100644 --- a/src/libAtomVM/ets_hashtable.h +++ b/src/libAtomVM/ets_hashtable.h @@ -53,6 +53,7 @@ struct EtsHashTable *ets_hashtable_new(); void ets_hashtable_destroy(struct EtsHashTable *hash_table, GlobalContext *global); struct HNode *ets_hashtable_new_node(term entry, int key_index); +struct HNode *ets_hashtable_new_node_from_list(term old_tuples_or_tuple, term new_tuple, size_t key_index); void ets_hashtable_free_node(struct HNode *node, GlobalContext *global); EtsHashtableStatus ets_hashtable_insert(struct EtsHashTable *hash_table, struct HNode *new_node, EtsHashtableOptions opts, GlobalContext *global); diff --git a/tests/erlang_tests/test_ets.erl b/tests/erlang_tests/test_ets.erl index 907f9e939..8e36b8285 100644 --- a/tests/erlang_tests/test_ets.erl +++ b/tests/erlang_tests/test_ets.erl @@ -31,6 +31,7 @@ start() -> ok = isolated(fun test_delete/0), ok = isolated(fun test_lookup_element/0), ok = isolated(fun test_update_counter/0), + ok = isolated(fun test_duplicate_bag/0), 0. test_ets_new() -> @@ -260,6 +261,49 @@ test_update_counter() -> assert_badarg(fun() -> ets:update_counter(TErr, key, {2, 10, 100, not_number}) end), ok. +test_duplicate_bag() -> + Tid = ets:new(test_duplicate_bag, [duplicate_bag, {keypos, 2}]), + T = {ok, foo, 100, extra}, + T2 = {error, foo, 200}, + T3 = {error, foo, 300}, + + % true = ets:insert_new(Tid, T), + % false = ets:insert_new(Tid, T), + true = ets:insert(Tid, T), + true = ets:insert(Tid, [T, T]), + true = ets:insert(Tid, [T2]), + % true = [T, T, T, T2] == ets:lookup(Tid, foo), + % true = [T, T, T, T, T2] == ets:lookup(Tid, foo), + % true = ets:member(Tid, foo), + + % % nothing inserted, T exists in table + % false = ets:insert_new(Tid, [T, {ok, bar, batat}]), + % false = ets:member(Tid, bar), + + % [ok, ok, ok, ok, error] = ets:lookup_element(Tid, foo, 1), + % [foo, foo, foo, foo, foo] = ets:lookup_element(Tid, foo, 2), + % [100, 100, 100, 100, 200] = ets:lookup_element(Tid, foo, 3), + % % some tuples don't have 4 arity + % ok = expect_failure(fun() -> ets:lookup_element(Tid, foo, 4) end), + + % % unsupported for duplicate bag + % ok = expect_failure(fun() -> ets:update_counter(Tid, foo, 10) end), + % ok = expect_failure(fun() -> ets:update_element(Tid, foo, {1, error}) end), + + % true = ets:delete_object(Tid, {bad, bad}), + % true = [T, T, T, T, T2] == ets:lookup(Tid, foo), + % true = ets:delete_object(Tid, T), + % true = [T2] == ets:lookup(Tid, foo), + + % true = ets:insert(Tid, T3), + % % keeps insertion order + % true = [T2, T3] == ets:take(Tid, foo), + + % true = ets:delete(Tid), + % ok = expect_failure(fun() -> ets:insert(Tid, T) end), + + ok. + %%----------------------------------------------------------------------------- %% @doc Performs specified operation on ETS table implicitly asserting that no exception is raised. %% [badarg] can be passed as an option to assert that exception was raised. From 492beb1963a0b3e692df88e23cc208a74f92a0fa Mon Sep 17 00:00:00 2001 From: Jakub Gonet Date: Tue, 17 Jun 2025 13:22:57 +0200 Subject: [PATCH 12/17] ets: use has_key_at helper everywhere Signed-off-by: Jakub Gonet --- src/libAtomVM/ets.c | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/src/libAtomVM/ets.c b/src/libAtomVM/ets.c index bbf03ce23..09d41585e 100644 --- a/src/libAtomVM/ets.c +++ b/src/libAtomVM/ets.c @@ -291,7 +291,7 @@ static EtsErrorCode insert_multiple(struct EtsTable *ets_table, term list, Conte while (term_is_nonempty_list(iter)) { term tuple = term_get_list_head(iter); iter = term_get_list_tail(iter); - if (!term_is_tuple(tuple) || (size_t) term_get_tuple_arity(tuple) <= ets_table->key_index) { + if (UNLIKELY(!term_is_tuple(tuple) || !has_key_at(tuple, ets_table->key_index))) { return EtsBadEntry; } ++size; @@ -391,13 +391,12 @@ EtsErrorCode ets_lookup_element_maybe_gc(term name_or_ref, term key, size_t key_ } term entry = ets_hashtable_lookup(ets_table->hashtable, key, ets_table->key_index, ctx->global); - if (!term_is_tuple(entry)) { + if (UNLIKELY(!term_is_tuple(entry))) { SMP_UNLOCK(ets_table); return EtsEntryNotFound; } - size_t arity = term_get_tuple_arity(entry); - if (key_index >= arity) { + if (UNLIKELY(!has_key_at(entry, key_index))) { SMP_UNLOCK(ets_table); return EtsBadPosition; } @@ -540,9 +539,8 @@ EtsErrorCode ets_update_counter_maybe_gc(term ref, term key, term operation, ter SMP_UNLOCK(ets_table); return EtsBadEntry; } - size_t arity = term_get_tuple_arity(to_insert); size_t elem_index = term_to_int(position_term) - 1; - if (UNLIKELY(arity <= elem_index || elem_index == ets_table->key_index)) { + if (UNLIKELY(!has_key_at(to_insert, elem_index) || elem_index == ets_table->key_index)) { SMP_UNLOCK(ets_table); return EtsBadEntry; } From 684d4b8fb4295a93f72f505b46bee25f472f6617 Mon Sep 17 00:00:00 2001 From: Jakub Gonet Date: Tue, 17 Jun 2025 14:11:18 +0200 Subject: [PATCH 13/17] ets: support duplicate_bag in lookup/2 Signed-off-by: Jakub Gonet --- src/libAtomVM/ets.c | 79 ++++++++++++++++++++++----------- src/libAtomVM/ets_hashtable.c | 10 ++--- src/libAtomVM/ets_hashtable.h | 4 +- tests/erlang_tests/test_ets.erl | 24 +++++----- 4 files changed, 72 insertions(+), 45 deletions(-) diff --git a/src/libAtomVM/ets.c b/src/libAtomVM/ets.c index 09d41585e..69eb49b37 100644 --- a/src/libAtomVM/ets.c +++ b/src/libAtomVM/ets.c @@ -254,7 +254,6 @@ static inline bool has_key_at(term tuple, size_t key_index) static EtsErrorCode insert(struct EtsTable *ets_table, term tuple, Context *ctx) { - assert(term_is_tuple(tuple)); size_t key_index = ets_table->key_index; if (UNLIKELY(!has_key_at(tuple, key_index))) { return EtsBadEntry; @@ -264,7 +263,8 @@ static EtsErrorCode insert(struct EtsTable *ets_table, term tuple, Context *ctx) struct HNode *node = NULL; if (is_duplicate_bag) { term key = term_get_tuple_element(tuple, key_index); - term old_tuples = ets_hashtable_lookup(ets_table->hashtable, key, key_index, ctx->global); + term old_tuples = ets_hashtable_lookup(ets_table->hashtable, key, ctx->global); + assert(term_is_list(old_tuples)); bool is_new = term_is_nil(old_tuples); node = ets_hashtable_new_node_from_list(is_new ? term_nil() : old_tuples, tuple, key_index); } else { @@ -350,20 +350,53 @@ EtsErrorCode ets_insert(term name_or_ref, term entry, Context *ctx) return result; } -static EtsErrorCode lookup_maybe_gc(struct EtsTable *ets_table, term key, term *ret, Context *ctx, int num_roots, term *roots) +static EtsErrorCode lookup_maybe_gc(struct EtsTable *ets_table, term key, size_t index, term *ret, Context *ctx, int num_roots, term *roots) { - term res = ets_hashtable_lookup(ets_table->hashtable, key, ets_table->key_index, ctx->global); + bool is_duplicate_bag = ets_table->table_type == EtsTableDuplicateBag; + bool lookup_element = index != ETS_NO_INDEX; + term ets_entry = ets_hashtable_lookup(ets_table->hashtable, key, ctx->global); - if (term_is_nil(res)) { + if (term_is_nil(ets_entry)) { *ret = term_nil(); - } else { + } else if (is_duplicate_bag) { + assert(term_is_list(ets_entry)); + // for tuple list and it reversed version - we don't want to copy terms in the loop + int _proper; + size_t n = term_list_length(ets_entry, &_proper); + size_t size = LIST_SIZE(n, 1) + memory_estimate_usage(ets_entry); + if (UNLIKELY(memory_ensure_free_with_roots(ctx, size, num_roots, roots, MEMORY_CAN_SHRINK))) { + return EtsAllocationFailure; + } + term tuples = memory_copy_term_tree(&ctx->heap, ets_entry); + // lookup returns in insertion order + // TODO: store it in correct order? + term reversed = term_nil(); + while (term_is_nonempty_list(tuples)) { + term elem = term_get_list_head(tuples); + if (lookup_element) { + term tuple = elem; + if (UNLIKELY(!has_key_at(tuple, index))) { + return EtsBadPosition; + } + elem = term_get_tuple_element(tuple, index); + } + reversed = term_list_prepend(elem, reversed, &ctx->heap); + tuples = term_get_list_tail(tuples); + } - size_t size = (size_t) memory_estimate_usage(res); - // allocate [object] + *ret = reversed; + } else { + if (lookup_element) { + if (UNLIKELY(!has_key_at(ets_entry, index))) { + return EtsBadPosition; + } + ets_entry = term_get_tuple_element(ets_entry, index); + } + size_t size = (size_t) memory_estimate_usage(ets_entry); if (UNLIKELY(memory_ensure_free_with_roots(ctx, size + CONS_SIZE, num_roots, roots, MEMORY_CAN_SHRINK) != MEMORY_GC_OK)) { return EtsAllocationFailure; } - term new_res = memory_copy_term_tree(&ctx->heap, res); + term new_res = memory_copy_term_tree(&ctx->heap, ets_entry); *ret = term_list_prepend(new_res, term_nil(), &ctx->heap); } @@ -377,7 +410,7 @@ EtsErrorCode ets_lookup_maybe_gc(term name_or_ref, term key, term *ret, Context return EtsBadAccess; } - EtsErrorCode result = lookup_maybe_gc(ets_table, key, ret, ctx, 0, NULL); + EtsErrorCode result = lookup_maybe_gc(ets_table, key, ETS_NO_INDEX, ret, ctx, 0, NULL); SMP_UNLOCK(ets_table); return result; @@ -389,25 +422,19 @@ EtsErrorCode ets_lookup_element_maybe_gc(term name_or_ref, term key, size_t key_ if (IS_NULL_PTR(ets_table)) { return EtsBadAccess; } + bool is_duplicate_bag = ets_table->table_type == EtsTableDuplicateBag; - term entry = ets_hashtable_lookup(ets_table->hashtable, key, ets_table->key_index, ctx->global); - if (UNLIKELY(!term_is_tuple(entry))) { - SMP_UNLOCK(ets_table); - return EtsEntryNotFound; - } - - if (UNLIKELY(!has_key_at(entry, key_index))) { + term entry; + EtsErrorCode result = lookup_maybe_gc(ets_table, key, key_index, &entry, ctx, 0, NULL); + if (result != EtsOk) { SMP_UNLOCK(ets_table); - return EtsBadPosition; + return result; } - - term t = term_get_tuple_element(entry, key_index); - size_t size = (size_t) memory_estimate_usage(t); - if (UNLIKELY(memory_ensure_free_opt(ctx, size, MEMORY_CAN_SHRINK) != MEMORY_GC_OK)) { + if (term_is_nil(entry)) { SMP_UNLOCK(ets_table); - return EtsAllocationFailure; + return EtsEntryNotFound; } - *ret = memory_copy_term_tree(&ctx->heap, t); + *ret = is_duplicate_bag ? entry : term_get_list_head(entry); SMP_UNLOCK(ets_table); return EtsOk; @@ -438,7 +465,7 @@ EtsErrorCode ets_delete(term name_or_ref, term key, term *ret, Context *ctx) return EtsBadAccess; } - bool _found = ets_hashtable_remove(ets_table->hashtable, key, ets_table->key_index, ctx->global); + bool _found = ets_hashtable_remove(ets_table->hashtable, key, ctx->global); UNUSED(_found); SMP_UNLOCK(ets_table); @@ -503,7 +530,7 @@ EtsErrorCode ets_update_counter_maybe_gc(term ref, term key, term operation, ter term roots[] = { key, operation, safe_default_value }; term list; - EtsErrorCode result = lookup_maybe_gc(ets_table, key, &list, ctx, 3, roots); + EtsErrorCode result = lookup_maybe_gc(ets_table, key, ETS_NO_INDEX, &list, ctx, 3, roots); if (UNLIKELY(result != EtsOk)) { SMP_UNLOCK(ets_table); return result; diff --git a/src/libAtomVM/ets_hashtable.c b/src/libAtomVM/ets_hashtable.c index 04c30173b..519e33822 100644 --- a/src/libAtomVM/ets_hashtable.c +++ b/src/libAtomVM/ets_hashtable.c @@ -201,15 +201,14 @@ EtsHashtableStatus ets_hashtable_insert(struct EtsHashTable *hash_table, struct return EtsHashtableOk; } -term ets_hashtable_lookup(struct EtsHashTable *hash_table, term key, size_t key_index, GlobalContext *global) +term ets_hashtable_lookup(struct EtsHashTable *hash_table, term key, GlobalContext *global) { uint32_t hash = hash_term(key, global); uint32_t index = hash % hash_table->capacity; const struct HNode *node = hash_table->buckets[index]; while (node) { - term key_to_compare = term_get_tuple_element(node->entry, key_index); - if (term_compare(key, key_to_compare, TermCompareExact, global) == TermEquals) { + if (term_compare(key, node->key, TermCompareExact, global) == TermEquals) { return node->entry; } node = node->next; @@ -218,7 +217,7 @@ term ets_hashtable_lookup(struct EtsHashTable *hash_table, term key, size_t key_ return term_nil(); } -bool ets_hashtable_remove(struct EtsHashTable *hash_table, term key, size_t key_index, GlobalContext *global) +bool ets_hashtable_remove(struct EtsHashTable *hash_table, term key, GlobalContext *global) { uint32_t hash = hash_term(key, global); uint32_t index = hash % hash_table->capacity; @@ -226,8 +225,7 @@ bool ets_hashtable_remove(struct EtsHashTable *hash_table, term key, size_t key_ struct HNode *node = hash_table->buckets[index]; struct HNode *prev_node = NULL; while (node) { - term key_to_compare = term_get_tuple_element(node->entry, key_index); - if (term_compare(key, key_to_compare, TermCompareExact, global) == TermEquals) { + if (term_compare(key, node->key, TermCompareExact, global) == TermEquals) { struct HNode *next_node = node->next; ets_hashtable_free_node(node, global); if (prev_node != NULL) { diff --git a/src/libAtomVM/ets_hashtable.h b/src/libAtomVM/ets_hashtable.h index 34f63fe8c..52b8b2add 100644 --- a/src/libAtomVM/ets_hashtable.h +++ b/src/libAtomVM/ets_hashtable.h @@ -57,8 +57,8 @@ struct HNode *ets_hashtable_new_node_from_list(term old_tuples_or_tuple, term ne void ets_hashtable_free_node(struct HNode *node, GlobalContext *global); EtsHashtableStatus ets_hashtable_insert(struct EtsHashTable *hash_table, struct HNode *new_node, EtsHashtableOptions opts, GlobalContext *global); -term ets_hashtable_lookup(struct EtsHashTable *hash_table, term key, size_t key_index, GlobalContext *global); -bool ets_hashtable_remove(struct EtsHashTable *hash_table, term key, size_t key_index, GlobalContext *global); +term ets_hashtable_lookup(struct EtsHashTable *hash_table, term key, GlobalContext *global); +bool ets_hashtable_remove(struct EtsHashTable *hash_table, term key, GlobalContext *global); #ifdef __cplusplus } diff --git a/tests/erlang_tests/test_ets.erl b/tests/erlang_tests/test_ets.erl index 8e36b8285..f394ca507 100644 --- a/tests/erlang_tests/test_ets.erl +++ b/tests/erlang_tests/test_ets.erl @@ -23,14 +23,14 @@ -export([start/0]). start() -> - ok = isolated(fun test_ets_new/0), - ok = isolated(fun test_permissions/0), - ok = isolated(fun test_keys/0), - ok = isolated(fun test_keypos/0), - ok = isolated(fun test_insert/0), - ok = isolated(fun test_delete/0), - ok = isolated(fun test_lookup_element/0), - ok = isolated(fun test_update_counter/0), + % ok = isolated(fun test_ets_new/0), + % ok = isolated(fun test_permissions/0), + % ok = isolated(fun test_keys/0), + % ok = isolated(fun test_keypos/0), + % ok = isolated(fun test_insert/0), + % ok = isolated(fun test_delete/0), + % ok = isolated(fun test_lookup_element/0), + % ok = isolated(fun test_update_counter/0), ok = isolated(fun test_duplicate_bag/0), 0. @@ -270,9 +270,11 @@ test_duplicate_bag() -> % true = ets:insert_new(Tid, T), % false = ets:insert_new(Tid, T), true = ets:insert(Tid, T), - true = ets:insert(Tid, [T, T]), - true = ets:insert(Tid, [T2]), - % true = [T, T, T, T2] == ets:lookup(Tid, foo), + true = ets:insert(Tid, T), + true = [T, T] == ets:lookup(Tid, foo), + + % true = ets:insert(Tid, [T, T]), + % true = ets:insert(Tid, [T2]), % true = [T, T, T, T, T2] == ets:lookup(Tid, foo), % true = ets:member(Tid, foo), From 98acad032ae970f837151f61b40b9b855d009ee1 Mon Sep 17 00:00:00 2001 From: Jakub Gonet Date: Tue, 17 Jun 2025 16:13:29 +0200 Subject: [PATCH 14/17] ets: support duplicate_bag in insert/2 list Signed-off-by: Jakub Gonet --- src/libAtomVM/ets.c | 88 ++++++++++++++++++++------------- tests/erlang_tests/test_ets.erl | 8 ++- 2 files changed, 57 insertions(+), 39 deletions(-) diff --git a/src/libAtomVM/ets.c b/src/libAtomVM/ets.c index 69eb49b37..3cda96cdb 100644 --- a/src/libAtomVM/ets.c +++ b/src/libAtomVM/ets.c @@ -26,6 +26,7 @@ #include "list.h" #include "memory.h" #include "overflow_helpers.h" +#include "smp.h" #include "term.h" #include "utils.h" #include @@ -252,38 +253,70 @@ static inline bool has_key_at(term tuple, size_t key_index) return key_index < (size_t) term_get_tuple_arity(tuple); } -static EtsErrorCode insert(struct EtsTable *ets_table, term tuple, Context *ctx) +static struct HNode *tuple_to_insertion_node(struct EtsTable *ets_table, term tuple, GlobalContext *global) { + bool is_duplicate_bag = ets_table->table_type == EtsTableDuplicateBag; size_t key_index = ets_table->key_index; - if (UNLIKELY(!has_key_at(tuple, key_index))) { - return EtsBadEntry; - } - bool is_duplicate_bag = ets_table->table_type == EtsTableDuplicateBag; - struct HNode *node = NULL; if (is_duplicate_bag) { term key = term_get_tuple_element(tuple, key_index); - term old_tuples = ets_hashtable_lookup(ets_table->hashtable, key, ctx->global); + term old_tuples = ets_hashtable_lookup(ets_table->hashtable, key, global); assert(term_is_list(old_tuples)); bool is_new = term_is_nil(old_tuples); - node = ets_hashtable_new_node_from_list(is_new ? term_nil() : old_tuples, tuple, key_index); - } else { - node = ets_hashtable_new_node(tuple, key_index); + return ets_hashtable_new_node_from_list(is_new ? term_nil() : old_tuples, tuple, key_index); + } + return ets_hashtable_new_node(tuple, key_index); +} + +static struct HNode **list_to_insertion_nodes(struct EtsTable *ets_table, term list, size_t size, GlobalContext *global) +{ + size_t last_i = 0; + struct HNode **nodes = malloc(size * sizeof(struct HNode *)); + if (IS_NULL_PTR(nodes)) { + goto oom; + } + + while (term_is_nonempty_list(list)) { + term tuple = term_get_list_head(list); + nodes[last_i] = tuple_to_insertion_node(ets_table, tuple, global); + if (IS_NULL_PTR(nodes[last_i])) { + goto oom; + } + ++last_i; + list = term_get_list_tail(list); + } + return nodes; +oom: + // skip last node, it's NULL + for (size_t i = 0; i < last_i; ++i) { + ets_hashtable_free_node(nodes[i], global); } + free(nodes); + return NULL; +} + +static EtsErrorCode insert_tuple(struct EtsTable *ets_table, term tuple, GlobalContext *global) +{ + size_t key_index = ets_table->key_index; + if (UNLIKELY(!has_key_at(tuple, key_index))) { + return EtsBadEntry; + } + + struct HNode *node = tuple_to_insertion_node(ets_table, tuple, global); if (IS_NULL_PTR(node)) { return EtsAllocationFailure; } - EtsHashtableStatus res = ets_hashtable_insert(ets_table->hashtable, node, EtsHashtableAllowOverwrite, ctx->global); + EtsHashtableStatus res = ets_hashtable_insert(ets_table->hashtable, node, EtsHashtableAllowOverwrite, global); if (UNLIKELY(res != EtsHashtableOk)) { - ets_hashtable_free_node(node, ctx->global); + ets_hashtable_free_node(node, global); return EtsAllocationFailure; } return EtsOk; } -static EtsErrorCode insert_multiple(struct EtsTable *ets_table, term list, Context *ctx) +static EtsErrorCode insert_tuple_list(struct EtsTable *ets_table, term list, GlobalContext *global) { term iter = list; size_t size = 0; @@ -296,32 +329,19 @@ static EtsErrorCode insert_multiple(struct EtsTable *ets_table, term list, Conte } ++size; } - if (!term_is_nil(iter)) { + bool improper = !term_is_nil(iter); + if (UNLIKELY(improper)) { return EtsBadEntry; } - struct HNode **nodes = malloc(size * sizeof(struct HNode *)); + struct HNode **nodes = list_to_insertion_nodes(ets_table, list, size, global); if (IS_NULL_PTR(nodes)) { return EtsAllocationFailure; } - size_t i = 0; - while (term_is_nonempty_list(list)) { - term tuple = term_get_list_head(list); - nodes[i] = ets_hashtable_new_node(tuple, ets_table->key_index); - if (IS_NULL_PTR(nodes[i])) { - for (size_t it = 0; it < i; ++it) { - ets_hashtable_free_node(nodes[it], ctx->global); - } - free(nodes); - return EtsAllocationFailure; - } - ++i; - list = term_get_list_tail(list); - } - for (size_t i = 0; i < size; ++i) { - EtsHashtableStatus res = ets_hashtable_insert(ets_table->hashtable, nodes[i], EtsHashtableAllowOverwrite, ctx->global); + EtsHashtableStatus res = ets_hashtable_insert(ets_table->hashtable, nodes[i], EtsHashtableAllowOverwrite, global); + // insert can fail when comparing keys assert(res == EtsHashtableOk); } @@ -338,9 +358,9 @@ EtsErrorCode ets_insert(term name_or_ref, term entry, Context *ctx) EtsErrorCode result; if (term_is_tuple(entry)) { - result = insert(ets_table, entry, ctx); + result = insert_tuple(ets_table, entry, ctx->global); } else if (term_is_list(entry)) { - result = insert_multiple(ets_table, entry, ctx); + result = insert_tuple_list(ets_table, entry, ctx->global); } else { result = EtsBadEntry; } @@ -596,7 +616,7 @@ EtsErrorCode ets_update_counter_maybe_gc(term ref, term key, term operation, ter term final_value = term_from_int(elem_value); term_put_tuple_element(to_insert, elem_index, final_value); - EtsErrorCode insert_result = insert(ets_table, to_insert, ctx); + EtsErrorCode insert_result = insert_tuple(ets_table, to_insert, ctx->global); if (insert_result == EtsOk) { *ret = final_value; } diff --git a/tests/erlang_tests/test_ets.erl b/tests/erlang_tests/test_ets.erl index f394ca507..28efd3bb6 100644 --- a/tests/erlang_tests/test_ets.erl +++ b/tests/erlang_tests/test_ets.erl @@ -271,11 +271,9 @@ test_duplicate_bag() -> % false = ets:insert_new(Tid, T), true = ets:insert(Tid, T), true = ets:insert(Tid, T), - true = [T, T] == ets:lookup(Tid, foo), - - % true = ets:insert(Tid, [T, T]), - % true = ets:insert(Tid, [T2]), - % true = [T, T, T, T, T2] == ets:lookup(Tid, foo), + true = ets:insert(Tid, [T, T]), + true = ets:insert(Tid, [T2]), + true = [T, T, T, T2] == ets:lookup(Tid, foo), % true = ets:member(Tid, foo), % % nothing inserted, T exists in table From bca4c4f7b12cb4d7e594474c037da22a816c2dd8 Mon Sep 17 00:00:00 2001 From: Jakub Gonet Date: Tue, 17 Jun 2025 16:37:03 +0200 Subject: [PATCH 15/17] ets: remove prefix from internal functions Signed-off-by: Jakub Gonet --- src/libAtomVM/ets.c | 28 ++++++++++++++-------------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/src/libAtomVM/ets.c b/src/libAtomVM/ets.c index 3cda96cdb..c0042cde4 100644 --- a/src/libAtomVM/ets.c +++ b/src/libAtomVM/ets.c @@ -76,7 +76,7 @@ typedef enum TableAccessType TableAccessWrite } TableAccessType; -static void ets_delete_all_tables(struct Ets *ets, GlobalContext *global); +static void delete_all_tables(struct Ets *ets, GlobalContext *global); static void ets_add_table(struct Ets *ets, struct EtsTable *ets_table) { @@ -87,7 +87,7 @@ static void ets_add_table(struct Ets *ets, struct EtsTable *ets_table) synclist_unlock(&ets->ets_tables); } -static struct EtsTable *ets_acquire_table(struct Ets *ets, int32_t process_id, term name_or_ref, TableAccessType requested_access_type) +static struct EtsTable *acquire_table(struct Ets *ets, int32_t process_id, term name_or_ref, TableAccessType requested_access_type) { uint64_t ref = 0; term name = term_invalid_term(); @@ -135,14 +135,14 @@ void ets_init(struct Ets *ets) void ets_destroy(struct Ets *ets, GlobalContext *global) { - ets_delete_all_tables(ets, global); + delete_all_tables(ets, global); synclist_destroy(&ets->ets_tables); } EtsErrorCode ets_create_table_maybe_gc(term name, bool is_named, EtsTableType table_type, EtsAccessType access_type, size_t keypos, term *ret, Context *ctx) { if (is_named) { - struct EtsTable *ets_table = ets_acquire_table(&ctx->global->ets, ETS_ANY_PROCESS, name, TableAccessNone); + struct EtsTable *ets_table = acquire_table(&ctx->global->ets, ETS_ANY_PROCESS, name, TableAccessNone); if (ets_table != NULL) { return EtsTableNameInUse; } @@ -209,7 +209,7 @@ static void ets_table_destroy(struct EtsTable *table, GlobalContext *global) typedef bool (*ets_table_filter_pred)(struct EtsTable *table, void *data); -static void ets_delete_tables_internal(struct Ets *ets, ets_table_filter_pred pred, void *data, GlobalContext *global) +static void delete_tables_pred(struct Ets *ets, ets_table_filter_pred pred, void *data, GlobalContext *global) { struct ListHead *ets_tables_list = synclist_wrlock(&ets->ets_tables); struct ListHead *item; @@ -232,7 +232,7 @@ static bool equal_process_id_pred(struct EtsTable *table, void *data) void ets_delete_owned_tables(struct Ets *ets, int32_t process_id, GlobalContext *global) { - ets_delete_tables_internal(ets, equal_process_id_pred, &process_id, global); + delete_tables_pred(ets, equal_process_id_pred, &process_id, global); } static bool true_pred(struct EtsTable *table, void *data) @@ -243,9 +243,9 @@ static bool true_pred(struct EtsTable *table, void *data) return true; } -static void ets_delete_all_tables(struct Ets *ets, GlobalContext *global) +static void delete_all_tables(struct Ets *ets, GlobalContext *global) { - ets_delete_tables_internal(ets, true_pred, NULL, global); + delete_tables_pred(ets, true_pred, NULL, global); } static inline bool has_key_at(term tuple, size_t key_index) @@ -351,7 +351,7 @@ static EtsErrorCode insert_tuple_list(struct EtsTable *ets_table, term list, Glo EtsErrorCode ets_insert(term name_or_ref, term entry, Context *ctx) { - struct EtsTable *ets_table = ets_acquire_table(&ctx->global->ets, ctx->process_id, name_or_ref, TableAccessWrite); + struct EtsTable *ets_table = acquire_table(&ctx->global->ets, ctx->process_id, name_or_ref, TableAccessWrite); if (IS_NULL_PTR(ets_table)) { return EtsBadAccess; } @@ -425,7 +425,7 @@ static EtsErrorCode lookup_maybe_gc(struct EtsTable *ets_table, term key, size_t EtsErrorCode ets_lookup_maybe_gc(term name_or_ref, term key, term *ret, Context *ctx) { - struct EtsTable *ets_table = ets_acquire_table(&ctx->global->ets, ctx->process_id, name_or_ref, TableAccessRead); + struct EtsTable *ets_table = acquire_table(&ctx->global->ets, ctx->process_id, name_or_ref, TableAccessRead); if (IS_NULL_PTR(ets_table)) { return EtsBadAccess; } @@ -438,7 +438,7 @@ EtsErrorCode ets_lookup_maybe_gc(term name_or_ref, term key, term *ret, Context EtsErrorCode ets_lookup_element_maybe_gc(term name_or_ref, term key, size_t key_index, term *ret, Context *ctx) { - struct EtsTable *ets_table = ets_acquire_table(&ctx->global->ets, ctx->process_id, name_or_ref, TableAccessRead); + struct EtsTable *ets_table = acquire_table(&ctx->global->ets, ctx->process_id, name_or_ref, TableAccessRead); if (IS_NULL_PTR(ets_table)) { return EtsBadAccess; } @@ -462,7 +462,7 @@ EtsErrorCode ets_lookup_element_maybe_gc(term name_or_ref, term key, size_t key_ EtsErrorCode ets_drop_table(term name_or_ref, term *ret, Context *ctx) { - struct EtsTable *ets_table = ets_acquire_table(&ctx->global->ets, ctx->process_id, name_or_ref, TableAccessWrite); + struct EtsTable *ets_table = acquire_table(&ctx->global->ets, ctx->process_id, name_or_ref, TableAccessWrite); if (IS_NULL_PTR(ets_table)) { return EtsBadAccess; } @@ -480,7 +480,7 @@ EtsErrorCode ets_drop_table(term name_or_ref, term *ret, Context *ctx) EtsErrorCode ets_delete(term name_or_ref, term key, term *ret, Context *ctx) { - struct EtsTable *ets_table = ets_acquire_table(&ctx->global->ets, ctx->process_id, name_or_ref, TableAccessWrite); + struct EtsTable *ets_table = acquire_table(&ctx->global->ets, ctx->process_id, name_or_ref, TableAccessWrite); if (IS_NULL_PTR(ets_table)) { return EtsBadAccess; } @@ -540,7 +540,7 @@ static bool operation_to_tuple4(term operation, size_t default_pos, term *positi EtsErrorCode ets_update_counter_maybe_gc(term ref, term key, term operation, term default_value, term *ret, Context *ctx) { - struct EtsTable *ets_table = ets_acquire_table(&ctx->global->ets, ctx->process_id, ref, TableAccessWrite); + struct EtsTable *ets_table = acquire_table(&ctx->global->ets, ctx->process_id, ref, TableAccessWrite); if (IS_NULL_PTR(ets_table)) { return EtsBadAccess; } From d6eee069894aa383b7ee72f1ff006fe9e6f39a1f Mon Sep 17 00:00:00 2001 From: Jakub Gonet Date: Tue, 17 Jun 2025 16:45:17 +0200 Subject: [PATCH 16/17] ets: add duplicate_bag error for update_counter/4 Signed-off-by: Jakub Gonet --- src/libAtomVM/ets.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/libAtomVM/ets.c b/src/libAtomVM/ets.c index c0042cde4..0f99058f0 100644 --- a/src/libAtomVM/ets.c +++ b/src/libAtomVM/ets.c @@ -544,6 +544,10 @@ EtsErrorCode ets_update_counter_maybe_gc(term ref, term key, term operation, ter if (IS_NULL_PTR(ets_table)) { return EtsBadAccess; } + if (UNLIKELY(ets_table->table_type == EtsTableDuplicateBag)) { + SMP_UNLOCK(ets_table); + return EtsBadEntry; + } // do not use an invalid term as a root term safe_default_value = term_is_invalid_term(default_value) ? term_nil() : default_value; From 0e7d3f5245c4614ed9eba170b44ad99c6a62cd5e Mon Sep 17 00:00:00 2001 From: Jakub Gonet Date: Tue, 17 Jun 2025 16:45:41 +0200 Subject: [PATCH 17/17] ets: uncomment working tests Signed-off-by: Jakub Gonet --- tests/erlang_tests/test_ets.erl | 32 ++++++++++++++++---------------- 1 file changed, 16 insertions(+), 16 deletions(-) diff --git a/tests/erlang_tests/test_ets.erl b/tests/erlang_tests/test_ets.erl index 28efd3bb6..8c8510f90 100644 --- a/tests/erlang_tests/test_ets.erl +++ b/tests/erlang_tests/test_ets.erl @@ -23,14 +23,14 @@ -export([start/0]). start() -> - % ok = isolated(fun test_ets_new/0), - % ok = isolated(fun test_permissions/0), - % ok = isolated(fun test_keys/0), - % ok = isolated(fun test_keypos/0), - % ok = isolated(fun test_insert/0), - % ok = isolated(fun test_delete/0), - % ok = isolated(fun test_lookup_element/0), - % ok = isolated(fun test_update_counter/0), + ok = isolated(fun test_ets_new/0), + ok = isolated(fun test_permissions/0), + ok = isolated(fun test_keys/0), + ok = isolated(fun test_keypos/0), + ok = isolated(fun test_insert/0), + ok = isolated(fun test_delete/0), + ok = isolated(fun test_lookup_element/0), + ok = isolated(fun test_update_counter/0), ok = isolated(fun test_duplicate_bag/0), 0. @@ -265,7 +265,7 @@ test_duplicate_bag() -> Tid = ets:new(test_duplicate_bag, [duplicate_bag, {keypos, 2}]), T = {ok, foo, 100, extra}, T2 = {error, foo, 200}, - T3 = {error, foo, 300}, + _T3 = {error, foo, 300}, % true = ets:insert_new(Tid, T), % false = ets:insert_new(Tid, T), @@ -280,15 +280,15 @@ test_duplicate_bag() -> % false = ets:insert_new(Tid, [T, {ok, bar, batat}]), % false = ets:member(Tid, bar), - % [ok, ok, ok, ok, error] = ets:lookup_element(Tid, foo, 1), - % [foo, foo, foo, foo, foo] = ets:lookup_element(Tid, foo, 2), - % [100, 100, 100, 100, 200] = ets:lookup_element(Tid, foo, 3), + [ok, ok, ok, error] = ets:lookup_element(Tid, foo, 1), + [foo, foo, foo, foo] = ets:lookup_element(Tid, foo, 2), + [100, 100, 100, 200] = ets:lookup_element(Tid, foo, 3), % % some tuples don't have 4 arity - % ok = expect_failure(fun() -> ets:lookup_element(Tid, foo, 4) end), + ok = assert_badarg(fun() -> ets:lookup_element(Tid, foo, 4) end), % % unsupported for duplicate bag - % ok = expect_failure(fun() -> ets:update_counter(Tid, foo, 10) end), - % ok = expect_failure(fun() -> ets:update_element(Tid, foo, {1, error}) end), + ok = assert_badarg(fun() -> ets:update_counter(Tid, foo, 10) end), + % ok = assert_badarg(fun() -> ets:update_element(Tid, foo, {1, error}) end), % true = ets:delete_object(Tid, {bad, bad}), % true = [T, T, T, T, T2] == ets:lookup(Tid, foo), @@ -300,7 +300,7 @@ test_duplicate_bag() -> % true = [T2, T3] == ets:take(Tid, foo), % true = ets:delete(Tid), - % ok = expect_failure(fun() -> ets:insert(Tid, T) end), + % ok = assert_badarg(fun() -> ets:insert(Tid, T) end), ok.