From f1a78f768414e97d30e72b1530475f2893fc75c5 Mon Sep 17 00:00:00 2001 From: Anders Svensson Date: Mon, 17 Apr 2017 10:51:56 +0200 Subject: Don't create intermediate binaries unnecessarily during encode Dict:avp(encode, Value, Name) no longer needs to return a binary, only an iolist(). Message encode runs list_to_binary/1 to convert accumulated lists into a message binary. --- lib/diameter/include/diameter_gen.hrl | 10 ++++---- lib/diameter/src/base/diameter_codec.erl | 42 ++++++++++++++++--------------- lib/diameter/test/diameter_codec_test.erl | 6 +++-- 3 files changed, 31 insertions(+), 27 deletions(-) diff --git a/lib/diameter/include/diameter_gen.hrl b/lib/diameter/include/diameter_gen.hrl index e745e3d2d3..e35d448754 100644 --- a/lib/diameter/include/diameter_gen.hrl +++ b/lib/diameter/include/diameter_gen.hrl @@ -75,7 +75,7 @@ eraser(K) -> %% --------------------------------------------------------------------------- -spec encode_avps(parent_name(), parent_record() | avp_values()) - -> binary() + -> iolist() | no_return(). encode_avps(Name, Vals) @@ -84,7 +84,7 @@ encode_avps(Name, Vals) encode_avps(Name, Rec) -> try - list_to_binary(encode(Name, Rec)) + encode(Name, Rec) catch throw: {?MODULE, Reason} -> diameter_lib:log({encode, error}, @@ -655,7 +655,7 @@ value(_, Avp) -> -spec grouped_avp(decode, avp_name(), bitstring()) -> {avp_record(), [avp()]}; (encode, avp_name(), avp_record() | avp_values()) - -> binary() + -> iolist() | no_return(). %% Length error induced by diameter_codec:collect_avps/1: the AVP @@ -719,10 +719,10 @@ z(Name, {Min, _}) -> binary:copy(z(Name), Min). z('AVP') -> - <<0:64/integer>>; %% minimal header + <<0:64>>; %% minimal header z(Name) -> Bin = diameter_codec:pack_avp(avp_header(Name), empty_value(Name)), - Sz = size(Bin), + Sz = iolist_size(Bin), <<0:Sz/unit:8>>. %% --------------------------------------------------------------------------- diff --git a/lib/diameter/src/base/diameter_codec.erl b/lib/diameter/src/base/diameter_codec.erl index f7ba9bd33f..0d107bf8da 100644 --- a/lib/diameter/src/base/diameter_codec.erl +++ b/lib/diameter/src/base/diameter_codec.erl @@ -144,7 +144,8 @@ encode(Mod, Msg) -> e(_, #diameter_packet{msg = [#diameter_header{} = Hdr | As]} = Pkt) -> try encode_avps(reorder(As)) of Avps -> - Len = 20 + size(Avps), + Bin = list_to_binary(Avps), + Len = 20 + size(Bin), #diameter_header{version = Vsn, is_request = R, @@ -163,7 +164,7 @@ e(_, #diameter_packet{msg = [#diameter_header{} = Hdr | As]} = Pkt) -> Aid:32, Hid:32, Eid:32, - Avps/binary>>} + Bin/binary>>} catch error: Reason -> exit({Reason, diameter_lib:get_stacktrace(), Hdr}) @@ -191,7 +192,9 @@ e(Mod, #diameter_packet{header = Hdr0, msg = Msg} = Pkt) -> try encode_avps(Mod, MsgName, Values) of Avps -> - Len = size(Avps) + 20, + Bin = list_to_binary(Avps), + Len = 20 + size(Bin), + Hdr = Hdr0#diameter_header{length = Len, cmd_code = Code, application_id = Aid, @@ -199,13 +202,14 @@ e(Mod, #diameter_packet{header = Hdr0, msg = Msg} = Pkt) -> is_proxiable = PB, is_error = EB, is_retransmitted = TB}, + Pkt#diameter_packet{header = Hdr, bin = <>} + Bin/binary>>} catch error: Reason -> Hdr = Hdr0#diameter_header{cmd_code = Code, @@ -282,7 +286,7 @@ reorder([], _) -> %% encode_avps/1 encode_avps(Avps) -> - list_to_binary(lists:map(fun pack_avp/1, Avps)). + lists:map(fun pack_avp/1, Avps). %% msg_header/3 @@ -669,11 +673,10 @@ pack_avp(#diameter_avp{data = {Type, Value}} = A) %% ... with a header in various forms ... pack_avp(#diameter_avp{data = {{_,_,_} = T, {Type, Value}}}) -> - pack_avp(T, iolist_to_binary(diameter_types:Type(encode, Value))); + pack_avp(T, diameter_types:Type(encode, Value)); -pack_avp(#diameter_avp{data = {{_,_,_} = T, Bin}}) - when is_binary(Bin) -> - pack_avp(T, Bin); +pack_avp(#diameter_avp{data = {{_,_,_} = T, Data}}) -> + pack_avp(T, Data); pack_avp(#diameter_avp{data = {Dict, Name, Data}}) -> pack_avp(Dict:avp_header(Name), Dict:avp(encode, Data, Name)); @@ -704,7 +707,7 @@ pack_avp(#diameter_avp{code = Code, Flags = lists:foldl(fun flag_avp/2, 0, [{V /= undefined, 2#10000000}, {M, 2#01000000}, {P, 2#00100000}]), - pack_avp({Code, Flags, V}, iolist_to_binary(Data)). + pack_avp({Code, Flags, V}, Data). header_length(<<_:32, 1:1, _/bitstring>>) -> 12; @@ -720,25 +723,24 @@ flag_avp({false, _}, F) -> %%% # pack_avp/2 %%% --------------------------------------------------------------------------- -pack_avp({Code, Flags, VendorId}, Bin) - when is_binary(Bin) -> - Sz = size(Bin), - pack_avp(Code, Flags, Sz, VendorId, Bin, ?PAD(Sz)). +pack_avp({Code, Flags, VendorId}, Data) -> + Sz = iolist_size(Data), + pack_avp(Code, Flags, Sz, VendorId, Data, ?PAD(Sz)). %% Padding is not included in the length field, as mandated by the RFC. %% pack_avp/6 %% %% Prepend the vendor id as required. -pack_avp(Code, Flags, Sz, Vid, Bin, Pad) +pack_avp(Code, Flags, Sz, Vid, Data, Pad) when 0 == Flags band 2#10000000 -> undefined = Vid, %% sanity check - pack_avp(Code, Flags, Sz, 0, 0, Bin, Pad); + pack_avp(Code, Flags, Sz, 0, 0, Data, Pad); -pack_avp(Code, Flags, Sz, Vid, Bin, Pad) -> - pack_avp(Code, Flags, Sz+4, Vid, 1, Bin, Pad). +pack_avp(Code, Flags, Sz, Vid, Data, Pad) -> + pack_avp(Code, Flags, Sz+4, Vid, 1, Data, Pad). %% pack_avp/7 -pack_avp(Code, Flags, Sz, VId, V, Bin, Pad) -> - <>. +pack_avp(Code, Flags, Sz, VId, V, Data, Pad) -> + [<>, Data, <<0:Pad/unit:8>>]. diff --git a/lib/diameter/test/diameter_codec_test.erl b/lib/diameter/test/diameter_codec_test.erl index d8b7377c0c..e041500b37 100644 --- a/lib/diameter/test/diameter_codec_test.erl +++ b/lib/diameter/test/diameter_codec_test.erl @@ -209,8 +209,10 @@ avp_decode(Mod, Name, Type, Eq, Value) -> avp(Mod, decode = X, V, Name, 'Grouped') -> {Rec, _} = Mod:avp(X, V, Name), Rec; -avp(Mod, X, V, Name, _) -> - Mod:avp(X, V, Name). +avp(Mod, decode = X, V, Name, _) -> + Mod:avp(X, V, Name); +avp(Mod, encode = X, V, Name, _) -> + iolist_to_binary(Mod:avp(X, V, Name)). %% v/1 -- cgit v1.2.3