If we refactor rabbit_misc:build_acyclic_graph so that we don't assume the arity of the functions we pass in, then it's easy to build the complete graph then filter it by application, rather than the other way round - avoiding rather a lot of work.

5 files changed, 20 insertions, 66 deletions

diff --git a/src/app_utils.erl b/src/app_utils.erl
index ce07ac5087..e321888d2e 100644
--- a/src/app_utils.erl
+++ b/src/app_utils.erl
@@ -75,8 +75,8 @@ wait_for_applications(Apps) ->
 
 app_dependency_order(RootApps, StripUnreachable) ->
     {ok, G} = rabbit_misc:build_acyclic_graph(
-                fun (App, _Deps) -> [{App, App}] end,
-                fun (App,  Deps) -> [{Dep, App} || Dep <- Deps] end,
+                fun ({App, _Deps}) -> [{App, App}] end,
+                fun ({App,  Deps}) -> [{Dep, App} || Dep <- Deps] end,
                 [{App, app_dependencies(App)} ||
                     {App, _Desc, _Vsn} <- application:loaded_applications()]),
     try
diff --git a/src/rabbit.erl b/src/rabbit.erl
index 1610614570..07b21e5073 100644
--- a/src/rabbit.erl
+++ b/src/rabbit.erl
@@ -531,11 +531,17 @@ run_boot_steps() ->
     run_boot_steps([App || {App, _, _} <- application:loaded_applications()]).
 
 run_boot_steps(Apps) ->
-    Steps = load_steps(Apps),
+    Steps = find_steps(Apps),
     [ok = run_step(StepName, Attributes, mfa) ||
         {_, StepName, Attributes} <- Steps],
     ok.
 
+find_steps(BaseApps) ->
+    Apps = BaseApps -- [App || {App, _, _} <- rabbit_misc:which_applications()],
+    FullBoot = sort_boot_steps(
+                 rabbit_misc:all_module_attributes(rabbit_boot_step)),
+    [Step || {App, _, _} = Step <- FullBoot, lists:member(App, Apps)].
+
 run_step(StepName, Attributes, AttributeName) ->
     case [MFA || {Key, MFA} <- Attributes,
                  Key =:= AttributeName] of
@@ -555,62 +561,10 @@ run_step(StepName, Attributes, AttributeName) ->
             ok
     end.
 
-load_steps(BaseApps) ->
-    Apps = BaseApps -- [App || {App, _, _} <- rabbit_misc:which_applications()],
-    StepAttrs = rabbit_misc:all_module_attributes(rabbit_boot_step),
-    {AllSteps, StepsDict} =
-        lists:foldl(
-          fun({AppName, Mod, Steps}, {AccSteps, AccDict}) ->
-                  {[{Mod, {AppName, Steps}}|AccSteps],
-                   lists:foldl(
-                     fun({StepName, _}, Acc) ->
-                             dict:store(StepName, AppName, Acc)
-                     end, AccDict, Steps)}
-          end, {[], dict:new()}, StepAttrs),
-    Steps = lists:foldl(filter_steps(Apps, StepsDict), [], AllSteps),
-    sort_boot_steps(lists:usort(Steps)).
-
-filter_steps(Apps, Dict) ->
-    fun({Mod, {AppName, Steps}}, Acc) ->
-            Steps2 = [begin
-                          Filtered = lists:foldl(filter_attrs(Apps, Dict),
-                                                 [], Attrs),
-                          {Step, Filtered}
-                      end || {Step, Attrs} <- Steps,
-                             filter_app(Apps, Dict, Step)],
-            [{Mod, {AppName, Steps2}}|Acc]
-    end.
-
-filter_app(Apps, Dict, Step) ->
-    case dict:find(Step, Dict) of
-        {ok, App} -> lists:member(App, Apps);
-        error     -> false
-    end.
-
-filter_attrs(Apps, Dict) ->
-    fun(Attr={Type, Other}, AccAttrs) when Type =:= requires orelse
-                                           Type =:= enables ->
-            %% If we don't know about a dependency, we allow it through,
-            %% since we don't *know* that it should be ignored. If, on
-            %% the other hand, we recognise a dependency then we _only_
-            %% include it (i.e., the requires/enables attribute itself)
-            %% if the referenced step comes from one of the Apps we're
-            %% actively working with at this point.
-            case dict:find(Other, Dict) of
-                error     -> [Attr | AccAttrs];
-                {ok, App} -> case lists:member(App, Apps) of
-                                 true  -> [Attr | AccAttrs];
-                                 false -> AccAttrs
-                             end
-            end;
-       (Attr, AccAttrs) ->
-            [Attr | AccAttrs]
-    end.
-
-vertices(_Module, {AppName, Steps}) ->
+vertices({AppName, _Module, Steps}) ->
     [{StepName, {AppName, StepName, Atts}} || {StepName, Atts} <- Steps].
 
