File 9191-erts-Make-map-element-ordering-consistent-across-all.patch of Package erlang
From a0d491f33b98bd8d0680f02c295c0bb6474a9f4c Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Micha=C5=82=20Muska=C5=82a?= <micmus@whatsapp.com>
Date: Tue, 3 Feb 2026 08:19:46 -0800
Subject: [PATCH] erts: Make map element ordering consistent across all
operations
For hashmaps (large maps with >32 elements), the element ordering
previously differed between operations:
- maps:to_list/1, maps:keys/1, maps:values/1 returned elements in
reverse traversal order
- maps:to_list(maps:iterator(M)) and map comprehensions returned
elements in forward traversal order
This inconsistency was confusing, and while there were no guarantees
about map orders, a seemingly insignificant code change
(comprehension over `maps:to_list` to a map comprehension) could
wreck havoc on the result of a calculation.
In particular, we don't give a guarantee about any specific order
elements will be yielded, but give a simpler guarantee that all the
ways of iterating the same map value will yield elements in the same
order.
This commit changes hashmap_keys/1, hashmap_values/1, and the list
mode of erts_internal:map_next/3 to use reverse traversal instead of
forward traversal. Combined with CONS prepending, this produces
forward traversal order, matching the iterator mode.
After this change, all map operations return elements in the same
order for a given map:
- maps:keys/1
- maps:values/1
- maps:to_list/1
- maps:to_list(maps:iterator(M))
- Map comprehensions [{K,V} || K := V <- M]
This change shouldn't impact performance of the operations in any
meaningful way.
---
erts/emulator/beam/erl_map.c | 268 +++++++++++++++++++++++----------
lib/stdlib/test/maps_SUITE.erl | 43 ++++++
2 files changed, 233 insertions(+), 78 deletions(-)
diff --git a/erts/emulator/beam/erl_map.c b/erts/emulator/beam/erl_map.c
index 541d3c1220..d0bf8fe8f9 100644
--- a/erts/emulator/beam/erl_map.c
+++ b/erts/emulator/beam/erl_map.c
@@ -2832,8 +2832,8 @@ static Eterm hashmap_keys(Process* p, Eterm node) {
YCF_SPECIAL_CODE_END();
root = (hashmap_head_t*) boxed_val(node);
hp = HAlloc(p, root->size * 2);
- hashmap_iterator_init(&stack, node, 0);
- while ((kv=hashmap_iterator_next(&stack)) != NULL) {
+ hashmap_iterator_init(&stack, node, 1);
+ while ((kv=hashmap_iterator_prev(&stack)) != NULL) {
res = CONS(hp, CAR(kv), res);
hp += 2;
}
@@ -2876,8 +2876,8 @@ static Eterm hashmap_values(Process* p, Eterm node) {
root = (hashmap_head_t*) boxed_val(node);
hp = HAlloc(p, root->size * 2);
- hashmap_iterator_init(&stack, node, 0);
- while ((kv=hashmap_iterator_next(&stack)) != NULL) {
+ hashmap_iterator_init(&stack, node, 1);
+ while ((kv=hashmap_iterator_prev(&stack)) != NULL) {
res = CONS(hp, CDR(kv), res);
hp += 2;
}
@@ -3762,9 +3762,15 @@ BIF_RETTYPE erts_internal_map_next_3(BIF_ALIST_3) {
* `node` is not really needed, but it is very nice to
* have when debugging.
*
- * `idx` always points to the next un-explored entry in
- * a node. If there are no more un-explored entries,
- * `idx` is equal to `sz`.
+ * For iterator mode:
+ * `idx` always points to the next un-explored entry in
+ * a node (forward direction). If there are no more entries,
+ * `idx` is equal to `sz`.
+ *
+ * For list mode:
+ * `idx` always points to the next un-explored entry in
+ * a node (reverse direction). If there are no more entries,
+ * `idx` is 0 (meaning we've gone past index 0).
*
* `sz` is the number of elements in the node.
*
@@ -3814,7 +3820,12 @@ BIF_RETTYPE erts_internal_map_next_3(BIF_ALIST_3) {
/* First we look for the leaf to start at using the
path given. While doing so, we push each map node
- and the index onto the stack to use later. */
+ and the index onto the stack to use later.
+
+ For list mode, we traverse in reverse order (from high to low index)
+ to produce elements in forward order when combined with CONS prepending.
+ For iterator mode, we traverse in forward order (from low to high index).
+ */
for (i = 1; ; i++) {
Eterm hdr;
@@ -3823,7 +3834,12 @@ BIF_RETTYPE erts_internal_map_next_3(BIF_ALIST_3) {
sz = hashmap_node_size(hdr, &ptr);
- idx = PATH_ELEM(curr_path);
+ if (type == iterator) {
+ idx = PATH_ELEM(curr_path);
+ } else {
+ /* List mode: path encodes offset from end */
+ idx = sz - 1 - PATH_ELEM(curr_path);
+ }
if (idx >= sz)
goto badarg;
@@ -3832,7 +3848,12 @@ BIF_RETTYPE erts_internal_map_next_3(BIF_ALIST_3) {
break;
}
- WSTACK_PUSH4(stack, node, idx+1, sz, (UWord)ptr);
+ if (type == iterator) {
+ WSTACK_PUSH4(stack, node, idx+1, sz, (UWord)ptr);
+ } else {
+ /* List mode: store idx-1 as the next index to visit (going backward) */
+ WSTACK_PUSH4(stack, node, idx-1, sz, (UWord)ptr);
+ }
node = ptr[idx];
@@ -3877,85 +3898,168 @@ BIF_RETTYPE erts_internal_map_next_3(BIF_ALIST_3) {
orig_elems = elems;
- /* We traverse the hashmap and return at most `elems` elements */
+ /* We traverse the hashmap and return at most `elems` elements.
+ * For iterator mode, we traverse forward (idx 0 to sz-1).
+ * For list mode, we traverse backward (idx sz-1 to 0) to produce
+ * forward order when combined with CONS prepending.
+ */
while(1) {
- if (idx == 0) {
- if (elems < sz && is_arity_value(*hashmap_val(node))) {
- /*
- * This is a collision node!
- * Make sure 'elems' is large enough not to yield in the
- * middle of it. Collision nodes may be larger than 16
- * and that would complicate the 4-bit path format.
- */
- elems = sz;
- HRelease(BIF_P, hp_end, hp);
- hp = HAlloc(BIF_P, words_per_elem * elems);
- hp_end = hp + words_per_elem * elems;
+ if (type == iterator) {
+ /* Forward traversal: check collision at start (idx == 0) */
+ if (idx == 0) {
+ if (elems < sz && is_arity_value(*hashmap_val(node))) {
+ /*
+ * This is a collision node!
+ * Make sure 'elems' is large enough not to yield in the
+ * middle of it. Collision nodes may be larger than 16
+ * and that would complicate the 4-bit path format.
+ */
+ elems = sz;
+ HRelease(BIF_P, hp_end, hp);
+ hp = HAlloc(BIF_P, words_per_elem * elems);
+ hp_end = hp + words_per_elem * elems;
+ }
}
- }
- else
- ASSERT(!is_arity_value(*hashmap_val(node)));
+ else
+ ASSERT(!is_arity_value(*hashmap_val(node)));
- while (idx < sz && elems != 0 && is_list(ptr[idx])) {
- Eterm *lst = list_val(ptr[idx]);
- if (type == iterator) {
+ /* Forward traversal: increment idx */
+ while (idx < sz && elems != 0 && is_list(ptr[idx])) {
+ Eterm *lst = list_val(ptr[idx]);
*patch_ptr = make_tuple(hp);
hp[0] = make_arityval(3);
hp[1] = CAR(lst);
hp[2] = CDR(lst);
patch_ptr = &hp[3];
hp += 4;
- } else {
- Eterm tup = TUPLE2(hp, CAR(lst), CDR(lst)); hp += 3;
- res = CONS(hp, tup, res); hp += 2;
+ elems--;
+ idx++;
}
- elems--;
- idx++;
- }
- ASSERT(idx == sz || !is_arity_value(*hashmap_val(node)));
+ ASSERT(idx == sz || !is_arity_value(*hashmap_val(node)));
- if (elems == 0) {
- if (idx < sz) {
- /* There are more elements in this node to explore */
- WSTACK_PUSH4(stack, node, idx+1, sz, (UWord)ptr);
- } else {
- /* pop stack to find the next value */
- while (!WSTACK_ISEMPTY(stack)) {
- Eterm *ptr = (Eterm*)WSTACK_POP(stack);
- Uint sz = (Uint)WSTACK_POP(stack);
- Uint idx = (Uint)WSTACK_POP(stack);
- Eterm node = (Eterm)WSTACK_POP(stack);
- if (idx < sz) {
- WSTACK_PUSH4(stack, node, idx+1, sz, (UWord)ptr);
- break;
+ if (elems == 0) {
+ if (idx < sz) {
+ /* There are more elements in this node to explore */
+ WSTACK_PUSH4(stack, node, idx+1, sz, (UWord)ptr);
+ } else {
+ /* pop stack to find the next value */
+ while (!WSTACK_ISEMPTY(stack)) {
+ Eterm *ptr = (Eterm*)WSTACK_POP(stack);
+ Uint sz = (Uint)WSTACK_POP(stack);
+ Uint idx = (Uint)WSTACK_POP(stack);
+ Eterm node = (Eterm)WSTACK_POP(stack);
+ if (idx < sz) {
+ WSTACK_PUSH4(stack, node, idx+1, sz, (UWord)ptr);
+ break;
+ }
}
}
+ break;
}
- break;
- }
- else if (idx < sz) {
- Eterm hdr;
- /* Push next idx in current node */
- WSTACK_PUSH4(stack, node, idx+1, sz, (UWord)ptr);
+ else if (idx < sz) {
+ Eterm hdr;
+ /* Push next idx in current node */
+ WSTACK_PUSH4(stack, node, idx+1, sz, (UWord)ptr);
- /* Continue with first idx in child node */
- node = ptr[idx];
- ptr = hashmap_val(ptr[idx]);
- hdr = *ptr++;
- sz = hashmap_node_size(hdr, &ptr);
- idx = 0;
- }
- else if (!WSTACK_ISEMPTY(stack)) {
- ptr = (Eterm*)WSTACK_POP(stack);
- sz = (Uint)WSTACK_POP(stack);
- idx = (Uint)WSTACK_POP(stack);
- node = (Eterm)WSTACK_POP(stack);
- }
- else {
- /* There are no more element in the hashmap */
- break;
+ /* Continue with first idx in child node */
+ node = ptr[idx];
+ ptr = hashmap_val(ptr[idx]);
+ hdr = *ptr++;
+ sz = hashmap_node_size(hdr, &ptr);
+ idx = 0;
+ }
+ else if (!WSTACK_ISEMPTY(stack)) {
+ ptr = (Eterm*)WSTACK_POP(stack);
+ sz = (Uint)WSTACK_POP(stack);
+ idx = (Uint)WSTACK_POP(stack);
+ node = (Eterm)WSTACK_POP(stack);
+ }
+ else {
+ /* There are no more elements in the hashmap */
+ break;
+ }
+ } else {
+ /* List mode: reverse traversal */
+
+ /* Reverse traversal: check collision at end (idx == sz-1) */
+ if (idx == sz - 1) {
+ if (elems < sz && is_arity_value(*hashmap_val(node))) {
+ /*
+ * This is a collision node!
+ * Make sure 'elems' is large enough not to yield in the
+ * middle of it. Collision nodes may be larger than 16
+ * and that would complicate the 4-bit path format.
+ */
+ elems = sz;
+ HRelease(BIF_P, hp_end, hp);
+ hp = HAlloc(BIF_P, words_per_elem * elems);
+ hp_end = hp + words_per_elem * elems;
+ }
+ }
+ else
+ ASSERT(!is_arity_value(*hashmap_val(node)));
+
+ /* Reverse traversal: decrement idx. We use a signed variable
+ * to detect when we go below 0. */
+ {
+ Sint sidx = (Sint)idx;
+ while (sidx >= 0 && elems != 0 && is_list(ptr[sidx])) {
+ Eterm *lst = list_val(ptr[sidx]);
+ Eterm tup = TUPLE2(hp, CAR(lst), CDR(lst)); hp += 3;
+ res = CONS(hp, tup, res); hp += 2;
+ elems--;
+ sidx--;
+ }
+ idx = (Uint)sidx; /* May wrap to large value if sidx < 0 */
+ }
+
+ ASSERT(idx >= sz || !is_arity_value(*hashmap_val(node)));
+
+ if (elems == 0) {
+ if (idx < sz) {
+ /* There are more elements in this node to explore (idx is valid) */
+ WSTACK_PUSH4(stack, node, idx-1, sz, (UWord)ptr);
+ } else {
+ /* idx wrapped (was -1), pop stack to find the next value */
+ while (!WSTACK_ISEMPTY(stack)) {
+ Eterm *ptr = (Eterm*)WSTACK_POP(stack);
+ Uint sz = (Uint)WSTACK_POP(stack);
+ Uint idx = (Uint)WSTACK_POP(stack);
+ Eterm node = (Eterm)WSTACK_POP(stack);
+ if (idx < sz) {
+ /* idx is valid, continue from here */
+ WSTACK_PUSH4(stack, node, idx-1, sz, (UWord)ptr);
+ break;
+ }
+ }
+ }
+ break;
+ }
+ else if (idx < sz) {
+ Eterm hdr;
+ /* Push previous idx in current node (going backward) */
+ WSTACK_PUSH4(stack, node, idx-1, sz, (UWord)ptr);
+
+ /* Continue with last idx in child node */
+ node = ptr[idx];
+ ptr = hashmap_val(ptr[idx]);
+ hdr = *ptr++;
+ sz = hashmap_node_size(hdr, &ptr);
+ idx = sz - 1;
+ }
+ else if (!WSTACK_ISEMPTY(stack)) {
+ /* idx wrapped (was -1), pop from stack */
+ ptr = (Eterm*)WSTACK_POP(stack);
+ sz = (Uint)WSTACK_POP(stack);
+ idx = (Uint)WSTACK_POP(stack);
+ node = (Eterm)WSTACK_POP(stack);
+ }
+ else {
+ /* There are no more elements in the hashmap */
+ break;
+ }
}
}
@@ -3977,13 +4081,21 @@ BIF_RETTYPE erts_internal_map_next_3(BIF_ALIST_3) {
/* Pop the stack to create the complete path to the next leaf */
while(!WSTACK_ISEMPTY(stack)) {
- Uint idx;
+ Uint idx, sz, path_elem;
(void)WSTACK_POP(stack);
- (void)WSTACK_POP(stack);
- idx = (Uint)WSTACK_POP(stack)-1;
- /* idx - 1 because idx in the stack is pointing to
- the next element to fetch. */
+ sz = (Uint)WSTACK_POP(stack);
+ if (type == iterator) {
+ idx = (Uint)WSTACK_POP(stack) - 1;
+ /* idx - 1 because idx in the stack is pointing to
+ the next element to fetch (forward direction). */
+ path_elem = idx;
+ } else {
+ idx = (Uint)WSTACK_POP(stack) + 1;
+ /* idx + 1 because idx in the stack is pointing to
+ the next element to fetch (backward direction). */
+ path_elem = sz - 1 - idx; /* Encode as offset from end */
+ }
(void)WSTACK_POP(stack);
depth--;
@@ -3995,7 +4107,7 @@ BIF_RETTYPE erts_internal_map_next_3(BIF_ALIST_3) {
}
curr_path <<= PATH_ELEM_SIZE;
- curr_path |= idx;
+ curr_path |= path_elem;
}
if (path_digits) {
diff --git a/lib/stdlib/test/maps_SUITE.erl b/lib/stdlib/test/maps_SUITE.erl
index 0f20398803..f75bc998aa 100644
--- a/lib/stdlib/test/maps_SUITE.erl
+++ b/lib/stdlib/test/maps_SUITE.erl
@@ -32,6 +32,7 @@
t_fold_3/1,t_map_2/1,t_size_1/1, t_foreach_2/1,
t_iterator_1/1,
t_iterator_valid/1,
+ t_order_consistency/1,
t_put_opt/1, t_merge_opt/1,
t_with_2/1,t_without_2/1,
t_intersect/1, t_intersect_with/1,
@@ -61,6 +62,7 @@ all() ->
t_fold_3,t_map_2,t_size_1,t_foreach_2,
t_iterator_1,
t_iterator_valid,
+ t_order_consistency,
t_put_opt,t_merge_opt,
t_with_2,t_without_2,
t_intersect, t_intersect_with,
@@ -653,6 +655,47 @@ t_iterator_valid(Config) when is_list(Config) ->
ok.
+%% Test that maps:keys/1, maps:values/1, maps:to_list/1, iterators,
+%% and map comprehensions all return elements in the same order.
+t_order_consistency(Config) when is_list(Config) ->
+ %% Test small map (flatmap)
+ SmallMap = #{a => 1, b => 2, c => 3, z => 26, m => 13},
+ verify_order_consistency(SmallMap),
+
+ %% Test large map (hashmap)
+ LargeMap = maps:from_list([{I, I} || I <- lists:seq(1, 100)]),
+ verify_order_consistency(LargeMap),
+
+ %% Test very large map (hashmap with yielding)
+ VeryLargeMap = maps:from_list([{I, I * 10} || I <- lists:seq(1, 10000)]),
+ verify_order_consistency(VeryLargeMap),
+
+ %% Test with mixed key types
+ MixedMap = maps:from_list([{I, I} || I <- lists:seq(1, 50)] ++
+ [{list_to_atom("k" ++ integer_to_list(I)), I}
+ || I <- lists:seq(1, 50)]),
+ verify_order_consistency(MixedMap),
+
+ ok.
+
+verify_order_consistency(M) ->
+ Keys = maps:keys(M),
+ Values = maps:values(M),
+ ToList = maps:to_list(M),
+
+ %% to_list should match zip of keys and values
+ ToList = lists:zip(Keys, Values),
+
+ %% Iterator should produce same list
+ IterList = maps:to_list(maps:iterator(M)),
+ ToList = IterList,
+
+ %% Map comprehension should produce same list
+ CompList = [{K, V} || K := V <- M],
+ ToList = CompList,
+
+ ok.
+
t_put_opt(Config) when is_list(Config) ->
Value = id(#{complex => map}),
Small = id(#{a => Value}),
--
2.51.0
