From 859c9788cab850c94a4f303450096682435bae2b Mon Sep 17 00:00:00 2001 From: Siri Hansen Date: Fri, 29 Apr 2011 10:05:01 +0200 Subject: Change list to set in supervisor for saving pids of dynamic temprary children Since initial arguments of temporary children under simple_one_for_one supervisors are not saved, only a list of pids was stored in such supervisors. When adding/deleting many children, this would scale badly. To avoid this the list is now changed to a set. --- lib/stdlib/src/supervisor.erl | 34 +++++++++++++++--------- lib/stdlib/test/supervisor_SUITE.erl | 51 +++++++++++++++++++++++++++++++++--- 2 files changed, 68 insertions(+), 17 deletions(-) diff --git a/lib/stdlib/src/supervisor.erl b/lib/stdlib/src/supervisor.erl index 4fd7f1d47c..09a01a9aea 100644 --- a/lib/stdlib/src/supervisor.erl +++ b/lib/stdlib/src/supervisor.erl @@ -65,11 +65,13 @@ -type child() :: #child{}. -define(DICT, dict). +-define(SETS, sets). +-define(SET, set). -record(state, {name, strategy :: strategy(), children = [] :: [child()], - dynamics :: ?DICT() | list(), + dynamics :: ?DICT() | ?SET(), intensity :: non_neg_integer(), period :: pos_integer(), restarts = [], @@ -363,8 +365,8 @@ handle_call(which_children, _From, #state{children = [#child{restart_type = temp child_type = CT, modules = Mods}]} = State) when ?is_simple(State) -> - Reply = lists:map(fun(Pid) -> {undefined, Pid, CT, Mods} end, dynamics_db(temporary, - State#state.dynamics)), + Reply = lists:map(fun(Pid) -> {undefined, Pid, CT, Mods} end, + ?SETS:to_list(dynamics_db(temporary, State#state.dynamics))), {reply, Reply, State}; handle_call(which_children, _From, #state{children = [#child{restart_type = RType, @@ -389,7 +391,7 @@ handle_call(count_children, _From, #state{children = [#child{restart_type = temp child_type = CT}]} = State) when ?is_simple(State) -> {Active, Count} = - lists:foldl(fun(Pid, {Alive, Tot}) -> + ?SETS:fold(fun(Pid, {Alive, Tot}) -> if is_pid(Pid) -> {Alive+1, Tot +1}; true -> {Alive, Tot + 1} end end, {0, 0}, dynamics_db(temporary, State#state.dynamics)), @@ -800,24 +802,27 @@ save_child(Child, #state{children = Children} = State) -> State#state{children = [Child |Children]}. save_dynamic_child(temporary, Pid, _, #state{dynamics = Dynamics} = State) -> - State#state{dynamics = [Pid | dynamics_db(temporary, Dynamics)]}; + State#state{dynamics = ?SETS:add_element(Pid, dynamics_db(temporary, Dynamics))}; save_dynamic_child(RestartType, Pid, Args, #state{dynamics = Dynamics} = State) -> State#state{dynamics = ?DICT:store(Pid, Args, dynamics_db(RestartType, Dynamics))}. dynamics_db(temporary, undefined) -> - []; + ?SETS:new(); dynamics_db(_, undefined) -> ?DICT:new(); dynamics_db(_,Dynamics) -> Dynamics. -dynamic_child_args(_, Dynamics) when is_list(Dynamics)-> - {ok, undefined}; dynamic_child_args(Pid, Dynamics) -> - ?DICT:find(Pid, Dynamics). + case ?SETS:is_set(Dynamics) of + true -> + {ok, undefined}; + false -> + ?DICT:find(Pid, Dynamics) + end. state_del_child(#child{pid = Pid, restart_type = temporary}, State) when ?is_simple(State) -> - NDynamics = lists:delete(Pid, dynamics_db(temporary, State#state.dynamics)), + NDynamics = ?SETS:del_element(Pid, dynamics_db(temporary, State#state.dynamics)), State#state{dynamics = NDynamics}; state_del_child(#child{pid = Pid, restart_type = RType}, State) when ?is_simple(State) -> NDynamics = ?DICT:erase(Pid, dynamics_db(RType, State#state.dynamics)), @@ -871,10 +876,13 @@ get_dynamic_child(Pid, #state{children=[Child], dynamics=Dynamics}) -> end end. -is_dynamic_pid(Pid, Dynamics) when is_list(Dynamics) -> - lists:member(Pid, Dynamics); is_dynamic_pid(Pid, Dynamics) -> - dict:is_key(Pid, Dynamics). + case ?SETS:is_set(Dynamics) of + true -> + ?SETS:is_element(Pid, Dynamics); + false -> + ?DICT:is_key(Pid, Dynamics) + end. replace_child(Child, State) -> Chs = do_replace_child(Child, State#state.children), diff --git a/lib/stdlib/test/supervisor_SUITE.erl b/lib/stdlib/test/supervisor_SUITE.erl index cc271bd047..c79a5002fb 100644 --- a/lib/stdlib/test/supervisor_SUITE.erl +++ b/lib/stdlib/test/supervisor_SUITE.erl @@ -28,8 +28,8 @@ init_per_group/2,end_per_group/2, init_per_testcase/2, end_per_testcase/2]). -%% Indirect spawn export --export([init/1]). +%% Internal export +-export([init/1, terminate_all_children/1]). %% API tests -export([ sup_start_normal/1, sup_start_ignore_init/1, @@ -55,7 +55,8 @@ %% Misc tests -export([child_unlink/1, tree/1, count_children_memory/1, do_not_save_start_parameters_for_temporary_children/1, - do_not_save_child_specs_for_temporary_children/1]). + do_not_save_child_specs_for_temporary_children/1, + simple_one_for_one_scale_many_temporary_children/1]). %%------------------------------------------------------------------------- @@ -72,7 +73,8 @@ all() -> {group, normal_termination}, {group, abnormal_termination}, child_unlink, tree, count_children_memory, do_not_save_start_parameters_for_temporary_children, - do_not_save_child_specs_for_temporary_children]. + do_not_save_child_specs_for_temporary_children, + simple_one_for_one_scale_many_temporary_children]. groups() -> [{sup_start, [], @@ -1201,6 +1203,47 @@ restarted(Sup, {Id,_,_,_,_,_} = ChildSpec, TerminateHow) -> {error, {already_started, _}} = supervisor:start_child(Sup, ChildSpec). +%%------------------------------------------------------------------------- +%% OTP-9242: Pids for dynamic temporary children were saved as a list, +%% which caused bad scaling when adding/deleting many processes. +simple_one_for_one_scale_many_temporary_children(_Config) -> + process_flag(trap_exit, true), + Child = {child, {supervisor_1, start_child, []}, temporary, 1000, + worker, []}, + {ok, _SupPid} = start_link({ok, {{simple_one_for_one, 2, 3600}, [Child]}}), + + C1 = [begin + {ok,P} = supervisor:start_child(sup_test,[]), + P + end || _<- lists:seq(1,1000)], + {T1,done} = timer:tc(?MODULE,terminate_all_children,[C1]), + + C2 = [begin + {ok,P} = supervisor:start_child(sup_test,[]), + P + end || _<- lists:seq(1,10000)], + {T2,done} = timer:tc(?MODULE,terminate_all_children,[C2]), + + Scaling = T2 div T1, + if Scaling > 20 -> + %% The scaling shoul be linear (i.e.10, really), but we + %% give some extra here to avoid failing the test + %% unecessarily. + ?t:fail({bad_scaling,Scaling}); + true -> + ok + end. + + +terminate_all_children([C|Cs]) -> + ok = supervisor:terminate_child(sup_test,C), + terminate_all_children(Cs); +terminate_all_children([]) -> + done. + + + +%%------------------------------------------------------------------------- terminate(Pid, Reason) when Reason =/= supervisor -> terminate(dummy, Pid, dummy, Reason). -- cgit v1.2.3