iilyak commented on code in PR #4812:
URL: https://github.com/apache/couchdb/pull/4812#discussion_r1731576347
##########
src/rexi/src/rexi.erl:
##########
@@ -328,3 +336,23 @@ drain_acks(Count) ->
after 0 ->
{ok, Count}
end.
+
+get_delta() ->
+ {delta, couch_stats_resource_tracker:make_delta()}.
+
+maybe_add_delta(T) ->
+ case couch_stats_resource_tracker:is_enabled() of
+ false ->
+ T;
+ true ->
+ add_delta(T, get_delta())
+ end.
+
+add_delta({A}, Delta) -> {A, Delta};
Review Comment:
There is also https://www.erlang.org/doc/apps/erts/erlang#append_element/2.
However in the way it is written there might be an opportunity for structural
sharing. Since in this case the new tuple is constructed from existing
pointers. I am not sure whether VM would eliminate copying in this case.
I did a quick experiment just to see what gets emited.
```erlang
% erlc +time +to_core add_delta.erl
-module(add_delta).
-export([test/0]).
add_delta0({A}, Delta) -> {A, Delta};
add_delta0({A, B}, Delta) -> {A, B, Delta}.
add_delta1(T, Delta) when 0 < erlang:size(T), erlang:size(T) < 8 ->
erlang:insert_element(erlang:size(T) + 1, T, Delta);
add_delta1(T, _Delta) -> T.
add_delta2(T, Delta) -> erlang:append_element(T, Delta).
test() ->
{1,2,3, {delta, 4}} = add_delta0({1,2,3}, {delta, 4}),
{1,2,3, {delta, 4}} = add_delta1({1,2,3}, {delta, 4}),
{1,2,3, {delta, 4}} = add_delta2({1,2,3}, {delta, 4}),
ok.
```
The resulting core erlang is
```
module 'add_delta' ['module_info'/0,
'module_info'/1,
'test'/0]
attributes [%% Line 1
'file' =
%% Line 1
[{[97|[100|[100|[95|[100|[101|[108|[116|[97|[46|[101|[114|[108]]]]]]]]]]]]],1}]]
'add_delta0'/2 =
%% Line 6
( fun (_0,_1) ->
( case <_0,_1> of
<{A},Delta> when 'true' ->
{A,Delta}
%% Line 7
<{A,B},Delta> when 'true' ->
{A,B,Delta}
( <_3,_2> when 'true' ->
( primop 'match_fail'
(( {'function_clause',_3,_2}
-| [{'function',{'add_delta0',2}}] ))
-| [{'function',{'add_delta0',2}}] )
-| ['compiler_generated'] )
end
-| [{'function',{'add_delta0',2}}] )
-| [{'function',{'add_delta0',2}}] )
'add_delta1'/2 =
%% Line 9
( fun (_0,_1) ->
( case <_0,_1> of
<T,Delta>
when try
let <_2> =
call 'erlang':'size'
(T)
in let <_3> =
call 'erlang':'<'
(0, _2)
in let <_4> =
call 'erlang':'size'
(T)
in let <_5> =
call 'erlang':'<'
(_4, 8)
in call 'erlang':'and'
(_3, _5)
of <Try> ->
Try
catch <T,R> ->
'false' ->
let <_6> =
call %% Line 10
'erlang':%% Line 10
'size'
(%% Line 10
T)
in let <_7> =
call %% Line 10
'erlang':%% Line 10
'+'
(%% Line 10
_6, %% Line 10
1)
in %% Line 10
call 'erlang':'insert_element'
(_7, T, Delta)
%% Line 11
<T,_X_Delta> when 'true' ->
T
end
-| [{'function',{'add_delta1',2}}] )
-| [{'function',{'add_delta1',2}}] )
'add_delta2'/2 =
%% Line 13
( fun (_0,_1) ->
call 'erlang':'append_element'
(_0, _1)
-| [{'function',{'add_delta2',2}}] )
'test'/0 =
%% Line 15
( fun () ->
%% Line 16
case apply 'add_delta0'/2
({1,2,3}, {'delta',4}) of
<{1,2,3,{'delta',4}}> when 'true' ->
%% Line 17
case apply 'add_delta1'/2
({1,2,3}, {'delta',4}) of
<{1,2,3,{'delta',4}}> when 'true' ->
%% Line 18
case apply 'add_delta2'/2
({1,2,3}, {'delta',4}) of
<{1,2,3,{'delta',4}}> when 'true' ->
%% Line 19
'ok'
( <_2> when 'true' ->
primop 'match_fail'
({'badmatch',_2})
-| ['compiler_generated'] )
end
( <_1> when 'true' ->
primop 'match_fail'
({'badmatch',_1})
-| ['compiler_generated'] )
end
( <_0> when 'true' ->
primop 'match_fail'
({'badmatch',_0})
-| ['compiler_generated'] )
end
-| [{'function',{'test',0}}] )
'module_info'/0 =
( fun () ->
call 'erlang':'get_module_info'
('add_delta')
-| [{'function',{'module_info',0}}] )
'module_info'/1 =
( fun (_0) ->
call 'erlang':'get_module_info'
('add_delta', ( _0
-| [{'function',{'module_info',1}}] ))
-| [{'function',{'module_info',1}}] )
end
```
In the first case there is a direct reuse of tuple elements. Most likely no
data copying.
In the second case there are calls to size, construction of a new frame in
try/catch and `'erlang':'insert_element'`. AFAIK the
`'erlang':'insert_element'`(BIF written in C) copies the values
https://github.com/erlang/otp/blob/1afaa459eaae8d51585acc2a363dd2714b98c9c2/erts/emulator/beam/bif.c#L2866.
In the third case `'erlang':'append_element'` which is a call to BIF written
in C. This BIF copies the values
https://github.com/erlang/otp/blob/1afaa459eaae8d51585acc2a363dd2714b98c9c2/erts/emulator/beam/bif.c#L2833
I think the way it is written would be the fastest. I think Russell wanted
to optimize this code path. If I am right Russell could you add a comment about
it. Something like "The tuple match is used to enable subterm sharing and avoid
data copying.".
Also there might be a middle ground.
```erlang
add_delta({A, B}, Delta) -> {A, B, Delta};
add_delta({A, B, C}, Delta) -> {A, B, C, Delta};
add_delta({A, B, C, D}, Delta) -> {A, B, C, D, Delta};
add_delta({A, B, C, D, E}, Delta) -> {A, B, C, D, E, Delta};
add_delta({A, B, C, D, E, F}, Delta) -> {A, B, C, D, E, F, Delta};
add_delta({A, B, C, D, E, F, G}, Delta) -> {A, B, C, D, E, F, G, Delta};
add_delta(T, Delta) when is_tuple(T) -> erlang:append_element(T, Delta).
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
