File 1207-erts-Fix-ets-insert-with-list-into-bag-bei... of Package erlang

Overview Repositories Revisions Requests Users Attributes Meta

File 1207-erts-Fix-ets-insert-with-list-into-bag-being-reverse.patch of Package erlang (Revision 1e94e72e3f4fed1ca9cd5c2fd777ed02)

Currently displaying revision 1e94e72e3f4fed1ca9cd5c2fd777ed02 , Show latest

From 68328c1f89c3622f9f6882f4924207c5cc5c44b2 Mon Sep 17 00:00:00 2001
From: Julian Doherty <madlep@madlep.com>
Date: Sat, 28 Jan 2023 17:27:02 +1300
Subject: [PATCH 1/2] erts: Fix ets:insert with list into bag being reversed

After discussion in https://github.com/erlang/otp/issues/6730

Changed `db_term_list_prepend*` to `db_term_list_append*` so values to
be stored against a key have their order preserved. Previously due to
building the linked list via prepending, the values insert order was
reversed from OTP 23 onward.
---
 erts/emulator/beam/erl_db.c           |  7 ++++---
 erts/emulator/beam/erl_db_catree.c    |  2 +-
 erts/emulator/beam/erl_db_hash.c      | 10 ++++-----
 erts/emulator/beam/erl_db_tree.c      |  8 +++----
 erts/emulator/beam/erl_db_tree_util.h |  2 +-
 erts/emulator/beam/erl_db_util.h      |  2 +-
 lib/stdlib/doc/src/ets.xml            | 21 +++++++++++++++++++
 lib/stdlib/test/ets_SUITE.erl         | 30 ++++++++++++++++++++++++++-
 8 files changed, 66 insertions(+), 16 deletions(-)

