Merge branch 'ia/supervisor-saves-unnecessary-data/OTP-9064' into dev

* ia/supervisor-saves-unnecessary-data/OTP-9064:
  Added test case do_not_save_start_parameters_for_temporary_children and fixed dialyzer spec.
  Do not save parameter list for any temporary processes
  Do not save initial arguments for dynamic temporary processes

Conflicts:
	lib/stdlib/test/supervisor_SUITE.erl

2 files changed, 176 insertions, 68 deletions

diff --git a/lib/stdlib/src/supervisor.erl b/lib/stdlib/src/supervisor.erl
index 7102fb9f6e..3c5800effa 100644
--- a/lib/stdlib/src/supervisor.erl
+++ b/lib/stdlib/src/supervisor.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
@@ -40,7 +40,7 @@
 %%--------------------------------------------------------------------------
 
 -type child_id() :: pid() | 'undefined'.
--type mfargs()   :: {module(), atom(), [term()]}.
+-type mfargs()   :: {module(), atom(), [term()] | undefined}.
 -type modules()  :: [module()] | 'dynamic'.
 -type restart()  :: 'permanent' | 'transient' | 'temporary'.
 -type shutdown() :: 'brutal_kill' | timeout().
@@ -69,7 +69,7 @@
 -record(state, {name,
 		strategy               :: strategy(),
 		children = []          :: [child()],
-		dynamics = ?DICT:new() :: ?DICT(),
+		dynamics               :: ?DICT() | list(),
 		intensity              :: non_neg_integer(),
 		period                 :: pos_integer(),
 		restarts = [],
@@ -283,16 +283,15 @@ do_start_child_i(M, F, A) ->
 -spec handle_call(call(), term(), state()) -> {'reply', term(), state()}.
 
 handle_call({start_child, EArgs}, _From, State) when ?is_simple(State) ->
-    #child{mfargs = {M, F, A}} = hd(State#state.children),
+    Child = hd(State#state.children),
+    #child{mfargs = {M, F, A}} = Child,
     Args = A ++ EArgs,
     case do_start_child_i(M, F, Args) of
 	{ok, Pid} ->
-	    NState = State#state{dynamics = 
-				 ?DICT:store(Pid, Args, State#state.dynamics)},
+	    NState = save_dynamic_child(Child#child.restart_type, Pid, Args, State),
 	    {reply, {ok, Pid}, NState};
 	{ok, Pid, Extra} ->
-	    NState = State#state{dynamics = 
-				 ?DICT:store(Pid, Args, State#state.dynamics)},
+	    NState = save_dynamic_child(Child#child.restart_type, Pid, Args, State),
 	    {reply, {ok, Pid, Extra}, NState};
 	What ->
 	    {reply, What, State}
@@ -351,10 +350,20 @@ handle_call({terminate_child, Name}, _From, State) ->
 	    {reply, {error, not_found}, State}
     end;
 
-handle_call(which_children, _From, State) when ?is_simple(State) ->
-    [#child{child_type = CT, modules = Mods}] = State#state.children,
+handle_call(which_children, _From, #state{children = [#child{restart_type = temporary,
+							     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, Reply, State};
+
+handle_call(which_children, _From, #state{children = [#child{restart_type = RType,
+							 child_type = CT,
+							 modules = Mods}]} =
+		State) when ?is_simple(State) ->
     Reply = lists:map(fun({Pid, _}) -> {undefined, Pid, CT, Mods} end,
-		      ?DICT:to_list(State#state.dynamics)),
+		      ?DICT:to_list(dynamics_db(RType, State#state.dynamics))),
     {reply, Reply, State};
 
 handle_call(which_children, _From, State) ->
@@ -366,13 +375,31 @@ handle_call(which_children, _From, State) ->
 		  State#state.children),
     {reply, Resp, State};
 
-handle_call(count_children, _From, State) when ?is_simple(State) ->
-    [#child{child_type = CT}] = State#state.children,
+
+handle_call(count_children, _From, #state{children = [#child{restart_type = temporary,
+							     child_type = CT}]} = State)
+  when ?is_simple(State) ->
+    {Active, Count} =
+	lists:foldl(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)),
+    Reply = case CT of
+		supervisor -> [{specs, 1}, {active, Active},
+			       {supervisors, Count}, {workers, 0}];
+		worker -> [{specs, 1}, {active, Active},
+			   {supervisors, 0}, {workers, Count}]
+	    end,
+    {reply, Reply, State};
+
+handle_call(count_children, _From,  #state{children = [#child{restart_type = RType,
+							      child_type = CT}]} = State)
+  when ?is_simple(State) ->
     {Active, Count} =
 	?DICT:fold(fun(Pid, _Val, {Alive, Tot}) ->
 			   if is_pid(Pid) -> {Alive+1, Tot +1};
 			      true -> {Alive, Tot + 1} end
-		   end, {0, 0}, State#state.dynamics),
+		   end, {0, 0}, dynamics_db(RType, State#state.dynamics)),
     Reply = case CT of
 		supervisor -> [{specs, 1}, {active, Active},
 			       {supervisors, Count}, {workers, 0}];
@@ -535,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;
@@ -558,22 +581,21 @@ handle_start_child(Child, State) ->
 %%% Returns: {ok, state()} | {shutdown, state()}
 %%% ---------------------------------------------------
 
-restart_child(Pid, Reason, State) when ?is_simple(State) ->
-    case ?DICT:find(Pid, State#state.dynamics) of
+restart_child(Pid, Reason, #state{children = [Child]} = State) when ?is_simple(State) ->
+    RestartType = Child#child.restart_type,
+    case dynamic_child_args(Pid, dynamics_db(RestartType, State#state.dynamics)) of
 	{ok, Args} ->
-	    [Child] = State#state.children,
-	    RestartType = Child#child.restart_type,
 	    {M, F, _} = Child#child.mfargs,
 	    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}
@@ -608,7 +630,8 @@ restart(Child, State) ->
 
 restart(simple_one_for_one, Child, State) ->
     #child{mfargs = {M, F, A}} = Child,
-    Dynamics = ?DICT:erase(Child#child.pid, State#state.dynamics),
+    Dynamics = ?DICT:erase(Child#child.pid, dynamics_db(Child#child.restart_type,
+							State#state.dynamics)),
     case do_start_child_i(M, F, A) of
 	{ok, Pid} ->
 	    NState = State#state{dynamics = ?DICT:store(Pid, A, Dynamics)},
@@ -755,8 +778,40 @@ monitor_child(Pid) ->
 %%-----------------------------------------------------------------
 %% Child/State manipulating functions.
 %%-----------------------------------------------------------------
-state_del_child(#child{pid = Pid}, State) when ?is_simple(State) ->
-    NDynamics = ?DICT:erase(Pid, State#state.dynamics),
+
+%% 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) ->
+    State#state{dynamics = ?DICT:store(Pid, Args, dynamics_db(RestartType, Dynamics))}.
+
+dynamics_db(temporary, undefined) ->
+    [];
+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).
+
+state_del_child(#child{pid = Pid, restart_type = temporary}, State) when ?is_simple(State) ->
+    NDynamics = lists:delete(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)),
     State#state{dynamics = NDynamics};
 state_del_child(Child, State) ->
     NChildren = del_child(Child#child.name, State#state.children),
diff --git a/lib/stdlib/test/supervisor_SUITE.erl b/lib/stdlib/test/supervisor_SUITE.erl
index 82643e105f..988e98aca4 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
@@ -52,10 +52,12 @@
 	 simple_one_for_one_extra/1]).
 
 %% Misc tests
--export([child_unlink/1, tree/1, count_children_memory/1]).
+-export([child_unlink/1, tree/1, count_children_memory/1,
+	 do_not_save_start_parameters_for_temporary_children/1]).
 
 %-------------------------------------------------------------------------
 
+
 suite() -> [{ct_hooks,[ts_install_cth]}].
 
 all() -> 
@@ -67,7 +69,7 @@ all() ->
      {group, restart_rest_for_one},
      {group, normal_termination},
      {group, abnormal_termination}, child_unlink, tree,
-     count_children_memory].
+     count_children_memory, do_not_save_start_parameters_for_temporary_children].
 
 groups() -> 
     [{sup_start, [],
@@ -117,7 +119,6 @@ init_per_testcase(_Case, Config) ->
 end_per_testcase(_Case, _Config) ->
     ok.
 
-
 start(InitResult) ->
     supervisor:start_link({local, sup_test}, ?MODULE, InitResult).
 
@@ -135,14 +136,8 @@ get_child_counts(Supervisor) ->
      proplists:get_value(supervisors, Counts),
      proplists:get_value(workers, Counts)].
 
-
 %-------------------------------------------------------------------------
-%
 % Test cases starts here.
-%
-%-------------------------------------------------------------------------
-
-
 %-------------------------------------------------------------------------
 sup_start_normal(doc) ->
     ["Tests that the supervisor process starts correctly and that it "
@@ -242,8 +237,6 @@ sup_start_fail(Config) when is_list(Config) ->
     ok.
 %-------------------------------------------------------------------------
 
-%-------------------------------------------------------------------------
-
 sup_stop_infinity(doc) ->
     ["See sup_stop/1 when Shutdown = infinity, this walue is only allowed "
     "for children of type supervisor"];
@@ -594,7 +587,6 @@ child_specs(Config) when is_list(Config) ->
     ?line ok = supervisor:check_childspecs([C3]),
     ?line ok = supervisor:check_childspecs([C4]),
     ok.
-%-------------------------------------------------------------------------
 
 %-------------------------------------------------------------------------
 permanent_normal(doc) ->
@@ -656,7 +648,6 @@ temporary_normal(Config) when is_list(Config) ->
     ?line [1,0,0,1] = get_child_counts(sup_test),
 
     ok.
-%-------------------------------------------------------------------------
 
 %-------------------------------------------------------------------------
 permanent_abnormal(doc) ->
@@ -725,8 +716,6 @@ temporary_abnormal(Config) when is_list(Config) ->
 
     ok.
 %-------------------------------------------------------------------------
-
-%-------------------------------------------------------------------------
 one_for_one(doc) ->
     ["Test the one_for_one base case."];
 one_for_one(suite) -> [];
@@ -805,8 +794,6 @@ one_for_one_escalation(Config) when is_list(Config) ->
     end,
     ok.
 %-------------------------------------------------------------------------
-
-%-------------------------------------------------------------------------
 one_for_all(doc) ->
     ["Test the one_for_all base case."];
 one_for_all(suite) -> [];
@@ -894,8 +881,6 @@ one_for_all_escalation(Config) when is_list(Config) ->
     ok.
 
 %-------------------------------------------------------------------------
-
-%-------------------------------------------------------------------------
 simple_one_for_one(doc) ->
     ["Test the simple_one_for_one base case."];
 simple_one_for_one(suite) -> [];
@@ -1012,8 +997,6 @@ simple_one_for_one_escalation(Config) when is_list(Config) ->
     end,
     ok.
 %-------------------------------------------------------------------------
-
-%-------------------------------------------------------------------------
 rest_for_one(doc) ->
     ["Test the rest_for_one base case."];
 rest_for_one(suite) -> [];
@@ -1287,7 +1270,9 @@ tree(Config) when is_list(Config) ->
     ok.
 %-------------------------------------------------------------------------
 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) ->
+    [];
 count_children_memory(Config) when is_list(Config) ->
     process_flag(trap_exit, true),
     Child = {child, {supervisor_1, start_child, []}, temporary, 1000,
@@ -1324,8 +1309,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 ->
@@ -1337,23 +1322,10 @@ 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.
-
 count_children_allocator_test(MemoryState) ->
     Allocators = [temp_alloc, eheap_alloc, binary_alloc, ets_alloc,
 		  driver_alloc, sl_alloc, ll_alloc, fix_alloc, std_alloc,
@@ -1364,3 +1336,84 @@ count_children_allocator_test(MemoryState) ->
     AllocStates = [lists:keyfind(e, 1, AllocValue)
 		   || {_Type, AllocValue} <- AllocTypes],
     lists:all(fun(State) -> State == {e, true} end, AllocStates).
+%-------------------------------------------------------------------------
+do_not_save_start_parameters_for_temporary_children(doc) ->
+    ["Temporary children shall not be restarted so they should not"
+     "save start parameters, as it potentially can"
+     "take up a huge amount of memory for no purpose."];
+do_not_save_start_parameters_for_temporary_children(suite) ->
+    [];
+do_not_save_start_parameters_for_temporary_children(Config) when is_list(Config) ->
+    process_flag(trap_exit, true),
+    dont_save_start_parameters_for_temporary_children(one_for_all),
+    dont_save_start_parameters_for_temporary_children(one_for_one),
+    dont_save_start_parameters_for_temporary_children(rest_for_one),
+    dont_save_start_parameters_for_temporary_children(simple_one_for_one).
+
+dont_save_start_parameters_for_temporary_children(simple_one_for_one = Type) ->
+    Permanent = {child, {supervisor_1, start_child, []},
+		 permanent, 1000, worker, []},
+    Transient = {child, {supervisor_1, start_child, []},
+		 transient, 1000, worker, []},
+    Temporary = {child, {supervisor_1, start_child, []},
+		 temporary, 1000, worker, []},
+    {ok, Sup1} = supervisor:start_link(?MODULE, {ok, {{Type, 2, 3600}, [Permanent]}}),
+    {ok, Sup2} = supervisor:start_link(?MODULE, {ok, {{Type, 2, 3600}, [Transient]}}),
+    {ok, Sup3} = supervisor:start_link(?MODULE, {ok, {{Type, 2, 3600}, [Temporary]}}),
+
+    LargeList = lists:duplicate(10, "Potentially large"),
+
+    start_children(Sup1, [LargeList], 100),
+    start_children(Sup2, [LargeList], 100),
+    start_children(Sup3, [LargeList], 100),
+
+    [{memory,Mem1}] = process_info(Sup1, [memory]),
+    [{memory,Mem2}] = process_info(Sup2, [memory]),
+    [{memory,Mem3}] = process_info(Sup3, [memory]),
+
+    true = (Mem3 < Mem1)  and  (Mem3 < Mem2),
+
+    exit(Sup1, shutdown),
+    exit(Sup2, shutdown),
+    exit(Sup3, shutdown);
+
+dont_save_start_parameters_for_temporary_children(Type) ->
+    {ok, Sup1} = supervisor:start_link(?MODULE, {ok, {{Type, 2, 3600}, []}}),
+    {ok, Sup2} = supervisor:start_link(?MODULE, {ok, {{Type, 2, 3600}, []}}),
+    {ok, Sup3} = supervisor:start_link(?MODULE, {ok, {{Type, 2, 3600}, []}}),
+
+    LargeList = lists:duplicate(10, "Potentially large"),
+
+    Permanent = {child1, {supervisor_1, start_child, [LargeList]},
+		 permanent, 1000, worker, []},
+    Transient = {child2, {supervisor_1, start_child, [LargeList]},
+		 transient, 1000, worker, []},
+    Temporary = {child3, {supervisor_1, start_child, [LargeList]},
+		 temporary, 1000, worker, []},
+
+    start_children(Sup1, Permanent, 100),
+    start_children(Sup2, Transient, 100),
+    start_children(Sup3, Temporary, 100),
+
+    [{memory,Mem1}] = process_info(Sup1, [memory]),
+    [{memory,Mem2}] = process_info(Sup2, [memory]),
+    [{memory,Mem3}] = process_info(Sup3, [memory]),
+
+    true = (Mem3 < Mem1)  and  (Mem3 < Mem2),
+
+    exit(Sup1, shutdown),
+    exit(Sup2, shutdown),
+    exit(Sup3, shutdown).
+
+start_children(_,_, 0) ->
+    ok;
+start_children(Sup, Args, N) ->
+    Spec = child_spec(Args, N),
+    {ok, _, _} = supervisor:start_child(Sup, Spec),
+    start_children(Sup, Args, N-1).
+
+child_spec([_|_] = SimpleOneForOneArgs, _) ->
+    SimpleOneForOneArgs;
+child_spec({Name, MFA, RestartType, Shutdown, Type, Modules}, N) ->
+    NewName = list_to_atom((atom_to_list(Name) ++ integer_to_list(N))),
+    {NewName, MFA, RestartType, Shutdown, Type, Modules}.