erlang / otp

Erlang/OTP
http://erlang.org
Apache License 2.0
11.32k stars 2.94k forks source link

gen_server:call and alias. is this a bug? #5085

Closed halturin closed 3 years ago

halturin commented 3 years ago

Trying to realize how the feature of an alias is working and found this code a bit questionable...

https://github.com/erlang/otp/blob/master/lib/stdlib/src/gen.erl#L335

%%
%% Send a reply to the client.
%%
reply({_To, [alias|Alias] = Tag}, Reply) when is_reference(Alias) ->
    Alias ! {Tag, Reply}, ok;
reply({_To, [[alias|Alias] | _] = Tag}, Reply) when is_reference(Alias) ->
    Alias ! {Tag, Reply}, ok;
reply({To, Tag}, Reply) ->
    try To ! {Tag, Reply}, ok catch _:_ -> ok end.

is this ok with pattern [alias|Alias]? don't you think it must be [alias,Alias]?

the same here https://github.com/erlang/otp/blob/master/lib/stdlib/src/gen.erl#L225

josevalim commented 3 years ago

@halturin if I understood your question correctly, that is called improper lists: it happens when the last element of the list is not an empty list. For example, [1] is the same as [1 | []], and they are both proper lists. But you can create an improper list such as [1 | 2].

Why would someone do that? The only case I am aware of is for optimizations, where instead of using {alias, Alias}, you use [alias | Alias] and you save one word per allocation.

halturin commented 3 years ago

@josevalim thanks for the explanation. Yeah, I am aware of improper lists. The question is "why" :) especially knowing that lists for the DIST proto are pretty expensive in terms of performance. I did some benchmarking for my framework and it shows how expensive to marshal/unmarshal them for the Erlang (https://github.com/halturin/ergo#benchmarks)

josevalim commented 3 years ago

Oh, good point on the DIST proto.

5> erlang:term_to_binary([1 | 2]).
<<131,108,0,0,0,1,97,1,97,2>>
6> erlang:term_to_binary({1, 2}).
<<131,104,2,97,1,97,2>>

I'd guess the current approach is optimized for memory. In order to optimize for dist, I guess we would either need a presentation for small lists (similar to tuples) or a representation for cons cells?

halturin commented 3 years ago

I would update the internal Ref by adding a flag "alias". This approach wouldn't have had to update gen_server:call code. Its just an idea :)

RaimoNiskanen commented 3 years ago

The internal format of a From tag by gen, used by gen_server, gen_statem, gen_event (and gen_fsm), is apparently [alias|AliasRef]; a cons cell. It is the same both when constructing in gen:do_call/4, and where you found it in gen:reply/2, so I have no doubt about its correctness. The format is probably chosen to minimize memory usage. It is anyway internal and type specified as term().

For the record; only gen code has been changed to handle aliases, not e.g gen_server, gen_statem, ...

To implement a process alias by making something special in a reference() was ruled out during design/implementation:

http://erlang.org/doc/reference_manual/processes.html#process-aliases:

It is not possible to:

  • create an alias identifying another process than the caller.
  • deactivate an alias unless it identifies the caller.
  • look up an alias.
  • look up the process identified by an alias.
  • check if an alias is active or not.
  • check if a reference is an alias.

These are all intentional design decisions relating to performance, scalability, and distribution transparency.