diff --git a/erts/emulator/beam/erl_db.c b/erts/emulator/beam/erl_db.c
index 9ce88c35d6..392a4b901c 100644
--- a/erts/emulator/beam/erl_db.c
+++ b/erts/emulator/beam/erl_db.c
@@ -1682,17 +1682,18 @@ static void* ets_insert_2_list_copy_term_list(DbTableMethod* meth,
 {
 void* db_term_list = NULL;
 void *term;
+ void *last_term;
 Eterm lst;
 for (lst = list; is_list(lst); lst = CDR(list_val(lst))) {
 term = meth->db_eterm_to_dbterm(compress,
 keypos,
 CAR(list_val(lst)));
 if (db_term_list != NULL) {
- db_term_list =
- meth->db_dbterm_list_prepend(db_term_list,
- term);
+ last_term =
+ meth->db_dbterm_list_append(last_term, term);
 } else {
 db_term_list = term;
+ last_term = term;
 }
 }
 
diff --git a/erts/emulator/beam/erl_db_catree.c b/erts/emulator/beam/erl_db_catree.c
index 3dd8c0b609..035aaaa3da 100644
--- a/erts/emulator/beam/erl_db_catree.c
+++ b/erts/emulator/beam/erl_db_catree.c
@@ -220,7 +220,7 @@ DbTableMethod db_catree =
 db_lookup_dbterm_catree,
 db_finalize_dbterm_catree,
 db_eterm_to_dbterm_tree_common,
- db_dbterm_list_prepend_tree_common,
+ db_dbterm_list_append_tree_common,
 db_dbterm_list_remove_first_tree_common,
 db_put_dbterm_catree,
 db_free_dbterm_tree_common,
diff --git a/erts/emulator/beam/erl_db_hash.c b/erts/emulator/beam/erl_db_hash.c
index 7178d7dc84..89d82cc436 100644
--- a/erts/emulator/beam/erl_db_hash.c
+++ b/erts/emulator/beam/erl_db_hash.c
@@ -726,7 +726,7 @@ db_lookup_dbterm_hash(Process *p, DbTable *tbl, Eterm key, Eterm obj,
 static void
 db_finalize_dbterm_hash(int cret, DbUpdateHandle* handle);
 static void* db_eterm_to_dbterm_hash(int compress, int keypos, Eterm obj);
-static void* db_dbterm_list_prepend_hash(void* list, void* db_term);
+static void* db_dbterm_list_append_hash(void* last_term, void* db_term);
 static void* db_dbterm_list_remove_first_hash(void** list);
 static int db_put_dbterm_hash(DbTable* tb,
 void* obj,
@@ -866,7 +866,7 @@ DbTableMethod db_hash =
 db_lookup_dbterm_hash,
 db_finalize_dbterm_hash,
 db_eterm_to_dbterm_hash,
- db_dbterm_list_prepend_hash,
+ db_dbterm_list_append_hash,
 db_dbterm_list_remove_first_hash,
 db_put_dbterm_hash,
 db_free_dbterm_hash,
@@ -4170,11 +4170,11 @@ static void* db_eterm_to_dbterm_hash(int compress, int keypos, Eterm obj)
 return term;
 }
 
-static void* db_dbterm_list_prepend_hash(void* list, void* db_term)
+static void* db_dbterm_list_append_hash(void* last_term, void* db_term)
 {
- HashDbTerm* l = list;
+ HashDbTerm* l = last_term;
 HashDbTerm* t = db_term;
- t->next = l;
+ l->next = t;
 return t;
 }
 
diff --git a/erts/emulator/beam/erl_db_tree.c b/erts/emulator/beam/erl_db_tree.c
index 23b28e47f8..e96c35e9ef 100644
--- a/erts/emulator/beam/erl_db_tree.c
+++ b/erts/emulator/beam/erl_db_tree.c
@@ -519,7 +519,7 @@ DbTableMethod db_tree =
 db_lookup_dbterm_tree,
 db_finalize_dbterm_tree,
 db_eterm_to_dbterm_tree_common,
- db_dbterm_list_prepend_tree_common,
+ db_dbterm_list_append_tree_common,
 db_dbterm_list_remove_first_tree_common,
 db_put_dbterm_tree,
 db_free_dbterm_tree_common,
@@ -3533,11 +3533,11 @@ void* db_eterm_to_dbterm_tree_common(int compress, int keypos, Eterm obj)
 return term;
 }
 
-void* db_dbterm_list_prepend_tree_common(void *list, void *db_term)
+void* db_dbterm_list_append_tree_common(void *last_term, void *db_term)
 {
- TreeDbTerm* l = list;
+ TreeDbTerm* l = last_term;
 TreeDbTerm* t = db_term;
- t->left = l;
+ l->left = t;
 return t;
 }
 
diff --git a/erts/emulator/beam/erl_db_tree_util.h b/erts/emulator/beam/erl_db_tree_util.h
index 75c82003b2..7592efb2b8 100644
--- a/erts/emulator/beam/erl_db_tree_util.h
+++ b/erts/emulator/beam/erl_db_tree_util.h
@@ -172,7 +172,7 @@ void db_finalize_dbterm_tree_common(int cret,
 TreeDbTerm **root,
 DbTableTree *stack_container);
 void* db_eterm_to_dbterm_tree_common(int compress, int keypos, Eterm obj);
-void* db_dbterm_list_prepend_tree_common(void* list, void* db_term);
+void* db_dbterm_list_append_tree_common(void* last_term, void* db_term);
 void* db_dbterm_list_remove_first_tree_common(void **list);
 int db_put_dbterm_tree_common(DbTableCommon *tb, TreeDbTerm **root, TreeDbTerm *value_to_insert,
 int key_clash_fail, DbTableTree *stack_container);
diff --git a/erts/emulator/beam/erl_db_util.h b/erts/emulator/beam/erl_db_util.h
index 11f1be017e..1d8261b4ca 100644
--- a/erts/emulator/beam/erl_db_util.h
+++ b/erts/emulator/beam/erl_db_util.h
@@ -237,7 +237,7 @@ typedef struct db_table_method
 ** not DB_ERROR_NONE, the object is removed from the table. */
 void (*db_finalize_dbterm)(int cret, DbUpdateHandle* handle);
 void* (*db_eterm_to_dbterm)(int compress, int keypos, Eterm obj);
- void* (*db_dbterm_list_prepend)(void* list, void* db_term);
+ void* (*db_dbterm_list_append)(void* last_term, void* db_term);
 void* (*db_dbterm_list_remove_first)(void** list);
 int (*db_put_dbterm)(DbTable* tb, /* [in out] */
 void* obj,
diff --git a/lib/stdlib/doc/src/ets.xml b/lib/stdlib/doc/src/ets.xml
index 507d27d8d6..005fda8089 100644
--- a/lib/stdlib/doc/src/ets.xml
+++ b/lib/stdlib/doc/src/ets.xml
@@ -825,6 +825,27 @@ Error: fun containing local Erlang function calls
 The entire operation is guaranteed to be
 <seeerl marker="#concurrency">atomic and isolated</seeerl>,
 even when a list of objects is inserted.
+ <marker id="insert_list_order"></marker>
+ 
+	 For <c>bag</c> and <c>duplicate_bag</c>, objects in the list with
+	 identical keys will be inserted in list order (from head to tail). That
+	 is, a subsequent call to <seemfa marker="#lookup/2"><c>lookup(T,Key)</c></seemfa>
+	 will return them in that inserted order.
+ 
+	<note>
+	 
+	 For <c>bag</c> the insertion order of indentical keys described above was
+	 accidentally reverted in OTP 23.0 and later fixed in OTP 25.3. That
+	 is, from OTP 23.0 up until OTP 25.3 the objects in a list are
+	 inserted in reverse order (from tail to head).
+ 
+	 
+	 For <c>duplicate_bag</c> the same faulty reverse insertion exist
+	 from OTP 23.0 until OTP 25.3. However, it is unpredictable and may
+	 or may not happen. A longer list will increase the probabiliy of the
+	 insertion being done in reverse.
+ 
+ </note>
 </desc>
 </func>
 
diff --git a/lib/stdlib/test/ets_SUITE.erl b/lib/stdlib/test/ets_SUITE.erl
index 7f63a9903c..4cfc6933c8 100644
--- a/lib/stdlib/test/ets_SUITE.erl
+++ b/lib/stdlib/test/ets_SUITE.erl
@@ -51,7 +51,8 @@
 -export([t_insert_list/1, t_insert_list_bag/1, t_insert_list_duplicate_bag/1,
 t_insert_list_set/1, t_insert_list_delete_set/1,
 t_insert_list_parallel/1, t_insert_list_delete_parallel/1,
- t_insert_list_kill_process/1]).
+ t_insert_list_kill_process/1,
+ t_insert_list_insert_order_preserved/1]).
 -export([test_table_size_concurrency/1,test_table_memory_concurrency/1,
 test_delete_table_while_size_snapshot/1, test_delete_table_while_size_snapshot_helper/1,
 test_decentralized_counters_setting/1]).
@@ -222,7 +223,8 @@ groups() ->
 [t_insert_list, t_insert_list_set, t_insert_list_bag,
 t_insert_list_duplicate_bag, t_insert_list_delete_set,
 t_insert_list_parallel, t_insert_list_delete_parallel,
- t_insert_list_kill_process]}].
+ t_insert_list_kill_process,
+ t_insert_list_insert_order_preserved]}].
 
 init_per_suite(Config) ->
 erts_debug:set_internal_state(available_internal_state, true),
