aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorRaimo Niskanen <[email protected]>2019-01-17 16:08:01 +0100
committerRaimo Niskanen <[email protected]>2019-01-17 16:08:01 +0100
commit12cecd07628cb28d7bc80c17bfc01ca783b8683c (patch)
treef6c6adb6492eb4f665d3e1e0b3acdde2ad0bad04
parent7c0df9ac2879e5d5bcdf511fc81b1465105c4b61 (diff)
downloadotp-12cecd07628cb28d7bc80c17bfc01ca783b8683c.tar.gz
otp-12cecd07628cb28d7bc80c17bfc01ca783b8683c.tar.bz2
otp-12cecd07628cb28d7bc80c17bfc01ca783b8683c.zip
Write some more comments in the engine loop
-rw-r--r--lib/stdlib/src/gen_statem.erl193
1 files changed, 155 insertions, 38 deletions
diff --git a/lib/stdlib/src/gen_statem.erl b/lib/stdlib/src/gen_statem.erl
index beb37e43e0..2c03b4fff6 100644
--- a/lib/stdlib/src/gen_statem.erl
+++ b/lib/stdlib/src/gen_statem.erl
@@ -815,7 +815,10 @@ format_status(
T -> [T]
end].
-
+%% Update #params.parent only if it differs. This should not
+%% be possible today (OTP-22.0), but could happen for example
+%% if someone implements changing a server's parent
+%% in a new sys call.
-compile({inline, update_parent/2}).
update_parent(P, Parent) ->
case P of
@@ -885,17 +888,31 @@ wakeup_from_hibernate(P, Debug, S) ->
loop_receive(P, Debug, S).
%%%==========================================================================
-%%% State Machine engine implementation of proc_lib/gen server
+%%% State Machine engine implementation on proc_lib/gen
%% Server loop, consists of all loop* functions
%% and detours through sys:handle_system_message/7 and proc_lib:hibernate/3
+%%
+%% The loop tries to keep all temporary values in arguments
+%% and takes shortcuts for ?not_sys_debug, empty lists, etc.
+%% The engine state #state{} is picked apart during the loop,
+%% new values are kept in arguments, and a new #state{} is
+%% composed at the end of the loop. #params{} collect engine
+%% state fields that rarely changes.
+%%
+%% The loop is optimized a bit for staying in the loop, assuming that
+%% system events are rare. So a detour to sys requires re-packing
+%% of the engine state.
%% Entry point for system_continue/3
+%%
loop(P, Debug, #state{hibernate = true} = S) ->
loop_hibernate(P, Debug, S);
loop(P, Debug, S) ->
loop_receive(P, Debug, S).
+%% Go to hibernation
+%%
loop_hibernate(P, Debug, S) ->
%%
%% Does not return but restarts process at
@@ -908,6 +925,9 @@ loop_hibernate(P, Debug, S) ->
%% Entry point for wakeup_from_hibernate/3
+%%
+%% Receive a new process message
+%%
loop_receive(
#params{hibernate_after = HibernateAfterTimeout} = P, Debug, S) ->
%%
@@ -958,6 +978,8 @@ loop_receive(
loop_hibernate(P, Debug, S)
end.
+%% We have received an event
+%%
loop_receive_result(P, ?not_sys_debug = Debug, S, Event) ->
%% Here is the queue of not yet handled events created
Events = [],
@@ -976,7 +998,8 @@ loop_receive_result(
Events = [],
loop_event(P, Debug_1, S, Event, Events).
-%% Entry point for handling an event; received or enqueued
+%% Handle one event; received or enqueued
+%%
loop_event(
P, Debug, #state{hibernate = true} = S, Event, Events) ->
%%
@@ -993,8 +1016,9 @@ loop_event(
loop_event(P, Debug, S, Event, Events) ->
loop_event_handler(P, Debug, S, Event, Events).
+%% Call the state function, eventually
+%%
-compile({inline, [loop_event_handler/5]}).
-%% Call the state function
loop_event_handler(
P, Debug, #state{state_data = State_Data} = S, Event, Events) ->
%%
@@ -1004,6 +1028,8 @@ loop_event_handler(
Q = [Event|Events],
loop_callback_mode(P, Debug, S, Q, State_Data, Event).
+%% Figure out the callback mode
+%%
loop_callback_mode(
#params{callback_mode = undefined} = P, Debug, S,
Q, State_Data, CallbackEvent) ->
@@ -1028,6 +1054,8 @@ loop_callback_mode(
loop_callback_mode(P, Debug, S, Q, State_Data, CallbackEvent) ->
loop_state_callback(P, Debug, S, Q, State_Data, CallbackEvent).
+%% Check the result of Module:callback_mode()
+%%
loop_callback_mode_result(
P, Debug, S, Q, State_Data, CallbackEvent,
CallbackMode, [H|T], NewCallbackMode, NewStateEnter) ->
@@ -1072,7 +1100,9 @@ loop_callback_mode_result(
end.
-%% Make a state enter call to the state function
+%% Make a state enter call to the state function, we loop back here
+%% from further down if state enter calls are enabled
+%%
loop_state_enter(
P, Debug, #state{state_data = {PrevState,_PrevData}} = S,
Q, NextState_NewData,
@@ -1085,6 +1115,8 @@ loop_state_enter(
NextEventsR, Hibernate, TimeoutsR, Postpone,
StateCall, CallbackEvent).
+%% Make a state call (not state enter call) to the state function
+%%
loop_state_callback(P, Debug, S, Q, State_Data, CallbackEvent) ->
NextEventsR = [],
Hibernate = false,
@@ -1134,9 +1166,7 @@ loop_state_callback(
NextEventsR, Hibernate, TimeoutsR, Postpone,
CallEnter, StateCall, Actions).
-%% Process the result from the state function.
-%% When TransOpts =:= trans_opts it was a state function call,
-%% otherwise it is an option tuple and it was a state enter call.
+%% Process the result from the state function
%%
loop_state_callback_result(
P, Debug, S, Q, {State,_Data} = State_Data,
@@ -1278,6 +1308,7 @@ loop_state_callback_result(
end.
%% Ensure that Actions are a list
+%%
loop_actions(
P, Debug, S, Q, NextState_NewData,
NextEventsR, Hibernate, TimeoutsR, Postpone,
@@ -1315,6 +1346,8 @@ loop_actions(
NextEventsR, Hibernate, TimeoutsR, Postpone)
end.
+%% Process the returned actions
+%%
loop_actions_list(
P, Debug, S, Q, NextState_NewData,
NextEventsR, Hibernate, TimeoutsR, Postpone,
@@ -1389,6 +1422,8 @@ loop_actions_list(
CallEnter, StateCall, Actions, Timeout)
end.
+%% Process a reply action
+%%
loop_actions_reply(
P, Debug, S, Q, NextState_NewData,
NextEventsR, Hibernate, TimeoutsR, Postpone,
@@ -1417,6 +1452,8 @@ loop_actions_reply(
Q)
end.
+%% Process a next_event action
+%%
loop_actions_next_event(
P, Debug, S, Q, NextState_NewData,
NextEventsR, Hibernate, TimeoutsR, Postpone,
@@ -1445,7 +1482,12 @@ loop_actions_next_event(
_ ->
terminate(
error,
- {bad_state_enter_action_from_state_function,
+ {if
+ StateCall ->
+ bad_action_from_state_function;
+ true ->
+ bad_state_enter_action_from_state_function
+ end,
{next_event,Type,Content}},
?STACKTRACE(), P, Debug,
S#state{
@@ -1454,6 +1496,8 @@ loop_actions_next_event(
Q)
end.
+%% Process a timeout action, or also any unrecognized action
+%%
loop_actions_timeout(
P, Debug, S, Q, NextState_NewData,
NextEventsR, Hibernate, TimeoutsR, Postpone,
@@ -1630,6 +1674,8 @@ loop_state_transition(
end
end.
+%% State transition to the same state
+%%
loop_keep_state(
P, Debug, #state{timers = {TimerRefs,TimeoutTypes} = Timers} = S,
Events, NextState_NewData,
@@ -1657,6 +1703,8 @@ loop_keep_state(
Timers)
end.
+%% State transition to a different state
+%%
loop_state_change(
P, Debug, S, Events, NextState_NewData,
NextEventsR, Hibernate, TimeoutsR, Postponed) ->
@@ -1723,6 +1771,9 @@ loop_state_change(
Timers)
end.
+%% Continue state transition with processing of
+%% inserted events and timeout events
+%%
loop_next_events(
P, Debug, S,
Events, NextState_NewData,
@@ -1754,6 +1805,8 @@ loop_next_events(
NextEventsR, Hibernate, TimeoutsR, Postponed,
Timers, Seen, TimeoutEvents).
+%% Continue state transition with processing of timeout events
+%%
loop_timeouts(
P, Debug, S,
Events, NextState_NewData,
@@ -1838,48 +1891,29 @@ loop_timeouts(
P, Debug, S,
Events, NextState_NewData,
NextEventsR, Hibernate, TimeoutsR, Postponed,
- {TimerRefs,TimeoutTypes} = Timers, Seen, TimeoutEvents,
+ Timers, Seen, TimeoutEvents,
TimeoutType, Time, TimeoutMsg, TimeoutOpts) ->
%%
case Time of
infinity ->
%% Cancel any running timer
- Timers_1 = cancel_timer_by_type(TimeoutType, Timers),
- loop_timeouts(
+ loop_timeouts_cancel(
P, Debug, S,
Events, NextState_NewData,
NextEventsR, Hibernate, TimeoutsR, Postponed,
- Timers_1, Seen#{TimeoutType => true}, TimeoutEvents);
+ Timers, Seen, TimeoutEvents,
+ TimeoutType);
0 when TimeoutOpts =:= [] ->
%% Relative timeout zero
%% - cancel any running timer
%% handle timeout zero events later
%%
- %% Instead of using cancel_timer_by_type/2 here, duplicate
- %% the code, so we can make the function exit differ,
- %% so the common subexpression optimization does not
- %% kick in and accidentally force spilling all live
- %% registers for the no TimeoutType match path
- case TimeoutTypes of
- #{TimeoutType := TimerRef} ->
- Timers_1 =
- cancel_timer_by_ref_and_type(
- TimerRef, TimeoutType, TimerRefs, TimeoutTypes),
- loop_timeouts(
- P, Debug, S,
- Events, NextState_NewData,
- NextEventsR, Hibernate, TimeoutsR, Postponed,
- Timers_1, Seen,
- [{TimeoutType,TimeoutMsg}|TimeoutEvents],
- TimeoutType);
- #{} ->
- loop_timeouts(
- P, Debug, S,
- Events, NextState_NewData,
- NextEventsR, Hibernate, TimeoutsR, Postponed,
- Timers, Seen#{TimeoutType => true},
- [{TimeoutType,TimeoutMsg}|TimeoutEvents])
- end;
+ loop_timeouts_cancel(
+ P, Debug, S,
+ Events, NextState_NewData,
+ NextEventsR, Hibernate, TimeoutsR, Postponed,
+ Timers, Seen, [{TimeoutType,TimeoutMsg}|TimeoutEvents],
+ TimeoutType);
_ ->
%% (Re)start the timer
TimerRef =
@@ -1915,6 +1949,11 @@ loop_timeouts(
end
end.
%%
+%% Helper function for the ugly mis-optimization workaround
+%% loop_timeouts_cancel below
+%%
+%% Do not inline!
+%%
loop_timeouts(
P, Debug, S,
Events, NextState_NewData,
@@ -1928,11 +1967,83 @@ loop_timeouts(
NextEventsR, Hibernate, TimeoutsR, Postponed,
Timers, Seen#{TimeoutType => true}, TimeoutEvents).
+%% Loop helper to cancel a timeout
+%%
+loop_timeouts_cancel(
+ P, Debug, S,
+ Events, NextState_NewData,
+ NextEventsR, Hibernate, TimeoutsR, Postponed,
+ {TimerRefs,TimeoutTypes} = Timers, Seen, TimeoutEvents,
+ TimeoutType) ->
+ %% Ugly mis-optimization workaround
+ %%
+ %% This function body should have been:
+ %% Timers_1 = cancel_timer_by_type(TimeoutType, Timers),
+ %% loop_timeouts(
+ %% P, Debug, S,
+ %% Events, NextState_NewData,
+ %% NextEventsR, Hibernate, TimeoutsR, Postponed,
+ %% Timers_1, Seen#{TimeoutType => true}, TimeoutEvents).
+ %%
+ %% Since cancel_timer_by_type is inlined there is a code path
+ %% that checks if TimeoutType is a key in the map TimeoutTypes
+ %% and if not simply makes Timers_1 = Timers and then does
+ %% the map update of Seen and loops back to loop_timeouts/12.
+ %% This code path does not contain any external call and the
+ %% map update is an instruction, so that should be a simple
+ %% and fast path.
+ %%
+ %% The other code path when TimeoutType is a key in the map
+ %% TimeoutTypes calls erlang:cancel_timer/1 which forces
+ %% all live registers (about 13 of them) out on the stack
+ %% and back again afterwards.
+ %%
+ %% Unfortunately the optimization of common subexpressions
+ %% sees that both these function exits are identical and
+ %% joins them. Then during register allocation the common
+ %% function exit is adapted for the code path that spills
+ %% all live registers to the stack. So also the simple
+ %% and fast path spills all live registers around its
+ %% map update... Bummer!
+ %%
+ %% So this workaround duplicates cancel_timer_by_type/2 here,
+ %% and makes the function exits differ - the slow case
+ %% calls a helper function above to update Seen, and
+ %% the fast case updates Seen in this function. This tricks
+ %% the common subexpression optimizer into not joining
+ %% these two code paths.
+ %%
+ %% So the helper function above must not be inlined, please!
+ %%
+ case TimeoutTypes of
+ #{TimeoutType := TimerRef} ->
+ Timers_1 =
+ cancel_timer_by_ref_and_type(
+ TimerRef, TimeoutType, TimerRefs, TimeoutTypes),
+ loop_timeouts(
+ P, Debug, S,
+ Events, NextState_NewData,
+ NextEventsR, Hibernate, TimeoutsR, Postponed,
+ Timers_1, Seen, TimeoutEvents,
+ TimeoutType);
+ #{} ->
+ loop_timeouts(
+ P, Debug, S,
+ Events, NextState_NewData,
+ NextEventsR, Hibernate, TimeoutsR, Postponed,
+ Timers, Seen#{TimeoutType => true}, TimeoutEvents)
+ end.
+
+%% Continue state transition with prepending timeout zero events
+%% before event queue reversal i.e appending timeout zero events
+%%
loop_prepend_timeout_events(P, Debug, S, TimeoutEvents, EventsR) ->
{Debug_1,Events_1R} =
prepend_timeout_events(P, Debug, S, TimeoutEvents, EventsR),
loop_done(P, Debug_1, S, Events_1R, []).
+%% Place inserted events first in the event queue
+%%
loop_done(P, Debug, S, NextEventsR, Events) ->
case NextEventsR of
[] ->
@@ -1945,6 +2056,9 @@ loop_done(P, Debug, S, NextEventsR, Events) ->
loop_done(P, Debug, S, lists:reverse(NextEventsR, Events))
end.
%%
+%% State transition is done, keep looping if there are
+%% enqueued events, otherwise get a new event
+%%
loop_done(P, Debug, S, Q) ->
%%% io:format(
%%% "loop_done: state_data = ~p,~n"
@@ -1963,6 +2077,9 @@ loop_done(P, Debug, S, Q) ->
%%---------------------------------------------------------------------------
%% Server loop helpers
+%% Parse an option list for erlang:start_timer/4 to figure out
+%% if the timeout will be absolute or relative
+%%
parse_timeout_opts_abs(Opts) ->
parse_timeout_opts_abs(Opts, false).
%%