Skip to content

Commit b0260cf

Browse files
ansdansd
committed
Avoid creation of binaries in AMQP 1.0 generator
When generating iodata() in the AMQP 1.0 generator, prefer integers over binaries. Rename functions and variable names to better reflect the AMQP 1.0 spec instead of using AMQP 0.9.1 wording.
1 parent 5ad61fe commit b0260cf

14 files changed

+311
-282
lines changed

‎deps/amqp10_client/src/amqp10_client_session.erl‎

Lines changed: 16 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -836,7 +836,7 @@ handle_link_flow(#'v1_0.flow'{delivery_count = MaybeTheirDC,
836836
{uint, DC} -> DC;
837837
undefined -> ?INITIAL_DELIVERY_COUNT
838838
end,
839-
LinkCredit = diff(add(TheirDC, TheirCredit), OurDC),
839+
LinkCredit = amqp10_util:link_credit_snd(TheirDC, TheirCredit, OurDC),
840840
{ok, Link#link{link_credit = LinkCredit}};
841841
handle_link_flow(#'v1_0.flow'{delivery_count = TheirDC,
842842
link_credit = {uint, TheirCredit},
@@ -1219,24 +1219,29 @@ handle_session_flow_pre_begin_test() ->
12191219
?assertEqual(998 - 51, State#state.remote_incoming_window).
12201220

12211221
handle_link_flow_sender_test() ->
1222-
Handle = 45,
1223-
DeliveryCount = 55,
1224-
Link = #link{role = sender, output_handle = 99,
1225-
link_credit = 0, delivery_count = DeliveryCount + 2},
1226-
Flow = #'v1_0.flow'{handle = {uint, Handle},
1227-
link_credit = {uint, 42},
1228-
delivery_count = {uint, DeliveryCount}
1222+
DeliveryCountRcv = 55,
1223+
DeliveryCountSnd = DeliveryCountRcv + 2,
1224+
LinkCreditRcv = 42,
1225+
Link = #link{role = sender,
1226+
output_handle = 99,
1227+
link_credit = 0,
1228+
delivery_count = DeliveryCountSnd},
1229+
Flow = #'v1_0.flow'{handle = {uint, 45},
1230+
link_credit = {uint, LinkCreditRcv},
1231+
delivery_count = {uint, DeliveryCountRcv}
12291232
},
12301233
{ok, Outcome} = handle_link_flow(Flow, Link),
12311234
% see section 2.6.7
1232-
?assertEqual(DeliveryCount + 42 - (DeliveryCount + 2), Outcome#link.link_credit),
1235+
?assertEqual(DeliveryCountRcv + LinkCreditRcv - DeliveryCountSnd,
1236+
Outcome#link.link_credit),
12331237

12341238
% receiver does not yet know the delivery_count
12351239
{ok, Outcome2} = handle_link_flow(Flow#'v1_0.flow'{delivery_count = undefined},
12361240
Link),
12371241
% using serial number arithmetic:
1238-
% ?INITIAL_DELIVERY_COUNT + 42 - (DeliveryCount + 2) = -18
1239-
?assertEqual(-18, Outcome2#link.link_credit).
1242+
% ?INITIAL_DELIVERY_COUNT + LinkCreditRcv - DeliveryCountSnd = -18
1243+
% but we maintain a floor of zero
1244+
?assertEqual(0, Outcome2#link.link_credit).
12401245

12411246
handle_link_flow_sender_drain_test() ->
12421247
Handle = 45,

‎deps/amqp10_common/app.bzl‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ def all_beam_files(name = "all_beam_files"):
1313
"src/amqp10_binary_parser.erl",
1414
"src/amqp10_framing.erl",
1515
"src/amqp10_framing0.erl",
16+
"src/amqp10_util.erl",
1617
"src/serial_number.erl",
1718
],
1819
hdrs = [":public_and_private_hdrs"],
@@ -35,6 +36,7 @@ def all_test_beam_files(name = "all_test_beam_files"):
3536
"src/amqp10_binary_parser.erl",
3637
"src/amqp10_framing.erl",
3738
"src/amqp10_framing0.erl",
39+
"src/amqp10_util.erl",
3840
"src/serial_number.erl",
3941
],
4042
hdrs = [":public_and_private_hdrs"],
@@ -64,6 +66,7 @@ def all_srcs(name = "all_srcs"):
6466
"src/amqp10_binary_parser.erl",
6567
"src/amqp10_framing.erl",
6668
"src/amqp10_framing0.erl",
69+
"src/amqp10_util.erl",
6770
"src/serial_number.erl",
6871
],
6972
)

‎deps/amqp10_common/codegen.py‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -87,8 +87,7 @@ def print_hrl(types, defines):
8787
for opt in d.options:
8888
print_define(opt, d.source)
8989
print("""
90-
-define(DESCRIBED, 0:8).
91-
-define(DESCRIBED_BIN, <<?DESCRIBED>>).
90+
-define(DESCRIBED, 0).
9291
""")
9392

9493

‎deps/amqp10_common/src/amqp10_binary_generator.erl‎

Lines changed: 119 additions & 107 deletions
Original file line numberDiff line numberDiff line change
@@ -61,153 +61,165 @@
6161
-define(DOFF, 2).
6262
-define(VAR_1_LIMIT, 16#FF).
6363

64-
-spec build_frame(integer(), iolist()) -> iolist().
65-
build_frame(Channel, Payload) ->
66-
build_frame(Channel, ?AMQP_FRAME_TYPE, Payload).
64+
-spec build_frame(non_neg_integer(), iolist()) -> iolist().
65+
build_frame(Channel, Body) ->
66+
build_frame(Channel, ?AMQP_FRAME_TYPE, Body).
6767

68-
build_frame(Channel, FrameType, Payload) ->
69-
Size = iolist_size(Payload) + 8, % frame header and no extension
70-
[ <<Size:32/unsigned, 2:8, FrameType:8, Channel:16/unsigned>>, Payload ].
68+
-spec build_frame(non_neg_integer(), non_neg_integer(), iolist()) -> iolist().
69+
build_frame(Channel, FrameType, Body) ->
70+
Size = iolist_size(Body) + 8, % frame header and no extension
71+
[<<Size:32, 2:8, FrameType:8, Channel:16>>, Body].
7172

7273
build_heartbeat_frame() ->
7374
%% length is inclusive
7475
<<8:32, ?DOFF:8, ?AMQP_FRAME_TYPE:8, 0:16>>.
7576

76-
-spec generate(amqp10_type()) -> iolist().
77-
generate({described, Descriptor, Value}) ->
78-
DescBin = generate(Descriptor),
79-
ValueBin = generate(Value),
80-
[ ?DESCRIBED_BIN, DescBin, ValueBin ];
81-
82-
generate(null) -> <<16#40>>;
83-
generate(true) -> <<16#41>>;
84-
generate(false) -> <<16#42>>;
85-
generate({boolean, true}) -> <<16#56, 16#01>>;
86-
generate({boolean, false}) -> <<16#56, 16#00>>;
77+
-spec generate(amqp10_type()) -> iodata().
78+
generate(Type) ->
79+
case generate1(Type) of
80+
Byte when is_integer(Byte) ->
81+
[Byte];
82+
IoData ->
83+
IoData
84+
end.
85+
86+
generate1({described, Descriptor, Value}) ->
87+
DescBin = generate1(Descriptor),
88+
ValueBin = generate1(Value),
89+
[?DESCRIBED, DescBin, ValueBin];
90+
91+
generate1(null) -> 16#40;
92+
generate1(true) -> 16#41;
93+
generate1(false) -> 16#42;
94+
generate1({boolean, true}) -> [16#56, 16#01];
95+
generate1({boolean, false}) -> [16#56, 16#00];
8796

8897
%% some integral types have a compact encoding as a byte; this is in
8998
%% particular for the descriptors of AMQP types, which have the domain
9099
%% bits set to zero and values < 256.
91-
generate({ubyte, V}) -> <<16#50,V:8/unsigned>>;
92-
generate({ushort, V}) -> <<16#60,V:16/unsigned>>;
93-
generate({uint, V}) when V =:= 0 -> <<16#43>>;
94-
generate({uint, V}) when V < 256 -> <<16#52,V:8/unsigned>>;
95-
generate({uint, V}) -> <<16#70,V:32/unsigned>>;
96-
generate({ulong, V}) when V =:= 0 -> <<16#44>>;
97-
generate({ulong, V}) when V < 256 -> <<16#53,V:8/unsigned>>;
98-
generate({ulong, V}) -> <<16#80,V:64/unsigned>>;
99-
generate({byte, V}) -> <<16#51,V:8/signed>>;
100-
generate({short, V}) -> <<16#61,V:16/signed>>;
101-
generate({int, V}) when V<128 andalso V>-129 -> <<16#54,V:8/signed>>;
102-
generate({int, V}) -> <<16#71,V:32/signed>>;
103-
generate({long, V}) when V<128 andalso V>-129 -> <<16#55,V:8/signed>>;
104-
generate({long, V}) -> <<16#81,V:64/signed>>;
105-
generate({float, V}) -> <<16#72,V:32/float>>;
106-
generate({double, V}) -> <<16#82,V:64/float>>;
107-
generate({char, V}) -> <<16#73,V:4/binary>>;
108-
generate({timestamp,V}) -> <<16#83,V:64/signed>>;
109-
generate({uuid, V}) -> <<16#98,V:16/binary>>;
110-
111-
generate({utf8, V}) when size(V) < ?VAR_1_LIMIT -> [<<16#a1,(size(V)):8>>, V];
112-
generate({utf8, V}) -> [<<16#b1,(size(V)):32>>, V];
113-
generate({symbol, V}) -> [<<16#a3,(size(V)):8>>, V];
114-
generate({binary, V}) ->
100+
generate1({ubyte, V}) -> [16#50, V];
101+
generate1({ushort, V}) -> <<16#60,V:16/unsigned>>;
102+
generate1({uint, V}) when V =:= 0 -> 16#43;
103+
generate1({uint, V}) when V < 256 -> [16#52, V];
104+
generate1({uint, V}) -> <<16#70,V:32/unsigned>>;
105+
generate1({ulong, V}) when V =:= 0 -> 16#44;
106+
generate1({ulong, V}) when V < 256 -> [16#53, V];
107+
generate1({ulong, V}) -> <<16#80,V:64/unsigned>>;
108+
generate1({byte, V}) -> <<16#51,V:8/signed>>;
109+
generate1({short, V}) -> <<16#61,V:16/signed>>;
110+
generate1({int, V}) when V<128 andalso V>-129 -> <<16#54,V:8/signed>>;
111+
generate1({int, V}) -> <<16#71,V:32/signed>>;
112+
generate1({long, V}) when V<128 andalso V>-129 -> <<16#55,V:8/signed>>;
113+
generate1({long, V}) -> <<16#81,V:64/signed>>;
114+
generate1({float, V}) -> <<16#72,V:32/float>>;
115+
generate1({double, V}) -> <<16#82,V:64/float>>;
116+
generate1({char, V}) -> <<16#73,V:4/binary>>;
117+
generate1({timestamp,V}) -> <<16#83,V:64/signed>>;
118+
generate1({uuid, V}) -> <<16#98,V:16/binary>>;
119+
120+
generate1({utf8, V}) when size(V) < ?VAR_1_LIMIT -> [16#a1, size(V), V];
121+
generate1({utf8, V}) -> [<<16#b1, (size(V)):32>>, V];
122+
generate1({symbol, V}) -> [16#a3, size(V), V];
123+
generate1({binary, V}) ->
115124
Size = iolist_size(V),
116-
if Size < ?VAR_1_LIMIT -> [<<16#a0,Size:8>>, V];
117-
true -> [<<16#b0,Size:32>>, V]
125+
case Size < ?VAR_1_LIMIT of
126+
true ->
127+
[16#a0, Size, V];
128+
false ->
129+
[<<16#b0, Size:32>>, V]
118130
end;
119131

120-
generate({list, []}) ->
121-
<<16#45>>;
122-
generate({list, List}) ->
132+
generate1({list, []}) ->
133+
16#45;
134+
generate1({list, List}) ->
123135
Count = length(List),
124-
Compound = lists:map(fun generate/1, List),
136+
Compound = lists:map(fun generate1/1, List),
125137
S = iolist_size(Compound),
126138
%% If the list contains less than (256 - 1) elements and if the
127139
%% encoded size (including the encoding of "Count", thus S + 1
128140
%% in the test) is less than 256 bytes, we use the short form.
129141
%% Otherwise, we use the large form.
130142
if Count >= (256 - 1) orelse (S + 1) >= 256 ->
131-
[<<16#d0, (S + 4):32/unsigned, Count:32/unsigned>>, Compound];
132-
true ->
133-
[<<16#c0, (S + 1):8/unsigned, Count:8/unsigned>>, Compound]
143+
[<<16#d0, (S + 4):32, Count:32>>, Compound];
144+
true ->
145+
[16#c0, S + 1, Count, Compound]
134146
end;
135147

136-
generate({map, ListOfPairs}) ->
148+
generate1({map, ListOfPairs}) ->
137149
Count = length(ListOfPairs) * 2,
138150
Compound = lists:map(fun ({Key, Val}) ->
139-
[(generate(Key)),
140-
(generate(Val))]
151+
[(generate1(Key)),
152+
(generate1(Val))]
141153
end, ListOfPairs),
142154
S = iolist_size(Compound),
143-
%% See generate({list, ...}) for an explanation of this test.
155+
%% See generate1({list, ...}) for an explanation of this test.
144156
if Count >= (256 - 1) orelse (S + 1) >= 256 ->
145-
[<<16#d1, (S + 4):32, Count:32>>, Compound];
146-
true ->
147-
[<<16#c1, (S + 1):8, Count:8>>, Compound]
157+
[<<16#d1, (S + 4):32, Count:32>>, Compound];
158+
true ->
159+
[16#c1, S + 1, Count, Compound]
148160
end;
149161

150-
generate({array, Type, List}) ->
162+
generate1({array, Type, List}) ->
151163
Count = length(List),
152-
Body = iolist_to_binary([constructor(Type),
153-
[generate(Type, I) || I <- List]]),
154-
S = size(Body),
155-
%% See generate({list, ...}) for an explanation of this test.
164+
Array = [constructor(Type),
165+
[generate2(Type, I) || I <- List]],
166+
S = iolist_size(Array),
167+
%% See generate1({list, ...}) for an explanation of this test.
156168
if Count >= (256 - 1) orelse (S + 1) >= 256 ->
157-
[<<16#f0, (S + 4):32/unsigned, Count:32/unsigned>>, Body];
158-
true ->
159-
[<<16#e0, (S + 1):8/unsigned, Count:8/unsigned>>, Body]
169+
[<<16#f0, (S + 4):32, Count:32>>, Array];
170+
true ->
171+
[16#e0, S + 1, Count, Array]
160172
end;
161173

162-
generate({as_is, TypeCode, Bin}) ->
174+
generate1({as_is, TypeCode, Bin}) ->
163175
<<TypeCode, Bin>>.
164176

165177
%% TODO again these are a stub to get SASL working. New codec? Will
166178
%% that ever happen? If not we really just need to split generate/1
167179
%% up into things like these...
168180
%% for these constructors map straight-forwardly
169-
constructor(symbol) -> <<16#b3>>;
170-
constructor(ubyte) -> <<16#50>>;
171-
constructor(ushort) -> <<16#60>>;
172-
constructor(short) -> <<16#61>>;
173-
constructor(uint) -> <<16#70>>;
174-
constructor(ulong) -> <<16#80>>;
175-
constructor(byte) -> <<16#51>>;
176-
constructor(int) -> <<16#71>>;
177-
constructor(long) -> <<16#81>>;
178-
constructor(float) -> <<16#72>>;
179-
constructor(double) -> <<16#82>>;
180-
constructor(char) -> <<16#73>>;
181-
constructor(timestamp) -> <<16#83>>;
182-
constructor(uuid) -> <<16#98>>;
183-
constructor(null) -> <<16#40>>;
184-
constructor(boolean) -> <<16#56>>;
185-
constructor(array) -> <<16#f0>>; % use large array type for all nested arrays
186-
constructor(utf8) -> <<16#b1>>;
181+
constructor(symbol) -> 16#b3;
182+
constructor(ubyte) -> 16#50;
183+
constructor(ushort) -> 16#60;
184+
constructor(short) -> 16#61;
185+
constructor(uint) -> 16#70;
186+
constructor(ulong) -> 16#80;
187+
constructor(byte) -> 16#51;
188+
constructor(int) -> 16#71;
189+
constructor(long) -> 16#81;
190+
constructor(float) -> 16#72;
191+
constructor(double) -> 16#82;
192+
constructor(char) -> 16#73;
193+
constructor(timestamp) -> 16#83;
194+
constructor(uuid) -> 16#98;
195+
constructor(null) -> 16#40;
196+
constructor(boolean) -> 16#56;
197+
constructor(array) -> 16#f0; % use large array type for all nested arrays
198+
constructor(utf8) -> 16#b1;
187199
constructor({described, Descriptor, Primitive}) ->
188-
[<<16#00>>, generate(Descriptor), constructor(Primitive)].
200+
[16#00, generate1(Descriptor), constructor(Primitive)].
189201

190202
% returns io_list
191-
generate(symbol, {symbol, V}) -> [<<(size(V)):32>>, V];
192-
generate(utf8, {utf8, V}) -> [<<(size(V)):32>>, V];
193-
generate(boolean, true) -> <<16#01>>;
194-
generate(boolean, false) -> <<16#00>>;
195-
generate(boolean, {boolean, true}) -> <<16#01>>;
196-
generate(boolean, {boolean, false}) -> <<16#00>>;
197-
generate(ubyte, {ubyte, V}) -> <<V:8/unsigned>>;
198-
generate(byte, {byte, V}) -> <<V:8/signed>>;
199-
generate(ushort, {ushort, V}) -> <<V:16/unsigned>>;
200-
generate(short, {short, V}) -> <<V:16/signed>>;
201-
generate(uint, {uint, V}) -> <<V:32/unsigned>>;
202-
generate(int, {int, V}) -> <<V:32/signed>>;
203-
generate(ulong, {ulong, V}) -> <<V:64/unsigned>>;
204-
generate(long, {long, V}) -> <<V:64/signed>>;
205-
generate({described, D, P}, {described, D, V}) ->
206-
generate(P, V);
207-
generate(array, {array, Type, List}) ->
203+
generate2(symbol, {symbol, V}) -> [<<(size(V)):32>>, V];
204+
generate2(utf8, {utf8, V}) -> [<<(size(V)):32>>, V];
205+
generate2(boolean, true) -> 16#01;
206+
generate2(boolean, false) -> 16#00;
207+
generate2(boolean, {boolean, true}) -> 16#01;
208+
generate2(boolean, {boolean, false}) -> 16#00;
209+
generate2(ubyte, {ubyte, V}) -> V;
210+
generate2(byte, {byte, V}) -> <<V:8/signed>>;
211+
generate2(ushort, {ushort, V}) -> <<V:16/unsigned>>;
212+
generate2(short, {short, V}) -> <<V:16/signed>>;
213+
generate2(uint, {uint, V}) -> <<V:32/unsigned>>;
214+
generate2(int, {int, V}) -> <<V:32/signed>>;
215+
generate2(ulong, {ulong, V}) -> <<V:64/unsigned>>;
216+
generate2(long, {long, V}) -> <<V:64/signed>>;
217+
generate2({described, D, P}, {described, D, V}) ->
218+
generate2(P, V);
219+
generate2(array, {array, Type, List}) ->
208220
Count = length(List),
209-
Body = iolist_to_binary([constructor(Type),
210-
[generate(Type, I) || I <- List]]),
211-
S = size(Body),
212-
%% See generate({list, ...}) for an explanation of this test.
213-
[<<(S + 4):32/unsigned, Count:32/unsigned>>, Body].
221+
Array = [constructor(Type),
222+
[generate2(Type, I) || I <- List]],
223+
S = iolist_size(Array),
224+
%% See generate1({list, ...}) for an explanation of this test.
225+
[<<(S + 4):32, Count:32>>, Array].

‎deps/amqp10_common/src/amqp10_framing.erl‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -177,7 +177,7 @@ encode_bin(X) ->
177177
amqp10_binary_generator:generate(encode(X)).
178178

179179
decode_bin(X) ->
180-
[decode(PerfDesc) || PerfDesc <- amqp10_binary_parser:parse_all(X)].
180+
[decode(DescribedPerformative) || DescribedPerformative <- amqp10_binary_parser:parse_all(X)].
181181

182182
symbol_for(X) ->
183183
amqp10_framing0:symbol_for(X).

‎deps/amqp10_common/src/amqp10_util.erl‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
%% This Source Code Form is subject to the terms of the Mozilla Public
2+
%% License, v. 2.0. If a copy of the MPL was not distributed with this
3+
%% file, You can obtain one at https://mozilla.org/MPL/2.0/.
4+
%%
5+
%% Copyright (c) 2007-2023 Broadcom. All Rights Reserved. The term “Broadcom” refers to Broadcom Inc. and/or its subsidiaries. All rights reserved.
6+
%%
7+
8+
-module(amqp10_util).
9+
-include_lib("amqp10_common/include/amqp10_types.hrl").
10+
-export([link_credit_snd/3]).
11+
12+
%% AMQP 1.0 §2.6.7
13+
-spec link_credit_snd(sequence_no(), uint(), sequence_no()) -> uint().
14+
link_credit_snd(DeliveryCountRcv, LinkCreditRcv, DeliveryCountSnd) ->
15+
LinkCreditSnd = serial_number:diff(
16+
serial_number:add(DeliveryCountRcv, LinkCreditRcv),
17+
DeliveryCountSnd),
18+
%% LinkCreditSnd can be negative when receiver decreases credits
19+
%% while messages are in flight. Maintain a floor of zero.
20+
max(0, LinkCreditSnd).

0 commit comments

Comments
 (0)