@@ -1553,6 +1555,32 @@ t_insert_list_kill_process_do(Opts) ->
 fun ets:insert_new/2]],
 ok.
 
+t_insert_list_insert_order_preserved(Config) when is_list(Config) ->
+ insert_list_insert_order_preserved(bag),
+ insert_list_insert_order_preserved(duplicate_bag),
+ ok.
+
+insert_list_insert_order_preserved(Type) ->
+ Tab = ets:new(?FUNCTION_NAME, [Type]),
+ K = a,
+ Values1 = [{K, 1}, {K, 2}, {K, 3}],
+ Values2 = [{K, 4}, {K, 5}, {K, 6}],
+ ets:insert(Tab, Values1),
+ ets:insert(Tab, Values2),
+ [{K, 1}, {K, 2}, {K, 3}, {K, 4}, {K, 5}, {K, 6}] = ets:lookup(Tab, K),
+
+ ets:delete(Tab, K),
+ [] = ets:lookup(Tab, K),
+
+ %% Insert order in duplicate_bag depended on reductions left
+ ITERATIONS_PER_RED = 8,
+ NTuples = 4000 * ITERATIONS_PER_RED + 10,
+ LongList = [{K, V} || V <- lists:seq(1, NTuples)],
+ ets:insert(Tab, LongList),
+ LongList = ets:lookup(Tab, K),
+
+ ets:delete(Tab).
+
 %% Test interface of ets:test_ms/2.
 t_test_ms(Config) when is_list(Config) ->
 EtsMem = etsmem(),
-- 
2.35.3

Places

File 1207-erts-Fix-ets-insert-with-list-into-bag-being-reverse.patch of Package erlang (Revision 1e94e72e3f4fed1ca9cd5c2fd777ed02)

Places