RFC: Refactoring SystemABI

[Maurizio Cimadamore]

For the records, I took the liberty to put together a patch which 
illustrates how we can get rid of dependencies on receiver object. The 
patch is based on current tip, and does not include other SystemABI 
changes (from Henry) - so that the actual differences will be more visible.

With this, UpcallHandler doesn't have much of an API. The only two 
methods used from outside are getNativeEntryPoint() [called immediately 
after creation] and free(). If, as discussed we move free() in 
SystemABI, then we can model upcalls with just a Pointer<?>. Cool!

http://cr.openjdk.java.net/~mcimadamore/panama/upcall-recv-removal/

(I had to comment a bunch of lines in a test that was checking for == 
equality on functional interface objects)

Maurizio

On 23/11/2018 12:19, Maurizio Cimadamore wrote:
> (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
>>