[erlang-patches] Allow limited pattern matching on magic binaries

Evax Software contact@REDACTED
Tue Apr 12 15:57:36 CEST 2011 

Thanks for developing on this, my comments are inlined.

Le mardi 12 avril 2011 à 12:19 +0200, Sverker Eriksson a écrit :
> It seems you have a good grasp of the implementation. There are some 
> design issues though, that need to be addressed. Here are the things 
> that crawled out of the top of my head:
> 
> Do we really need to support the distinction == vs =:=? Would it not be 
> enough with just a cmp-function and let a return value of 0 imply both 
> == and =:=.

You're right, a cmp function should be enough.

> Comparing two magic binaries with different cmp-functions?
> Comparing magic binary vs other term?
> The property (A < B and B < C implies A < C) must be true for all types 
> and values of terms A,B,C (relying on that resource type cmp-functions 
> are correctly implemented). Having magic binaries compare as empty 
> binaries toward other terms will break that property. One solution is to 
> let a magic binary compare larger than all normal terms, that would make 
> it behave like a normal binary with a very long prefix of 
> <<255,255,255,255,255,...>>.

It's quite tricky here, indeed.
Each distinct cmp function would define a specific magic binary subspace
in the same way a distinct destructor does.
If we have A < B with A and B from the same magic binary space (that is
sharing the same cmp function), and C some other term, then the property
holds: A < C (with A comparing here as empty binary just as B was
before)
What's subtly breaking, though, is when C is a magic binary from another
subspace: we'd have B == C, A == C, but clearly not A == B.
So I don't think we need to change how magic binary compare to other
terms, but rather to introduce some arbitrary order between magic binary
subspaces, so that the last example becomes indeed A < B, B < C and A <
C.
The same will hold for NIF resources, and it gets even trickier. As we
introduce a new nested comparison level (the nif_resource_cmp function
calling user defined callbacks), here again we'd need an arbitrary order
between subspaces.
The problem is that if on one hand magic binary users in the Erlang
source code are few, unlikely to change and thus and easily ordered, on
the other hand NIF resource types are potentially infinite and what's
worse potentially different from node to node.

> 
> What does the language buffs say? Are we passing the line of language 
> extension? Is this a new language type that needs an eep? Or can we 
> argue that magic binaries are still opaque terms that are not 
> distinguishable from other terms.
> 

IMHO it would be much simpler if magic binaries were left as they are
and NIF resources gained there own type. Also because doing as exposed
above, we'd be introducing this whole cmp mechanism at the magic binary
level for the sole real use of NIF resources.

> enif_open_resource_type? I guess the only decent way out is to create a 
> new function enif_open_resource_type_2 (or a more clever name maybe?) 
> and keep the old one for backward compatibility. Instead of new function 
> pointer argument(s), it could take a pointer to a new struct 
> ErlNifResourceInit where the callbacks are placed. That will make it 
> possible to add new callbacks in future (serialize, print,?) without 
> having to create new function names each time.
> 
> And then of course test cases and documentation.
> 

That's fine

> I hope I didn't discourage you too much :-)
> 

Not totally yet :)

Evax

