Skip to content

Commit 1ff2209

Browse files
committed
Use new OTP 26 fun ets:lookup_element/4
Routing to nowhere with the direct exchange is now a bit faster: ``` java -jar target/perf-test.jar -x 2 -y 0 -k route-nowhere -z 60 ``` Remove dead code rabbit_db_binding:route_v2/3
1 parent 7e3021e commit 1ff2209

File tree

6 files changed

+26
-85
lines changed

6 files changed

+26
-85
lines changed

deps/rabbit/src/rabbit_db_binding.erl

Lines changed: 7 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@
2020
fold/2]).
2121

2222
%% Routing. These functions are in the hot code path
23-
-export([match/2, match_routing_key/3]).
23+
-export([match/2, match_routing_key/2]).
2424

2525
%% Exported to be used by various rabbit_db_* modules
2626
-export([
@@ -398,31 +398,22 @@ match_in_mnesia(SrcName, Match) ->
398398
%% match_routing_key().
399399
%% -------------------------------------------------------------------
400400

401-
-spec match_routing_key(Src, RoutingKeys, UseIndex) -> [Dst] when
401+
-spec match_routing_key(Src, RoutingKeys) -> [Dst] when
402402
Src :: rabbit_types:binding_source(),
403403
Dst :: rabbit_types:binding_destination(),
404-
RoutingKeys :: [binary() | '_'],
405-
UseIndex :: boolean().
404+
RoutingKeys :: [binary() | '_'].
406405
%% @doc Matches all binding records that have `Src' as source of the binding
407406
%% and that match any routing key in `RoutingKeys'.
408407
%%
409408
%% @returns the list of destinations
410409
%%
411410
%% @private
412411

413-
match_routing_key(SrcName, RoutingKeys, UseIndex) ->
412+
match_routing_key(SrcName, RoutingKeys) ->
414413
rabbit_db:run(
415-
#{mnesia => fun() -> match_routing_key_in_mnesia(SrcName, RoutingKeys, UseIndex) end
414+
#{mnesia => fun() -> route_in_mnesia(SrcName, RoutingKeys) end
416415
}).
417416

418-
match_routing_key_in_mnesia(SrcName, RoutingKeys, UseIndex) ->
419-
case UseIndex of
420-
true ->
421-
route_v2(?MNESIA_INDEX_TABLE, SrcName, RoutingKeys);
422-
_ ->
423-
route_in_mnesia_v1(SrcName, RoutingKeys)
424-
end.
425-
426417
%% -------------------------------------------------------------------
427418
%% recover().
428419
%% -------------------------------------------------------------------
@@ -728,13 +719,13 @@ continue({[], Continuation}) -> continue(mnesia:select(Continuation)).
728719

729720
%% Routing. Hot code path
730721
%% -------------------------------------------------------------------------
731-
route_in_mnesia_v1(SrcName, [RoutingKey]) ->
722+
route_in_mnesia(SrcName, [RoutingKey]) ->
732723
MatchHead = #route{binding = #binding{source = SrcName,
733724
destination = '$1',
734725
key = RoutingKey,
735726
_ = '_'}},
736727
ets:select(?MNESIA_TABLE, [{MatchHead, [], ['$1']}]);
737-
route_in_mnesia_v1(SrcName, [_|_] = RoutingKeys) ->
728+
route_in_mnesia(SrcName, [_|_] = RoutingKeys) ->
738729
%% Normally we'd call mnesia:dirty_select/2 here, but that is quite
739730
%% expensive for the same reasons as above, and, additionally, due to
740731
%% mnesia 'fixing' the table with ets:safe_fixtable/2, which is wholly
@@ -753,36 +744,3 @@ route_in_mnesia_v1(SrcName, [_|_] = RoutingKeys) ->
753744
Conditions = [list_to_tuple(['orelse' | [{'=:=', '$2', RKey} ||
754745
RKey <- RoutingKeys]])],
755746
ets:select(?MNESIA_TABLE, [{MatchHead, Conditions, ['$1']}]).
756-
757-
%% rabbit_router:match_routing_key/2 uses ets:select/2 to get destinations.
758-
%% ets:select/2 is expensive because it needs to compile the match spec every
759-
%% time and lookup does not happen by a hash key.
760-
%%
761-
%% In contrast, route_v2/2 increases end-to-end message sending throughput
762-
%% (i.e. from RabbitMQ client to the queue process) by up to 35% by using ets:lookup_element/3.
763-
%% Only the direct exchange type uses the rabbit_index_route table to store its
764-
%% bindings by table key tuple {SourceExchange, RoutingKey}.
765-
-spec route_v2(ets:table(), rabbit_types:binding_source(), [rabbit_router:routing_key(), ...]) ->
766-
rabbit_router:match_result().
767-
route_v2(Table, SrcName, [RoutingKey]) ->
768-
%% optimization
769-
destinations(Table, SrcName, RoutingKey);
770-
route_v2(Table, SrcName, [_|_] = RoutingKeys) ->
771-
lists:flatmap(fun(Key) ->
772-
destinations(Table, SrcName, Key)
773-
end, RoutingKeys).
774-
775-
destinations(Table, SrcName, RoutingKey) ->
776-
%% Prefer try-catch block over checking Key existence with ets:member/2.
777-
%% The latter reduces throughput by a few thousand messages per second because
778-
%% of function db_member_hash in file erl_db_hash.c.
779-
%% We optimise for the happy path, that is the binding / table key is present.
780-
try
781-
ets:lookup_element(Table,
782-
{SrcName, RoutingKey},
783-
#index_route.destination)
784-
catch
785-
error:badarg ->
786-
[]
787-
end.
788-

deps/rabbit/src/rabbit_exchange_type_direct.erl

Lines changed: 5 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@ assert_args_equivalence(X, Args) ->
5353
%% time and lookup does not happen by a hash key.
5454
%%
5555
%% In contrast, route_v2/2 increases end-to-end message sending throughput
56-
%% (i.e. from RabbitMQ client to the queue process) by up to 35% by using ets:lookup_element/3.
56+
%% (i.e. from RabbitMQ client to the queue process) by up to 35% by using ets:lookup_element/4.
5757
%% Only the direct exchange type uses the rabbit_index_route table to store its
5858
%% bindings by table key tuple {SourceExchange, RoutingKey}.
5959
-spec route_v2(rabbit_types:binding_source(), [rabbit_router:routing_key(), ...]) ->
@@ -67,15 +67,7 @@ route_v2(SrcName, [_|_] = RoutingKeys) ->
6767
end, RoutingKeys).
6868

