aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorAnders Svensson <[email protected]>2017-04-15 14:46:47 +0200
committerAnders Svensson <[email protected]>2017-06-12 16:13:52 +0200
commitbcef012fe4a4534a9098b15ce42830b9a7a38db9 (patch)
treed77f9e9a68e565a7929d74d787188c86d5ced982
parent3da3055f5c807c3c11a349cba6c19a5abc6bc1c7 (diff)
downloadotp-bcef012fe4a4534a9098b15ce42830b9a7a38db9.tar.gz
otp-bcef012fe4a4534a9098b15ce42830b9a7a38db9.tar.bz2
otp-bcef012fe4a4534a9098b15ce42830b9a7a38db9.zip
Make AVP splitting more efficient
Profiling with fprof showed this prior to this commit: {[{{diameter_codec,decode,3}, 1000, 231.122, 4.092}, {{diameter_codec,collect_avps,1}, 1000, 0.000, 3.929}], { {diameter_codec,collect_avps,1}, 2000, 231.122, 8.021}, % [{{diameter_codec,collect_avps,3}, 1000, 222.932, 11.644}, {garbage_collect, 19, 0.169, 0.169}, {{diameter_codec,collect_avps,1}, 1000, 0.000, 3.929}]}. {[{{diameter_codec,collect_avps,1}, 1000, 222.932, 11.644}, {{diameter_codec,collect_avps,3}, 7000, 0.000, 68.186}], { {diameter_codec,collect_avps,3}, 8000, 222.932, 79.830}, % [{{diameter_codec,split_avp,1}, 7000, 120.886, 72.382}, {{erlang,setelement,3}, 7000, 21.830, 21.830}, {garbage_collect, 48, 0.386, 0.386}, {{diameter_codec,collect_avps,3}, 7000, 0.000, 68.186}]}. Note the time consumed in split_avp/1 and erlang:setelement/3. This commit does more matching in one go, without intermediate results, giving this: {[{{diameter_codec,decode,3}, 1000, 42.512, 3.701}, {{diameter_codec,collect_avps,1}, 1000, 0.000, 3.594}], { {diameter_codec,collect_avps,1}, 2000, 42.512, 7.295}, % [{{diameter_codec,collect_avps,3}, 1000, 35.217, 4.577}, {{diameter_codec,collect_avps,1}, 1000, 0.000, 3.594}]}. {[{{diameter_codec,collect_avps,1}, 1000, 35.217, 4.577}, {{diameter_codec,collect_avps,3}, 7000, 0.000, 27.754}], { {diameter_codec,collect_avps,3}, 8000, 35.217, 32.331}, % [{garbage_collect, 262, 2.647, 2.647}, {suspend, 9, 0.239, 0.000}, {{diameter_codec,collect_avps,3}, 7000, 0.000, 27.754}]}.
-rw-r--r--lib/diameter/src/base/diameter_codec.erl113
1 files changed, 50 insertions, 63 deletions
diff --git a/lib/diameter/src/base/diameter_codec.erl b/lib/diameter/src/base/diameter_codec.erl
index 1ea5357924..3a2f1caf2b 100644
--- a/lib/diameter/src/base/diameter_codec.erl
+++ b/lib/diameter/src/base/diameter_codec.erl
@@ -1,7 +1,7 @@
%%
%% %CopyrightBegin%
%%
-%% Copyright Ericsson AB 2010-2015. All Rights Reserved.
+%% Copyright Ericsson AB 2010-2017. All Rights Reserved.
%%
%% Licensed under the Apache License, Version 2.0 (the "License");
%% you may not use this file except in compliance with the License.
@@ -538,23 +538,14 @@ msg_id(<<_:32, Rbit:1, _:7, CmdCode:24, ApplId:32, _/binary>>) ->
Error :: {5014, #diameter_avp{}}.
collect_avps(#diameter_packet{bin = Bin}) ->
- <<_:20/binary, Avps/binary>> = Bin,
+ <<_:20/binary, Avps/binary>> = Bin, %% assert
collect_avps(Avps);
collect_avps(Bin)
when is_binary(Bin) ->
collect_avps(Bin, 0, []).
-collect_avps(<<>>, _, Acc) ->
- Acc;
-collect_avps(Bin, N, Acc) ->
- try split_avp(Bin) of
- {Rest, AVP} ->
- collect_avps(Rest, N+1, [AVP#diameter_avp{index = N} | Acc])
- catch
- ?FAILURE(Error) ->
- {Error, Acc}
- end.
+%% collect_avps/3
%% 0 1 2 3
%% 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1
@@ -568,32 +559,55 @@ collect_avps(Bin, N, Acc) ->
%% | Data ...
%% +-+-+-+-+-+-+-+-+
-%% split_avp/1
-
-split_avp(Bin) ->
- {Code, V, M, P, Len, HdrLen} = split_head(Bin),
-
- <<_:HdrLen/binary, Rest/binary>> = Bin,
- {Data, B} = split_data(Rest, Len - HdrLen),
-
- {B, #diameter_avp{code = Code,
- vendor_id = V,
- is_mandatory = 1 == M,
- need_encryption = 1 == P,
- data = Data}}.
-
-%% split_head/1
-
-split_head(<<Code:32, 1:1, M:1, P:1, _:5, Len:24, V:32, _/binary>>) ->
- {Code, V, M, P, Len, 12};
+collect_avps(<<Code:32, V:1, M:1, P:1, _:5, Len:24, I:V/unit:32, Rest/binary>>,
+ N,
+ Acc) ->
+ DataLen = Len - 8 - V*4, %% Might be negative, which ensures
+ Pad = (4 - (Len rem 4)) rem 4, %% failure of the Data match below.
+ VendorId = if 1 == V -> I; 0 == V -> undefined end,
+
+ %% Duplicate the diameter_avp creation in each branch below to
+ %% avoid modifying the record, which profiling has shown to be a
+ %% relatively costly part of building the list.
+
+ case Rest of
+ <<Data:DataLen/binary, _:Pad/binary, T/binary>> ->
+ Avp = #diameter_avp{code = Code,
+ vendor_id = VendorId,
+ is_mandatory = 1 == M,
+ need_encryption = 1 == P,
+ data = Data,
+ index = N},
+ collect_avps(T, N+1, [Avp | Acc]);
+ _ ->
+ %% Len points past the end of the message, or doesn't span
+ %% the header. As stated in the 6733 text above, it's
+ %% sufficient to return a zero-filled minimal payload if
+ %% this is a request. Do this (in cases that we know the
+ %% type) by inducing a decode failure and letting the
+ %% dictionary's decode (in diameter_gen) deal with it.
+ %%
+ %% Note that the extra bit can only occur in the trailing
+ %% AVP of a message or Grouped AVP, since a faulty AVP
+ %% Length is otherwise indistinguishable from a correct
+ %% one here, as we don't know the types of the AVPs being
+ %% extracted.
+ Avp = #diameter_avp{code = Code,
+ vendor_id = VendorId,
+ is_mandatory = 1 == M,
+ need_encryption = 1 == P,
+ data = <<0:1, Rest/binary>>,
+ index = N},
+ [Avp | Acc]
+ end;
-split_head(<<Code:32, 0:1, M:1, P:1, _:5, Len:24, _/binary>>) ->
- {Code, undefined, M, P, Len, 8};
+collect_avps(<<>>, _, Acc) ->
+ Acc;
-%% Header is truncated.
-split_head(Bin) ->
- ?THROW({5014, #diameter_avp{data = Bin}}).
-%% Note that pack_avp/1 will pad this at encode if sent in a Failed-AVP.
+%% Header is truncated. pack_avp/1 will pad this at encode if sent in
+%% a Failed-AVP.
+collect_avps(Bin, _, Acc) ->
+ {{5014, #diameter_avp{data = Bin}}, Acc}.
%% 3588:
%%
@@ -626,33 +640,6 @@ split_head(Bin) ->
%% the minimum value mean we might not know the identity of the AVP and
%% (2) the last sentence covers this case.
-%% split_data/3
-
-split_data(Bin, Len) ->
- Pad = (4 - (Len rem 4)) rem 4,
-
- %% Len might be negative here, but that ensures the failure of the
- %% binary match.
-
- case Bin of
- <<Data:Len/binary, _:Pad/binary, Rest/binary>> ->
- {Data, Rest};
- _ ->
- %% Header length points past the end of the message, or
- %% doesn't span the header. As stated in the 6733 text
- %% above, it's sufficient to return a zero-filled minimal
- %% payload if this is a request. Do this (in cases that we
- %% know the type) by inducing a decode failure and letting
- %% the dictionary's decode (in diameter_gen) deal with it.
- %%
- %% Note that the extra bit can only occur in the trailing
- %% AVP of a message or Grouped AVP, since a faulty AVP
- %% Length is otherwise indistinguishable from a correct
- %% one here, since we don't know the types of the AVPs
- %% being extracted.
- {<<0:1, Bin/binary>>, <<>>}
- end.
-
%%% ---------------------------------------------------------------------------
%%% # pack_avp/1
%%% ---------------------------------------------------------------------------