Minimised the number of calls to the XTs on deletion

3 files changed, 93 insertions, 37 deletions

diff --git a/src/rabbit_amqqueue.erl b/src/rabbit_amqqueue.erl
index 7dfe41ba..3c767eef 100644
--- a/src/rabbit_amqqueue.erl
+++ b/src/rabbit_amqqueue.erl
@@ -509,4 +509,3 @@ delegate_call(Pid, Msg, Timeout) ->
 
 delegate_cast(Pid, Msg) ->
     delegate:invoke(Pid, fun (P) -> gen_server2:cast(P, Msg) end).
-
diff --git a/src/rabbit_binding.erl b/src/rabbit_binding.erl
index eabb422e..2e9c580a 100644
--- a/src/rabbit_binding.erl
+++ b/src/rabbit_binding.erl
@@ -36,9 +36,11 @@
 -export([list_for_source/1, list_for_destination/1,
          list_for_source_and_destination/2]).
 -export([info_keys/0, info/1, info/2, info_all/1, info_all/2]).
+-export([post_binding_removal_fun/1]).
 %% these must all be run inside a mnesia tx
 -export([has_for_source/1, remove_for_source/1,
-         remove_for_destination/1, remove_transient_for_destination/1]).
+         remove_for_destination/1, remove_transient_for_destination/1,
+         remove_for_destination_inner/1]).
 
 %%----------------------------------------------------------------------------
 
@@ -84,9 +86,12 @@
 -spec(has_for_source/1 :: (rabbit_types:binding_source()) -> boolean()).
 -spec(remove_for_source/1 :: (rabbit_types:binding_source()) -> bindings()).
 -spec(remove_for_destination/1 ::
-        (rabbit_types:binding_destination()) -> fun (() -> any())).
+        (rabbit_types:binding_destination()) -> fun (() -> 'ok')).
 -spec(remove_transient_for_destination/1 ::
-        (rabbit_types:binding_destination()) -> fun (() -> any())).
+        (rabbit_types:binding_destination()) -> fun (() -> 'ok')).
+-spec(remove_for_destination_inner/1 ::
+        (rabbit_types:binding_destination()) -> dict:dictionary()).
+-spec(post_binding_removal_fun/1 :: (dict:dictionary()) -> fun (() -> 'ok')).
 
 -endif.
 
@@ -157,9 +162,9 @@ remove(Binding, InnerFun) ->
                                    ok = sync_binding(
                                           B, all_durable([Src, Dst]),
                                           fun mnesia:delete_object/3),
-                                   Deleted =
-                                       rabbit_exchange:maybe_auto_delete(Src),
-                                   {{Deleted, Src}, B};
+                                   {ok, merge_maybe_auto_delete(
+                                          Binding#binding.source, [B],
+                                          dict:new())};
                                {error, _} = E ->
                                    E
                            end
@@ -167,10 +172,8 @@ remove(Binding, InnerFun) ->
            end) of
         {error, _} = Err ->
             Err;
-        {{IsDeleted, Src}, B} ->
-            ok = post_binding_removal(IsDeleted, Src, [B]),
-            rabbit_event:notify(binding_deleted, info(B)),
-            ok
+        {ok, Grouped} ->
+            ok = (post_binding_removal_fun(Grouped))()
     end.
 
 list(VHostPath) ->
@@ -247,6 +250,9 @@ remove_for_source(SrcName) ->
 remove_for_destination(DstName) ->
     remove_for_destination(DstName, fun delete_forward_routes/1).
 
+remove_for_destination_inner(DstName) ->
+    remove_for_destination_inner(DstName, fun delete_forward_routes/1).
+
 remove_transient_for_destination(DstName) ->
     remove_for_destination(DstName, fun delete_transient_forward_routes/1).
 
@@ -307,6 +313,10 @@ continue({[_|_], _})         -> true;
 continue({[], Continuation}) -> continue(mnesia:select(Continuation)).
 
 remove_for_destination(DstName, FwdDeleteFun) ->
