Merge branch 'sverk/dist-ctrl-msg-overflow' into maint

OTP-13146  (more stuff)

* sverk/dist-ctrl-msg-overflow:
  erts: Tweak hashmap heap size estimation
  erts: Fix bug for remote control message containing fat maps
  erts: Add test for remote exit signal with fat map
  erts: Fix bug in heap_factory_undo for FACTORY_HEAP_FRAGS mode

5 files changed, 83 insertions, 41 deletions

diff --git a/erts/emulator/beam/dist.c b/erts/emulator/beam/dist.c
index 4846133aa6..170690ca89 100644
--- a/erts/emulator/beam/dist.c
+++ b/erts/emulator/beam/dist.c
@@ -1153,7 +1153,6 @@ int erts_net_message(Port *prt,
     Process* rp;
     DeclareTmpHeapNoproc(ctl_default,DIST_CTL_DEFAULT_SIZE);
     Eterm* ctl = ctl_default;
-    ErlOffHeap off_heap;
     ErtsHeapFactory factory;
     Eterm* hp;
     Sint type;
@@ -1168,9 +1167,6 @@ int erts_net_message(Port *prt,
 #endif
 
     UseTmpHeapNoproc(DIST_CTL_DEFAULT_SIZE);
-    /* Thanks to Luke Gorrie */
-    off_heap.first = NULL;
-    off_heap.overhead = 0;
 
     ERTS_SMP_CHK_NO_PROC_LOCKS;
 
@@ -1231,7 +1227,7 @@ int erts_net_message(Port *prt,
     }
     hp = ctl;
 
-    erts_factory_static_init(&factory, ctl, ctl_len, &off_heap);
+    erts_factory_tmp_init(&factory, ctl, ctl_len, ERTS_ALC_T_DCTRL_BUF);
     arg = erts_decode_dist_ext(&factory, &ede);
     if (is_non_value(arg)) {
 #ifdef ERTS_DIST_MSG_DBG
@@ -1719,7 +1715,7 @@ int erts_net_message(Port *prt,
 	goto invalid_message;
     }
 
-    erts_cleanup_offheap(&off_heap);
+    erts_factory_close(&factory);
     if (ctl != ctl_default) {
 	erts_free(ERTS_ALC_T_DCTRL_BUF, (void *) ctl);
     }
@@ -1734,7 +1730,7 @@ int erts_net_message(Port *prt,
     }
  data_error:
     PURIFY_MSG("data error");
-    erts_cleanup_offheap(&off_heap);
+    erts_factory_close(&factory);
     if (ctl != ctl_default) {
 	erts_free(ERTS_ALC_T_DCTRL_BUF, (void *) ctl);
     }
diff --git a/erts/emulator/beam/erl_map.h b/erts/emulator/beam/erl_map.h
index c391de3f11..4d9d74bc37 100644
--- a/erts/emulator/beam/erl_map.h
+++ b/erts/emulator/beam/erl_map.h
@@ -195,14 +195,17 @@ typedef struct hashmap_head_s {
    [one cons cell + one list term in parent node] per key
    [one header + one boxed term in parent node] per inner node
    [one header + one size word] for root node
+   Observed average number of nodes per key is about 0.35.
 */
-#define HASHMAP_HEAP_SIZE(KEYS,NODES) ((KEYS)*3 + (NODES)*2)
+#define HASHMAP_WORDS_PER_KEY 3
+#define HASHMAP_WORDS_PER_NODE 2
 #ifdef DEBUG
-#  define HASHMAP_ESTIMATED_NODE_COUNT(KEYS) (KEYS)
+#  define HASHMAP_ESTIMATED_TOT_NODE_SIZE(KEYS) \
+    (HASHMAP_WORDS_PER_NODE * (KEYS) * 3/10)   /* slightly under estimated */
 #else
-#  define HASHMAP_ESTIMATED_NODE_COUNT(KEYS) (2*(KEYS)/5)
+#  define HASHMAP_ESTIMATED_TOT_NODE_SIZE(KEYS) \
+    (HASHMAP_WORDS_PER_NODE * (KEYS) * 4/10)   /* slightly over estimated */
 #endif
 #define HASHMAP_ESTIMATED_HEAP_SIZE(KEYS) \
-        HASHMAP_HEAP_SIZE(KEYS,HASHMAP_ESTIMATED_NODE_COUNT(KEYS))
-
+        ((KEYS)*HASHMAP_WORDS_PER_KEY + HASHMAP_ESTIMATED_TOT_NODE_SIZE(KEYS))
 #endif
diff --git a/erts/emulator/beam/erl_message.c b/erts/emulator/beam/erl_message.c
index ef52823287..fa6b2fc613 100644
--- a/erts/emulator/beam/erl_message.c
+++ b/erts/emulator/beam/erl_message.c
@@ -1174,6 +1174,9 @@ void erts_factory_message_init(ErtsHeapFactory* factory,
     ASSERT(factory->hp >= factory->hp_start && factory->hp <= factory->hp_end);
 }
 
+/* One static sized heap that must suffice.
+   No extra heap fragments will be allocated.
+*/
 void erts_factory_static_init(ErtsHeapFactory* factory,
 			     Eterm* hp,
 			     Uint size,
@@ -1188,6 +1191,23 @@ void erts_factory_static_init(ErtsHeapFactory* factory,
     factory->off_heap_saved.overhead = factory->off_heap->overhead;
 }
 
+/* A temporary heap with default buffer allocated/freed by client.
+ * factory_close is same as factory_undo
+ */
+void erts_factory_tmp_init(ErtsHeapFactory* factory, Eterm* hp, Uint size,
+			   Uint32 atype)
+{
+    factory->mode     = FACTORY_TMP;
+    factory->hp_start = hp;
+    factory->hp       = hp;
+    factory->hp_end   = hp + size;
+    factory->heap_frags = NULL;
+    factory->off_heap_saved.first    = NULL;
+    factory->off_heap_saved.overhead = 0;
+    factory->off_heap = &factory->off_heap_saved;
+    factory->alloc_type = atype;
+}
+
 /* When we know the term is an immediate and need no heap.
 */
 void erts_factory_dummy_init(ErtsHeapFactory* factory)
@@ -1231,6 +1251,7 @@ static void reserve_heap(ErtsHeapFactory* factory, Uint need, Uint xtra)
 	return;
 
     case FACTORY_HEAP_FRAGS:
+    case FACTORY_TMP:
 	bp = factory->heap_frags;
 
         if (bp) {
@@ -1280,6 +1301,9 @@ void erts_factory_close(ErtsHeapFactory* factory)
 	    bp->used_size = factory->hp - bp->mem;
         }
 	break;
+    case FACTORY_TMP:
+	erts_factory_undo(factory);
+	break;
     case FACTORY_STATIC: break;
     case FACTORY_CLOSED: break;
     default:
@@ -1371,16 +1395,20 @@ void erts_factory_undo(ErtsHeapFactory* factory)
         }
         break;
 
+    case FACTORY_TMP:
     case FACTORY_HEAP_FRAGS:
+	erts_cleanup_offheap(factory->off_heap);
+	factory->off_heap->first = NULL;
+
         bp = factory->heap_frags;
-        do {
+        while (bp != NULL) {
             ErlHeapFragment* next_bp = bp->next;
 
-            erts_cleanup_offheap(&bp->off_heap);
+            ASSERT(bp->off_heap.first == NULL);
             ERTS_HEAP_FREE(factory->alloc_type, (void *) bp,
-                           ERTS_HEAP_FRAG_SIZE(bp->size));
+                           ERTS_HEAP_FRAG_SIZE(bp->alloc_size));
             bp = next_bp;
-        }while (bp != NULL);
+        }
 	break;
 
     case FACTORY_CLOSED: break;
diff --git a/erts/emulator/beam/erl_message.h b/erts/emulator/beam/erl_message.h
index fbdf3fb0e2..92ba3e571c 100644
--- a/erts/emulator/beam/erl_message.h
+++ b/erts/emulator/beam/erl_message.h
@@ -58,7 +58,8 @@ typedef struct {
         FACTORY_CLOSED = 0,
         FACTORY_HALLOC,
         FACTORY_HEAP_FRAGS,
-        FACTORY_STATIC
+        FACTORY_STATIC,
+        FACTORY_TMP
     } mode;
     Process* p;
     Eterm* hp_start;
@@ -75,6 +76,7 @@ void erts_factory_proc_init(ErtsHeapFactory*, Process*);
 void erts_factory_proc_prealloc_init(ErtsHeapFactory*, Process*, Sint size);
 void erts_factory_message_init(ErtsHeapFactory*, Process*, Eterm* hp, struct erl_heap_fragment*);
 void erts_factory_static_init(ErtsHeapFactory*, Eterm* hp, Uint size, ErlOffHeap*);
+void erts_factory_tmp_init(ErtsHeapFactory*, Eterm* hp, Uint size, Uint32 atype);
 void erts_factory_dummy_init(ErtsHeapFactory*);
 
 Eterm* erts_produce_heap(ErtsHeapFactory*, Uint need, Uint xtra);
diff --git a/erts/emulator/test/map_SUITE.erl b/erts/emulator/test/map_SUITE.erl
index 6890c42b7a..a256cf4195 100644
--- a/erts/emulator/test/map_SUITE.erl
+++ b/erts/emulator/test/map_SUITE.erl
@@ -2999,21 +2999,38 @@ id(I) -> I.
 t_gc_rare_map_overflow(Config) ->
     Pa = filename:dirname(code:which(?MODULE)),
     {ok, Node} = test_server:start_node(gc_rare_map_overflow, slave, [{args, "-pa \""++Pa++"\""}]),
-    Echo = spawn_link(Node, fun Loop() -> receive {From,Msg} -> From ! Msg
-					  end,
-					  Loop()
-			    end),
-    FatMap = fatmap(34),
-    false = (flatmap =:= erts_internal:map_type(FatMap)),
-
-    t_gc_rare_map_overflow_do(Echo, FatMap, fun() -> erlang:garbage_collect() end),
-
-    % Repeat test for minor gc:
-    minor_collect(), % need this to make the next gc really be a minor
-    t_gc_rare_map_overflow_do(Echo, FatMap, fun() -> true = minor_collect() end),
-
-    unlink(Echo),
-    test_server:stop_node(Node).
+    erts_debug:set_internal_state(available_internal_state, true),
+    try
+	Echo = spawn_link(Node, fun Loop() -> receive {From,Msg} -> From ! Msg
+					      end,
+					      Loop()
+				end),
+	FatMap = fatmap(34),
+	false = (flatmap =:= erts_internal:map_type(FatMap)),
+
+	t_gc_rare_map_overflow_do(Echo, FatMap, fun() -> erlang:garbage_collect() end),
+
+	%% Repeat test for minor gc:
+	t_gc_rare_map_overflow_do(Echo, FatMap, fun() -> minor_collect() end),
+
+	unlink(Echo),
+
+	%% Test fatmap in exit signal
+	Exiter = spawn_link(Node, fun Loop() -> receive {From,Msg} ->
+							"not_a_map" = Msg  % badmatch!
+						end,
+						Loop()
+				  end),
+	process_flag(trap_exit, true),
+	Exiter ! {self(), FatMap},
+	{'EXIT', Exiter, {{badmatch,FatMap}, _}} = receive M -> M end,
+	ok
+
+    after
+	process_flag(trap_exit, false),
+	erts_debug:set_internal_state(available_internal_state, false),
+	test_server:stop_node(Node)
+    end.
 
 t_gc_rare_map_overflow_do(Echo, FatMap, GcFun) ->
     Master = self(),
@@ -3033,15 +3050,11 @@ t_gc_rare_map_overflow_do(Echo, FatMap, GcFun) ->
     ok.
 
 minor_collect() ->
-    minor_collect(minor_gcs()).
-
-minor_collect(Before) ->
+    Before = minor_gcs(),
+    erts_debug:set_internal_state(force_gc, self()),
+    erlang:yield(),
     After = minor_gcs(),
-    case After of
-	_ when After > Before -> true;
-	_ when After =:= Before -> minor_collect(Before);
-	0 -> false
-    end.
+    io:format("minor_gcs: ~p -> ~p\n", [Before, After]).
 
 minor_gcs() ->
     {garbage_collection, Info} = process_info(self(), garbage_collection),
@@ -3051,7 +3064,7 @@ minor_gcs() ->
 %% Generate a map with N (or N+1) keys that has an abnormal heap demand.
 %% Done by finding keys that collide in the first 32-bit hash.
 fatmap(N) ->
-    erts_debug:set_internal_state(available_internal_state, true),
+    %%erts_debug:set_internal_state(available_internal_state, true),
     Table = ets:new(void, [bag, private]),
 
     Seed0 = rand:seed_s(exsplus, {4711, 3141592, 2718281}),