Fix hibernation subtlety

3 files changed, 113 insertions, 23 deletions

diff --git a/lib/stdlib/doc/src/gen_statem.xml b/lib/stdlib/doc/src/gen_statem.xml
index 91332fbdde..ec7f267c64 100644
--- a/lib/stdlib/doc/src/gen_statem.xml
+++ b/lib/stdlib/doc/src/gen_statem.xml
@@ -216,19 +216,23 @@ erlang:'!'            -----> Module:StateName/3
       if bad arguments are given.
     </p>
     <p>
-      The <c>gen_statem</c> process can go into hibernation (see
-      <seealso marker="erts:erlang#hibernate/3">
-	<c>erlang:hibernate/3</c>
-      </seealso>) if a
+      The <c>gen_statem</c> process can go into hibernation; see
+      <seealso marker="proc_lib#hibernate/3">
+	<c>proc_lib:hibernate/3</c>.
+      </seealso>
+      It is done when a
       <seealso marker="#state_function">state function</seealso> or
       <seealso marker="#Module:init/1"><c>Module:init/1</c></seealso>
       specifies <c>hibernate</c> in the returned
       <seealso marker="#type-action"><c>Actions</c></seealso>
-      list. This might be useful if the server is expected to be idle
-      for a long time. However use this feature with care
-      since hibernation implies at least two garbage collections
-      (when hibernating and shortly after waking up) and that is not
-      something you would want to do between each event on a busy server.
+      list. This feature might be useful to reclaim process heap memory
+      while the server is expected to be idle for a long time.
+      However, use this feature with care
+      since hibernation can be too costly
+      to use after every event; see
+      <seealso marker="erts:erlang#hibernate/3">
+	<c>erlang:hibernate/3</c>.
+      </seealso>
     </p>
   </description>
 
@@ -619,7 +623,8 @@ handle_event(_, _, State, Data) ->
 	    to wait for the next message.  In hibernation the next
 	    non-system event awakens the <c>gen_statem</c>, or rather
 	    the next incoming message awakens the <c>gen_statem</c>
-	    but if it is a system event it goes back into hibernation.
+	    but if it is a system event
+	    it goes right back into hibernation.
 	  </item>
 	</list>
       </desc>
@@ -645,9 +650,13 @@ handle_event(_, _, State, Data) ->
 	  </seealso>
 	  before going into <c>receive</c>
 	  to wait for a new external event.
-	  If there are enqueued events the <c>hibernate</c>
-	  is ignored as if an event just arrived and awakened
-	  the <c>gen_statem</c>.
+	  If there are enqueued events,
+	  to prevent receiving any new event; a
+	  <seealso marker="erts:erlang#garbage_collect/0">
+	    <c>garbage_collect/0</c>
+	  </seealso> is done instead to simulate
+	  that the <c>gen_statem</c> entered hibernation
+	  and immediately got awakened by the oldest enqueued event.
 	</p>
       </desc>
     </datatype>
diff --git a/lib/stdlib/src/gen_statem.erl b/lib/stdlib/src/gen_statem.erl
index c85c521d8e..f9e2e5f7d2 100644
--- a/lib/stdlib/src/gen_statem.erl
+++ b/lib/stdlib/src/gen_statem.erl
@@ -723,8 +723,8 @@ wakeup_from_hibernate(Parent, Debug, S) ->
 %% and some detours through sys and proc_lib
 
 %% Entry point for system_continue/3