+    post_binding_removal_fun(
+      remove_for_destination_inner(DstName, FwdDeleteFun)).
+
+remove_for_destination_inner(DstName, FwdDeleteFun) ->
     DeletedBindings =
         [begin
              Route = reverse_route(ReverseRoute),
@@ -322,39 +332,85 @@ remove_for_destination(DstName, FwdDeleteFun) ->
                                             destination = DstName,
                                             _           = '_'}}),
                          write)],
-    Grouped = group_bindings_and_auto_delete(
-                lists:keysort(#binding.source, DeletedBindings), []),
-    fun () ->
-            lists:foreach(
-              fun ({{IsDeleted, Src}, Bs}) ->
-                      ok = post_binding_removal(IsDeleted, Src, Bs)
-              end, Grouped)
+    group_bindings_and_auto_delete(
+      lists:keysort(#binding.source, DeletedBindings), dict:new()).
+
+post_binding_removal_fun(Grouped) ->
+    fun () -> dict:fold(
+                fun (_SrcName, {Src, IsDeleted, Bs}, ok) ->
+                        post_binding_removal(IsDeleted, Src,
+                                             lists:usort(lists:flatten(Bs)))
+                end, ok, Grouped)
     end.
 
 post_binding_removal(not_deleted, Src = #exchange{ type = Type }, Bs) ->
     ok = (type_to_module(Type)):remove_bindings(Src, Bs);
-post_binding_removal({auto_deleted, Fun}, Src = #exchange{ type = Type }, Bs) ->
-    ok = (type_to_module(Type)):delete(Src, Bs),
-    Fun(),
-    ok.
+post_binding_removal(deleted, Src = #exchange{ type = Type }, Bs) ->
+    ok = (type_to_module(Type)):delete(Src, Bs).
 
 %% Requires that its input binding list is sorted in exchange-name
 %% order, so that the grouping of bindings (for passing to
 %% group_bindings_and_auto_delete1) works properly.
 group_bindings_and_auto_delete([], Acc) ->
     Acc;
-group_bindings_and_auto_delete(
-  [B = #binding{source = SrcName} | Bs], Acc) ->
+group_bindings_and_auto_delete([B = #binding{source = SrcName} | Bs], Acc) ->
     group_bindings_and_auto_delete(SrcName, Bs, [B], Acc).
 
-group_bindings_and_auto_delete(
-  SrcName, [B = #binding{source = SrcName} | Bs], Bindings, Acc) ->
+group_bindings_and_auto_delete(SrcName, [B = #binding{source = SrcName} | Bs],
+                               Bindings, Acc) ->
     group_bindings_and_auto_delete(SrcName, Bs, [B | Bindings], Acc);
 group_bindings_and_auto_delete(SrcName, Removed, Bindings, Acc) ->
-    %% either Removed is [], or its head has a non-matching SrcName
-    [Src] = mnesia:read({rabbit_exchange, SrcName}),
-    NewAcc = [{{rabbit_exchange:maybe_auto_delete(Src), Src}, Bindings} | Acc],
-    group_bindings_and_auto_delete(Removed, NewAcc).
+    %% Either Removed is [], or its head has a non-matching SrcName.
+    group_bindings_and_auto_delete(
+      Removed, merge_maybe_auto_delete(SrcName, Bindings, Acc)).
+
+%% Once a binding source is deleted, we'll never revisit it, so we
+%% should never find that the existing entry is {deleted, Bindings}.
+merge_maybe_auto_delete(SrcName, Bindings, Acc) ->
+    UpdateFun = fun (NewResult, Src) ->
+                        dict:update(
+                          SrcName,
+                          fun ({Src1, Result, Bindings1}) ->
+                                  {not_undef(Src, Src1),
+                                   boolean_or(deleted, Result, NewResult),
+                                   [Bindings | Bindings1]}
+                          end, {Src, NewResult, Bindings}, Acc)
+                end,
+    case mnesia:read({rabbit_exchange, SrcName}) of
+        []    -> UpdateFun(deleted, undefined);
+        [Src] -> case rabbit_exchange:maybe_auto_delete(Src) of
+                     not_deleted ->
+                         UpdateFun(not_deleted, Src);
+                     {auto_deleted, Acc1} ->
+                         merge_binding_dicts(UpdateFun(deleted, Src), Acc1)
+                 end
+    end.
+
+%% Should never find that both have deleted the exchange.
+merge_binding_dicts(LHS, RHS) ->
+    dict:merge(
+      fun (_SrcName,
+           {SrcA, IsDeletedA, BindingsL}, {SrcB, IsDeletedB, BindingsR}) ->
+              {not_undef(SrcA, SrcB),
+               boolean_or(deleted, IsDeletedA, IsDeletedB),
+               [BindingsL | BindingsR]}
+      end, LHS, RHS).
+
+not_undef(undefined, undefined) ->
+    undefined;
+not_undef(undefined, N) ->
+    N;
+not_undef(N, undefined) ->
+    N;
+not_undef(N, N) ->
+    N.
+
+boolean_or(True, True, _Any) ->
+    True;
+boolean_or(True, _Any, True) ->
+    True;
+boolean_or(_True, Any, Any) ->
+    Any.
 
 delete_forward_routes(Route) ->
     ok = mnesia:delete_object(rabbit_route, Route, write),
diff --git a/src/rabbit_exchange.erl b/src/rabbit_exchange.erl
index 9581c229..50f49d0c 100644
--- a/src/rabbit_exchange.erl
+++ b/src/rabbit_exchange.erl
@@ -84,7 +84,7 @@
                               rabbit_types:error('in_use')).
 -spec(maybe_auto_delete/1::
         (rabbit_types:exchange())
-        -> 'not_deleted' | {'auto_deleted', fun (() -> any())}).
+        -> 'not_deleted' | {'auto_deleted', dict:dictionary()}).
 
 -endif.
 
@@ -278,10 +278,11 @@ delete(XName, IfUnused) ->
               false -> fun unconditional_delete/1
           end,
     case call_with_exchange(XName, Fun) of
-        {deleted, X = #exchange{type = Type}, Bs, Fun1} ->
-            (type_to_module(Type)):delete(X, Bs),
-            Fun1(),
-            ok;
+        {deleted, X, Bs, Grouped} ->
+            Grouped1 = dict:update(XName, fun ({_X, _MaybeDeleted, Bs1}) ->
+                                                  {X, deleted, [Bs | Bs1]}
+                                          end, {X, deleted, Bs}, Grouped),
+            ok = (rabbit_binding:post_binding_removal_fun(Grouped1))();
         Error = {error, _InUseOrNotFound} ->
             Error
     end.
@@ -291,7 +292,7 @@ maybe_auto_delete(#exchange{auto_delete = false}) ->
 maybe_auto_delete(#exchange{auto_delete = true} = X) ->
     case conditional_delete(X) of
         {error, in_use}       -> not_deleted;
-        {deleted, X, [], Fun} -> {auto_deleted, Fun}
+        {deleted, X, [], Res} -> {auto_deleted, Res}
     end.
 
 conditional_delete(X = #exchange{name = XName}) ->
@@ -305,4 +306,4 @@ unconditional_delete(X = #exchange{name = XName}) ->
     ok = mnesia:delete({rabbit_durable_exchange, XName}),
     ok = mnesia:delete({rabbit_exchange, XName}),
     rabbit_event:notify(exchange_deleted, [{name, XName}]),
-    {deleted, X, Bindings, rabbit_binding:remove_for_destination(XName)}.
+    {deleted, X, Bindings, rabbit_binding:remove_for_destination_inner(XName)}.