-edges(_Module, {_AppName, Steps}) ->
+edges({_AppName, _Module, Steps}) ->
     [case Key of
          requires -> {StepName, OtherStep};
          enables  -> {OtherStep, StepName}
@@ -619,7 +573,7 @@ edges(_Module, {_AppName, Steps}) ->
             Key =:= requires orelse Key =:= enables].
 
 sort_boot_steps(UnsortedSteps) ->
-    case rabbit_misc:build_acyclic_graph(fun vertices/2, fun edges/2,
+    case rabbit_misc:build_acyclic_graph(fun vertices/1, fun edges/1,
                                          UnsortedSteps) of
         {ok, G} ->
             %% Use topological sort to find a consistent ordering (if
diff --git a/src/rabbit_misc.erl b/src/rabbit_misc.erl
index 866166328d..eb79fca6fa 100644
--- a/src/rabbit_misc.erl
+++ b/src/rabbit_misc.erl
@@ -871,13 +871,13 @@ build_acyclic_graph(VertexFun, EdgeFun, Graph) ->
         [case digraph:vertex(G, Vertex) of
              false -> digraph:add_vertex(G, Vertex, Label);
              _     -> ok = throw({graph_error, {vertex, duplicate, Vertex}})
-         end || {Module, Atts}  <- Graph,
-                {Vertex, Label} <- VertexFun(Module, Atts)],
+         end || GraphElem       <- Graph,
+                {Vertex, Label} <- VertexFun(GraphElem)],
         [case digraph:add_edge(G, From, To) of
              {error, E} -> throw({graph_error, {edge, E, From, To}});
              _          -> ok
-         end || {Module, Atts} <- Graph,
-                {From, To}     <- EdgeFun(Module, Atts)],
+         end || GraphElem  <- Graph,
+                {From, To} <- EdgeFun(GraphElem)],
         {ok, G}
     catch {graph_error, Reason} ->
             true = digraph:delete(G),
diff --git a/src/rabbit_plugins.erl b/src/rabbit_plugins.erl
index d3d76be059..edb7b12f26 100644
--- a/src/rabbit_plugins.erl
+++ b/src/rabbit_plugins.erl
@@ -105,8 +105,8 @@ read_enabled(PluginsFile) ->
 %% the resulting list, otherwise they're skipped.
 dependencies(Reverse, Sources, AllPlugins) ->
     {ok, G} = rabbit_misc:build_acyclic_graph(
-                fun (App, _Deps) -> [{App, App}] end,
-                fun (App,  Deps) -> [{App, Dep} || Dep <- Deps] end,
+                fun ({App, _Deps}) -> [{App, App}] end,
+                fun ({App,  Deps}) -> [{App, Dep} || Dep <- Deps] end,
                 lists:ukeysort(
                   1, [{Name, Deps} ||
                          #plugin{name         = Name,
diff --git a/src/rabbit_version.erl b/src/rabbit_version.erl
index e4fcc5966b..ef480ccba6 100644
--- a/src/rabbit_version.erl
+++ b/src/rabbit_version.erl
@@ -115,8 +115,8 @@ upgrades_required(Scope) ->
 with_upgrade_graph(Fun, Scope) ->
     Attrs = rabbit_misc:all_module_attributes(rabbit_upgrade),
     case rabbit_misc:build_acyclic_graph(
-           fun (Module, Steps) -> vertices(Module, Steps, Scope) end,
-           fun (Module, Steps) -> edges(Module, Steps, Scope) end,
+           fun ({Module, Steps}) -> vertices(Module, Steps, Scope) end,
+           fun ({Module, Steps}) -> edges(Module, Steps, Scope) end,
            [{Mod, Steps} || {_, Mod, Steps} <- Attrs]) of
         {ok, G} -> try
                        Fun(G)