From a51fea68bbcf341bf7857a6bd3f0c17ffd0402bb Mon Sep 17 00:00:00 2001 From: Ingela Anderton Andin Date: Tue, 15 Feb 2011 16:51:38 +0100 Subject: Do not save parameter list for any temporary processes Previous commit changed the supervisor to not save parameter lists for temporary processes supervised by simple-one-for-one supervisors. But it is unnecessary to save them for any temporary processes as they should not be restarted. Proably the biggest gain is in the simple-one-for-one case. Also changed the test case count_children_memory so it does not test that which_children will produce garbage that must be reclaimed later. This is a strange thing to test and it is no longer true for all invocations of which_children. --- lib/stdlib/src/supervisor.erl | 25 ++++++++++++++++--------- lib/stdlib/test/supervisor_SUITE.erl | 20 ++++---------------- 2 files changed, 20 insertions(+), 25 deletions(-) diff --git a/lib/stdlib/src/supervisor.erl b/lib/stdlib/src/supervisor.erl index bfa7c96eca..469063689d 100644 --- a/lib/stdlib/src/supervisor.erl +++ b/lib/stdlib/src/supervisor.erl @@ -562,15 +562,11 @@ handle_start_child(Child, State) -> false -> case do_start_child(State#state.name, Child) of {ok, Pid} -> - Children = State#state.children, {{ok, Pid}, - State#state{children = - [Child#child{pid = Pid}|Children]}}; + save_child(Child#child{pid = Pid}, State)}; {ok, Pid, Extra} -> - Children = State#state.children, {{ok, Pid, Extra}, - State#state{children = - [Child#child{pid = Pid}|Children]}}; + save_child(Child#child{pid = Pid}, State)}; {error, What} -> {{error, {What, Child}}, State} end; @@ -593,13 +589,13 @@ restart_child(Pid, Reason, #state{children = [Child]} = State) when ?is_simple(S NChild = Child#child{pid = Pid, mfargs = {M, F, Args}}, do_restart(RestartType, Reason, NChild, State); error -> - {ok, State} + {ok, State} end; + restart_child(Pid, Reason, State) -> Children = State#state.children, case lists:keyfind(Pid, #child.pid, Children) of - #child{} = Child -> - RestartType = Child#child.restart_type, + #child{restart_type = RestartType} = Child -> do_restart(RestartType, Reason, Child, State); false -> {ok, State} @@ -783,6 +779,17 @@ monitor_child(Pid) -> %% Child/State manipulating functions. %%----------------------------------------------------------------- +%% Note we do not want to save the parameter list for temporary processes as +%% they will not be restarted, and hence we do not need this information. +%% Especially for dynamic children to simple_one_for_one supervisors +%% it could become very costly as it is not uncommon to spawn +%% very many such processes. +save_child(#child{restart_type = temporary, + mfargs = {M, F, _}} = Child, #state{children = Children} = State) -> + State#state{children = [Child#child{mfargs = {M, F, undefined}} |Children]}; +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)]}; save_dynamic_child(RestartType, Pid, Args, #state{dynamics = Dynamics} = State) -> diff --git a/lib/stdlib/test/supervisor_SUITE.erl b/lib/stdlib/test/supervisor_SUITE.erl index 039ea298c4..4708658a6d 100644 --- a/lib/stdlib/test/supervisor_SUITE.erl +++ b/lib/stdlib/test/supervisor_SUITE.erl @@ -1,7 +1,7 @@ %% %% %CopyrightBegin% %% -%% Copyright Ericsson AB 1996-2010. All Rights Reserved. +%% Copyright Ericsson AB 1996-2011. All Rights Reserved. %% %% The contents of this file are subject to the Erlang Public License, %% Version 1.1, (the "License"); you may not use this file except in @@ -1279,7 +1279,7 @@ count_children_allocator_test(MemoryState) -> lists:all(fun(State) -> State == {e, true} end, AllocStates). count_children_memory(doc) -> - ["Test that which_children eats memory, but count_children does not."]; + ["Test that count_children does not eat memory."]; count_children_memory(suite) -> MemoryState = erlang:system_info(allocator), case count_children_allocator_test(MemoryState) of @@ -1323,8 +1323,8 @@ count_children_memory(Config) when is_list(Config) -> ?line ChildCount3 = ChildCount2, %% count_children consumes memory using an accumulator function, - %% but the space can be reclaimed incrementally, whereas - %% which_children generates a return list. + %% but the space can be reclaimed incrementally, + %% which_children may generate garbage that will reclaimed later. case (Size5 =< Size4) of true -> ok; false -> @@ -1336,19 +1336,7 @@ count_children_memory(Config) when is_list(Config) -> ?line test_server:fail({count_children, used_more_memory}) end, - case Size4 > Size3 of - true -> ok; - false -> - ?line test_server:fail({which_children, used_no_memory}) - end, - case Size6 > Size5 of - true -> ok; - false -> - ?line test_server:fail({which_children, used_no_memory}) - end, - [exit(Pid, kill) || {undefined, Pid, worker, _Modules} <- Children3], test_server:sleep(100), ?line [1,0,0,0] = get_child_counts(sup_test), - ok. -- cgit v1.2.3