6969
destinations(SrcName, RoutingKey) ->
70-
%% Prefer try-catch block over checking Key existence with ets:member/2.
71-
%% The latter reduces throughput by a few thousand messages per second because
72-
%% of function db_member_hash in file erl_db_hash.c.
73-
%% We optimise for the happy path, that is the binding / table key is present.
74-
try
75-
ets:lookup_element(rabbit_index_route,
76-
{SrcName, RoutingKey},
77-
#index_route.destination)
78-
catch
79-
error:badarg ->
80-
[]
81-
end.
70+
ets:lookup_element(rabbit_index_route,
71+
{SrcName, RoutingKey},
72+
#index_route.destination,
73+
[]).

deps/rabbit/src/rabbit_quorum_queue.erl

Lines changed: 7 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1000,7 +1000,7 @@ cluster_state(Name) ->
10001000
case whereis(Name) of
10011001
undefined -> down;
10021002
_ ->
1003-
case ets_lookup_element(ra_state, Name, 2, undefined) of
1003+
case ets:lookup_element(ra_state, Name, 2, undefined) of
10041004
recover ->
10051005
recovering;
10061006
_ ->
@@ -1397,10 +1397,10 @@ i(messages, Q) when ?is_amqqueue(Q) ->
13971397
quorum_messages(QName);
13981398
i(messages_ready, Q) when ?is_amqqueue(Q) ->
13991399
QName = amqqueue:get_name(Q),
1400-
ets_lookup_element(queue_coarse_metrics, QName, 2, 0);
1400+
ets:lookup_element(queue_coarse_metrics, QName, 2, 0);
14011401
i(messages_unacknowledged, Q) when ?is_amqqueue(Q) ->
14021402
QName = amqqueue:get_name(Q),
1403-
ets_lookup_element(queue_coarse_metrics, QName, 3, 0);
1403+
ets:lookup_element(queue_coarse_metrics, QName, 3, 0);
14041404
i(policy, Q) ->
14051405
case rabbit_policy:name(Q) of
14061406
none -> '';
@@ -1418,7 +1418,7 @@ i(effective_policy_definition, Q) ->
14181418
end;
14191419
i(consumers, Q) when ?is_amqqueue(Q) ->
14201420
QName = amqqueue:get_name(Q),
1421-
Consumers = ets_lookup_element(queue_metrics, QName, 2, []),
1421+
Consumers = ets:lookup_element(queue_metrics, QName, 2, []),
14221422
proplists:get_value(consumers, Consumers, 0);
14231423
i(memory, Q) when ?is_amqqueue(Q) ->
14241424
{Name, _} = amqqueue:get_pid(Q),
@@ -1440,7 +1440,7 @@ i(state, Q) when ?is_amqqueue(Q) ->
14401440
end;
14411441
i(local_state, Q) when ?is_amqqueue(Q) ->
14421442
{Name, _} = amqqueue:get_pid(Q),
1443-
ets_lookup_element(ra_state, Name, 2, not_member);
1443+
ets:lookup_element(ra_state, Name, 2, not_member);
14441444
i(garbage_collection, Q) when ?is_amqqueue(Q) ->
14451445
{Name, _} = amqqueue:get_pid(Q),
14461446
try
@@ -1517,7 +1517,7 @@ open_files(Name) ->
15171517
undefined ->
15181518
{node(), 0};
15191519
Pid ->
1520-
{node(), ets_lookup_element(ra_open_file_metrics, Pid, 2, 0)}
1520+
{node(), ets:lookup_element(ra_open_file_metrics, Pid, 2, 0)}
15211521
end.
15221522

15231523
leader(Q) when ?is_amqqueue(Q) ->
@@ -1576,7 +1576,7 @@ is_process_alive(Name, Node) ->
15761576
-spec quorum_messages(rabbit_amqqueue:name()) -> non_neg_integer().
15771577

15781578
quorum_messages(QName) ->
1579-
ets_lookup_element(queue_coarse_metrics, QName, 4, 0).
1579+
ets:lookup_element(queue_coarse_metrics, QName, 4, 0).
15801580

15811581
quorum_ctag(Int) when is_integer(Int) ->
15821582
integer_to_binary(Int);
@@ -1695,14 +1695,6 @@ prepare_content(Content) ->
16951695
%% rabbit_fifo can directly parse it without having to decode again.
16961696
Content.
16971697

1698-
ets_lookup_element(Tbl, Key, Pos, Default) ->
1699-
try ets:lookup_element(Tbl, Key, Pos) of
1700-
V -> V
1701-
catch
1702-
_:badarg ->
1703-
Default
1704-
end.
1705-
17061698
erpc_call(Node, M, F, A, _Timeout)
17071699
when Node =:= node() ->
17081700
%% Only timeout 'infinity' optimises the local call in OTP 23-25 avoiding a new process being spawned:

deps/rabbit/src/rabbit_router.erl

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,4 +32,4 @@ match_bindings(SrcName, Match) ->
3232
match_result().
3333

3434
match_routing_key(SrcName, RoutingKeys) ->
35-
rabbit_db_binding:match_routing_key(SrcName, RoutingKeys, false).
35+
rabbit_db_binding:match_routing_key(SrcName, RoutingKeys).

deps/rabbit/test/rabbit_db_binding_SUITE.erl

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -322,10 +322,10 @@ match_routing_key1(_Config) ->
322322
Exchange2 = #exchange{name = XName2, durable = true},
323323
Binding = #binding{source = XName1, key = <<"*.*">>, destination = XName2,
324324
args = #{foo => bar}},
325-
?assertEqual([], rabbit_db_binding:match_routing_key(XName1, [<<"a.b.c">>], false)),
325+
?assertEqual([], rabbit_db_binding:match_routing_key(XName1, [<<"a.b.c">>])),
326326
?assertMatch({new, #exchange{}}, rabbit_db_exchange:create_or_get(Exchange1)),
327327
?assertMatch({new, #exchange{}}, rabbit_db_exchange:create_or_get(Exchange2)),
328328
?assertMatch(ok, rabbit_db_binding:create(Binding, fun(_, _) -> ok end)),
329-
?assertEqual([], rabbit_db_binding:match_routing_key(XName1, [<<"a.b.c">>], false)),
330-
?assertEqual([XName2], rabbit_db_binding:match_routing_key(XName1, [<<"a.b">>], false)),
329+
?assertEqual([], rabbit_db_binding:match_routing_key(XName1, [<<"a.b.c">>])),
330+
?assertEqual([XName2], rabbit_db_binding:match_routing_key(XName1, [<<"a.b">>])),
331331
passed.

deps/rabbitmq_management_agent/src/rabbit_mgmt_data.erl

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -367,12 +367,11 @@ match_consumer_spec(Id) ->
367367
match_queue_consumer_spec(Id) ->
368368
[{{{'$1', '_', '_'}, '_'}, [{'==', {Id}, '$1'}], ['$_']}].
369369

370-
lookup_element(Table, Key) -> lookup_element(Table, Key, 2).
370+
lookup_element(Table, Key) ->
371+
lookup_element(Table, Key, 2).
371372

372373
lookup_element(Table, Key, Pos) ->
373-
try ets:lookup_element(Table, Key, Pos)
374-
catch error:badarg -> []
375-
end.
374+
ets:lookup_element(Table, Key, Pos, []).
376375

377376
-spec lookup_smaller_sample(atom(), any()) -> maybe_slide().
378377
lookup_smaller_sample(Table, Id) ->

0 commit comments

Comments
 (0)