-loop(Parent, Debug, #{hibernate := Hib} = S) ->
-    case Hib of
+loop(Parent, Debug, #{hibernate := Hibernate} = S) ->
+    case Hibernate of
 	true ->
 	    %% Does not return but restarts process at
 	    %% wakeup_from_hibernate/3 that jumps to loop_receive/3
@@ -754,8 +754,7 @@ loop_receive(Parent, Debug, #{timer := Timer} = S) ->
 		    %% but this will stand out in the crash report...
 		    ?TERMINATE(exit, Reason, Debug, S, [EXIT]);
 		{timeout,Timer,Content} when Timer =/= undefined ->
-		    loop_event(
-		      Parent, Debug, S, {timeout,Content});
+		    loop_event(Parent, Debug, S, {timeout,Content});
 		_ ->
 		    %% Cancel Timer if running
 		    case Timer of
@@ -788,15 +787,15 @@ loop_receive(Parent, Debug, #{timer := Timer} = S) ->
     end.
 
 loop_event(Parent, Debug, S, Event) ->
-    %% The timer field in S is now invalid and ignored
-    %% until we get back to loop/3
+    %% The timer field and the hibernate flag in S
+    %% are now invalid and ignored until we get back to loop/3
     NewDebug = sys_debug(Debug, S, {in,Event}),
     %% Here the queue of not yet processed events is created
-    loop_events(Parent, NewDebug, S, [Event]).
+    loop_events(Parent, NewDebug, S, [Event], false).
 
 %% Process first the event queue, or if it is empty
 %% loop back to receive a new event
-loop_events(Parent, Debug, S, []) ->
+loop_events(Parent, Debug, S, [], _Hibernate) ->
     loop(Parent, Debug, S);
 loop_events(
   Parent, Debug,
@@ -804,7 +803,16 @@ loop_events(
     module := Module,
     state := State,
     data := Data} = S,
-  [{Type,Content} = Event|Events] = Q) ->
+  [{Type,Content} = Event|Events] = Q,
+  Hibernate) ->
+    %% If the Hibernate flag is true here it can only be
+    %% because it was set from an event action
+    %% and we did not go into hibernation since there
+    %% were events in queue, so we do what the user
+    %% might depend on i.e collect garbage which
+    %% would have happened if we actually hibernated
+    %% and immediately was awakened
+    Hibernate andalso garbage_collect(),
     try
 	case CallbackMode of
 	    state_functions ->
@@ -869,6 +877,8 @@ loop_event_result(
   Parent, Debug,
   #{state := State, data := Data} = S,
   Events, Event, Result) ->
+    %% From now until we loop back to the loop_events/4
+    %% the state and data fields in S are old
     case Result of
 	stop ->
 	    ?TERMINATE(exit, normal, Debug, S, [Event|Events]);
@@ -1094,7 +1104,7 @@ loop_event_actions(
 	timer := Timer,
 	postponed := P,
 	hibernate := Hibernate},
-      Q).
+      Q, Hibernate).
 
 %%---------------------------------------------------------------------------
 %% Server helpers
diff --git a/system/doc/design_principles/statem.xml b/system/doc/design_principles/statem.xml
index a4b8fb06a0..ca0fce55e2 100644
--- a/system/doc/design_principles/statem.xml
+++ b/system/doc/design_principles/statem.xml
@@ -1448,4 +1448,75 @@ format_status(Opt, [_PDict,State,Data]) ->
     </p>
   </section>
 
+<!-- =================================================================== -->
+
+  <section>
+    <title>Hibernation</title>
+    <p>
+      If you have many servers in one node
+      and they have some state(s) in their lifetime in which
+      the servers can be expected to idle for a while,
+      and the amount of heap memory all these servers need
+      is a problem; then it is possible to minimize
+      the memory footprint of a server by hibernating it through
+      <seealso marker="stdlib:proc_lib#hibernate/3">
+	<c>proc_lib:hibernate/3</c>.
+      </seealso>
+    </p>
+    <note>
+      <p>
+	To hibernate a process is rather costly. See
+	<seealso marker="erts:erlang#hibernate/3">
+	  <c>erlang:hibernate/3</c>.
+	</seealso>
+	It is in general not something you want to do
+	after every event.
+      </p>
+    </note>
+    <p>
+      We can in this example hibernate in the <c>{open,_}</c> state
+      since what normally happens in that state is that
+      the state timeout after a while
+      triggers a transition to <c>{locked,_}</c>:
+    </p>
+    <code type="erl"><![CDATA[
+...
+handle_event(
+  EventType, EventContent,
+  {open,LockButton}, #{timer := Timer} = Data) ->
+    case {EventType,EventContent} of
+	{internal,enter} ->
+	    Tref = erlang:start_timer(10000, self(), lock),
+	    do_unlock(),
+	    {keep_state,Data#{timer := Tref},[hibernate]};
+...
+    ]]></code>
+    <p>
+      The
+      <seealso marker="stdlib:gen_statem#type-hibernate">
+	<c>[hibernate]</c>
+      </seealso>
+      action list on the last line
+      when entering the <c>{open,_}</c> state is the only change.
+      If any event arrives in the <c>{open,_},</c> state we
+      do not bother to re-hibernate, so the server stays
+      awake after any event.
+    </p>
+    <p>
+      To change that we would need to insert
+      the <c>hibernate</c> action in more places,
+      for example for the state independent <c>set_lock_button</c>
+      and <c>code_length</c> operations that then would have to
+      be aware of using <c>hibernate</c> while in the
+      <c>{open,_}</c> state which would clutter the code.
+    </p>
+    <p>
+      This server probably does not use an amount of
+      heap memory worth hibernating for.
+      To gain anything from hibernation your server would
+      have to actually produce some garbage during callback execution,
+      for which this example server may serve as a bad example.
+    </p>
+  </section>
+
 </chapter>