> /Sverker, Erlang/OTP
> 
> 
> Evax Software wrote:
> > Thanks for your answer and enlightening explanations.
> > I'm fine with your proposal, and willing to implement it.
> >
> > Here's what I have in mind:
> >
> > * erts/emulator/beam/global.h:
> > add cmp and eq entries to ErtsMagicBinary and the corresponding
> > ERTS_MAGIC_BIN_{CMP,EQ} macros
> >
> > * erts/emulator/beam/erl_binary.h:
> > update erts_create_magic_binary to handle those two extra callbacks
> >
> > * erts/emulator/beam/utils.c:
> > update cmp and eq to use the corresponding callback if we have two magic
> > binaries and the callbacks match
> >
> > * Add cmd and eq callbacks to the enif_resource_type_t struct, implement
> > nif_resource_cmp and nif_resource_eq and modify enif_open_resource_type
> > to take the extra two parameters (erts/emulator/beam/erl_nif.c)
> >
> > So the changes will only introduce a small memory overhead and two
> > functions' signature updates.
> >
> > As erts_create_magic_binary is only used internally, it's easy enough to
> > update all users.
> > On the other hand enif_open_resource_type is widely used in existing
> > NIFs. I believe these NIFs should made to compile unmodified against the
> > new code. So, if you think the approach is right, how do you want me to
> > handle the API change for this function ?
> >
> > Evax
> >
> > Le lundi 11 avril 2011 à 12:22 +0200, Sverker Eriksson a écrit :
> >   
> >> Thank you for your explanation, but your patch was not rejected because 
> >> of lack of understanding. (Hopefully the opposite)
> >>
> >> I understand the need to match NIF resources. But the simple solution of 
> >> just comparing the (magic) binary data is just not enough as it may 
> >> cause very strange behavior that violates the functional properties of 
> >> the language.
> >>
> >> For example, the binary content of NIF resources are allowed to be 
> >> mutated. Your binary comparison will expose such a mutation and violate 
> >> the single assignment property. A variable referring a NIF resource may 
> >> suddenly compare different to what it did earlier.
> >>
> >> Another (too) simple solution could be to compare the pointers *to* the 
> >> magic binaries instead of their content. That would on the other hand 
> >> introduce a comparison that is based on identity (rather than equality), 
> >> which we otherwise don't have in the language.
> >>
> >> I think the right way to go, is to introduce a new callback for resource 
> >> types, that allows the NIF resource designer to implement how 
> >> comparisons should be done.
> >>
> >> /Sverker, Erlang/OTP Ericsson
> >>
> >>
> >>
> >>
> >>
> >> Evax Software wrote:
> >>     
> >>> Ok, this patch has just been rejected.
> >>>
> >>> Maybe I was unclear as to the goal of the patch, I'll try to explain it
> >>> a bit better.
> >>>
> >>> NIF resources are implemented as magic binaries.
> >>> Magic binaries are special binaries holding private data but presenting
> >>> an apparent length of zero.
> >>>
> >>> I understand that this is a good thing as the private data is not
> >>> exposed directly to the Erlang side, but this has a special side effect
> >>> for NIF resource users: any resource will match any other resource (even
> >>> from a different resource type, as well of course as the empty binary.
> >>>
> >>> This is somehow dangerous as it prevents us from catching some errors
> >>> from the Erlang side, where for example assigning a resource to a
> >>> variable already bound to a resource will NOT cause an error.
> >>>
> >>> Moreover not being able to distinguish NIF resources from Erlang causes
> >>> some extra work. Imagine a NIF project implementing some kind of
> >>> socket-like mechanism (erlzmq2 or libev-erlang for example), and imagine
> >>> a process handling several of these sockets in a receive loop (as in
> >>> active mode), then one must wrap resources in a tuple with a ref to be
> >>> able to match on them.
> >>>
> >>> The patch implements special cases for magic binaries in the eq and cmp
> >>> operations, causing the private data to be compared, and magic binaries
> >>> with identical private data to match.
> >>>
> >>> So, and maybe the patch name was badly chosen, there is no real
> >>> sub-binary pattern matching happening on private data here, it's just a
> >>> simple and straightforward way to tell resources apart, still without
> >>> exposing the private data to the Erlang side.
> >>>
> >>> Evax
> >>>
> >>> Le jeudi 17 mars 2011 à 11:59 +0100, Henrik Nord a écrit :
> >>>   
> >>>       
> >>>> On 03/16/2011 09:41 PM, Evax Software wrote:
> >>>>     
> >>>>         
> >>>>> Hi,
> >>>>>
> >>>>> The proposed patch allows limited pattern matching on magic binaries.
> >>>>> It tries to be the as non-intrusive as possible.
> >>>>> The goal of the patch is to allow matching on NIF resources, but it
> >>>>> shouldn't cause problems to other magic binary beam users.
> >>>>>
> >>>>> git fetch git@REDACTED:evax/otp.git pattern_match_on_magic_binaries
> >>>>>
> >>>>> Evax
> >>>>>
> >>>>>
> >>>>>
> >>>>> ________________________________________________________________
> >>>>> erlang-patches (at) erlang.org mailing list.
> >>>>> See http://www.erlang.org/faq.html
> >>>>> To unsubscribe; mailto:erlang-patches-unsubscribe@REDACTED
> >>>>>    
> >>>>>       
> >>>>>           
> >>>> Hello
> >>>>
> >>>> I have included this in 'pu' and will let it cook for a while.
> >>>>
> >>>> /Henrik Nord, Erlang/OTP
> >>>>
> >>>> ________________________________________________________________
> >>>> erlang-patches (at) erlang.org mailing list.
> >>>> See http://www.erlang.org/faq.html
> >>>> To unsubscribe; mailto:erlang-patches-unsubscribe@REDACTED
> >>>>
> >>>>     
> >>>>         
> >>> _______________________________________________
> >>> erlang-patches mailing list
> >>> erlang-patches@REDACTED
> >>> http://erlang.org/mailman/listinfo/erlang-patches
> >>>
> >>>   
> >>>       
> >>     
> >
> >
> >   
> 
>

More information about the erlang-patches mailing list