RFC: Refactoring SystemABI

[Maurizio Cimadamore]

(as also observed by Jorn), Pulling even more on this string, I think we 
can drop receiver object from the getUpcallHandle method alltogether.

The current API looks like this:

UpcallHandle upcallStub(Object receiver, MethodHandle target, 
NativeMethodType nmt);

I think we can simplify to this:

UpcallHandle upcallStub(MethodHandle target, NativeMethodType nmt);


In fact, the method already takes a MethodHandle - so we could require 
clients to pass in a 'bound' method handle, where the receiver object 
has already been 'attached' to the target MH. This will also make the 
SystemABI more flexible too, because, while in the case of the binder 
there is _always_ a receiver object (the receiver is an instance of a 
functional interface obtained from some lambda expression passed to the 
Scope::allocateCallback), in general this need not be the case - and one 
could envision cases where clients would want a callback to point to 
some static method with the right signature.

So, I think handling the receiver on the binder side (by binding it to 
the MH) is the right thing to do.

So, with all the simplifications discussed in this thread, the API would 
become:

Pointer<?> upcallStub(MethodHandle target, NativeMethodType nmt);

That is, gone is the UpcallHandle abstraction.

Additionally, we will have to add a new method to SystemABI:

void freeUpcallStub(Pointer<?>)

Which is used by Scope::close.


As mentioned in an earlier message, the downcall logic in SystemABI will 
have to special case the case where a stub entry point is passed. In 
such cases, the ABI will recover the method handle associated with the 
stub (we currently do so by saving a Java object in the stub itself - 
but other ways are possible too, e.g. global hashmap), and return the 
(already bound) method handle, which will already feature the right 
signature.


This will eliminate code paths duplication in CallbackImplGenerator and 
also CallbackImpl - where currently we do different things depending on 
whether the underlying pointer is a stub.

I think this looks good?

Maurizio


On 23/11/2018 02:06, Maurizio Cimadamore wrote:
>
> On 23/11/2018 01:47, Maurizio Cimadamore wrote:
>> What is it that bothers you about this approach? 
>
> Let me clarify - I like that you are thinking of ways to reduce the 
> surface of the API - but it seems that moving a method (getReceiver) 
> from UpcallHandle to SystemABI doesn't buy all that much.
>
> I personally prefer having a simple UpcallHandle lookup method in the 
> ABI, and then a way to get form the Handle to the receiver as it seems 
> more composable this way. Right now, true, we always need to do both 
> things at the same time (to speed up things in the upcall machinery), 
> but maybe having a 'are you a stub' predicate will turn out handy even 
> on its own (in case the user wants to write some pointer-walking code).
>
> But, if we really wanted to save some API estate, I think a better 
> approach (if viable) would be to completely hide the stub detection 
> logic. As you said very clearly yourself, we have this issue:
>
> "There is a shortcut being taken when native code gives us a Pointer 
> back to one of our upcall stubs, and we want to call it from Java. In 
> that case we don't really need to call into native, to call our own 
> upcall stub to then call back into Java to call our target method."
>
> This is the reason behind the 'isStub/getReceiver' logic. But let's 
> analyze this use case more in detail: we have a pointer to function, 
> and we want to do a downcall using that pointer as the entry point. 
> But this means that, in order to do the downcall, we will call 
> SystemABI.downcallHandle (again). So, I think it is theoretically 
> possible that the ABI would say: "hey, I know this guy!" and instead 
> of giving you a MethodHandle that 'goes down' it gives you a 
> MethodHandle that stays in Java-land. And the binder would not even know.
>
> If my conjecture is correct, then we just need your minimal 
> UpcallHandle interface (pointer + free) and no 
> getReceiver/getFunctorObject at all. And, if you pull on this some 
> more, you realize that UpcallHandle is not even needed: just use a 
> Pointer, and add a 'freeUpcallHandle(Pointer<?>)' to the SystemABI. 
> That'd be sweet.
>
> Thoughts?
>
> Maurizio
>