RFC: Refactoring SystemABI
Jorn Vernee
jbvernee at xs4all.nl
Thu Nov 22 21:30:07 UTC 2018
I don't think the story for upcalls is thought out enough and we should
talk some more about this.
Here's my current idea of how this should look:
1.) All the generic binder code really needs is the Pointer to the
upcall stub, which will be wrapped in a Callback, and the Scope that the
Callback belongs to needs a way to free the stub when the scope is
closed.
So I think the interface could just be:
interface UpcallHandle {
Pointer<?> entryPoint();
void free();
}
When the binder is asked to allocate a Callback it will call SystemABI
and get one of these UpcallHandles. It will wrap the Pointer in a
Callback object, and store the UpcallHandle instance in a list to be
freed later when the scope is closed.
2.) In the ABI implementation layer, when the ABI is asked for an
UpcallHandle, it will create an UpcallHandle**r** and pass a reference
to that to the upcall stub generation code, which saves this as a global
JNI handle. The Pointer to the generated stub is put into an
UpcallHandle and returned from SystemABI. Then when the upcall happens
the static method UpcallHandler.invoke will be the initial target of the
upcall, which will take the UpcallHandler instance embedded in the stub,
and then delegate to that. This UpcallHandler instance saves a reference
to the receiver (a functional interface instance) and target method
which are called after boxing up the arguments into Java carriers.
3.) 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. So I
think the ABI should also have a method like `Object
getFunctorForStub(Pointer<?> ptr);` which takes the role of what
`UpcallHandler.getUpcallHandler` is doing right now; It retrieves the
UpcallHandler instance that's embedded in the stub, and asks it for the
functor object that it stores, or otherwise returns null. I think for
this last one to work all UpcallHandler implementations should have a
common super type to expose this receiver object.
Does this all make sense? What do you think?
Cheers,
Jorn
Jorn Vernee schreef op 2018-11-22 16:07:
> As another comment. I don't think you should rename UpcallHandler ->
> UpcallHandle everywhere. I think we want to keep the current
> UpcallHandler, but then also add an UpcallHandle interface like
> Maurizio suggested:
>
>> interface UpcallHandle {
>>
>> Object receiver();
>>
>> void free()
>>
>> MethodHandle target();
>>
>> Pointer<?> entryPoint();
>>
>> }
>
> Which is used by UpcallHandler. The UpcallHandle would be the
> SystemABI API equivalent of MethodHandle for upcalls, and the
> UpcallHandler would be an implementation class that uses the
> UpcallHandle to do it's work (work being boxing/unboxing from
> UpcallContext and calling the target method).
>
> UpcallHandle would also be used when unboxing a Callback to pass to
> native, i.e. `callback.getUpcallHandle().entryPoint().addr()`.
>
> That said, I'm not sure that SystemABI needs to be moved to the public
> java.foreign package like Maurizio suggested. I think it's fine moving
> it to jdk.internal.foreign, but the UpcallHandle interface is not
> something that users of the public API should really have to touch
> imho (what if they free the upcall stub too soon?).
>
> Jorn
>
> Henry Jen schreef op 2018-11-22 09:21:
>> Updated webrev[1]
>>
>> - Added NativeMethodType and use it in SystemABI. This also allow us
>> to have varargs downcall MH from SystemABI considering that different
>> platform may handle varargs differently.
>> - UpcallHandler is now named UpcallHandle, and is returned by
>> SystemABI for upcallStub.
>>
>> Cheers,
>> Henry
>>
>> [1]
>> https://cr.openjdk.java.net/~henryjen/panama/SystemABI/webrev.01/webrev/
>>
>>> On Nov 16, 2018, at 2:05 PM, Jorn Vernee <jbvernee at xs4all.nl> wrote:
>>>
>>> Comments inline...
>>>
>>> Henry Jen schreef op 2018-11-16 18:50:
>>>>> On Nov 16, 2018, at 8:23 AM, Maurizio Cimadamore
>>>>> <maurizio.cimadamore at oracle.com> wrote:
>>>>> Hi Jorn,
>>>>> what you propose is a possible refinement; I've been noodling over
>>>>> this myself when writing the SystemABI interface in an attempt to
>>>>> find a way to reduce the number of parameters in the SystemABI
>>>>> calls, to make the API simpler. But I then decided to strive for
>>>>> minimality, at least for now.
>>>>> That said, creating the "native" equivalent of MethodType is
>>>>> something that makes total sense, and we should explore this path
>>>>> further.
>>>> I think that’s Function + MethodType, not sure if we need an
>>>> abstraction on top of those two. Create another type to hold two
>>>> information seems an overkill for me.
>>>
>>> I dont't think MethodType is good enough. That would treat a
>>> Pointer<MyStruct> as just a Pointer, since MethodType uses Class. You
>>> can get the layout of the struct from the `Function`, but you
>>> wouldn't know which Java carrier type you should actually use for it.
>>>
>>> But LayoutTypeImpl saves the pointee type if you create a pointer
>>> type [1], so that retains the information.
>>>
>>>>> As for adding the 'binding' method directly in the Library.Symbol
>>>>> method, I feel less sanguine about it; if you have a method in the
>>>>> SystemABI interface like:
>>>>> MethodHandle downcallHandle(Symbol s, FunctionType ft)
>>>> I gave this a thought, I am leaning toward to keep it as is. Note
>>>> that
>>>> SystemABI gave the final method handle which should have all the
>>>> arguments actually passed into the call, varargs should have been
>>>> expanded. So FunctonType for SystemABI don’t really give us extra
>>>> value except we need to compose such object from the argument
>>>> binding.
>>>
>>> Right. NativeInvoker.of, which creates VarargsInvoker, is currently
>>> being called in HeaderImplGenerator and CallbackImplGenerator, and
>>> they pass on the generics information by using j.l.r.Method. But if
>>> we want to move these calls into SystemABI as well then I think we
>>> want another way to pass on that information. Since not every binding
>>> operation will have a Method for it, just relying on Method will not
>>> work for that, and we can't rely on just MethodType for the same
>>> reason I gave above; it loses the generics information.
>>>
>>> There are also some differences between ABIs when it comes to
>>> varargs. For instance, the CallingSequence generation mechanism on
>>> windows needs to know exactly which arguments are being passed as
>>> varargs to determine which registers to use, but on SysV we don't
>>> care about that. I'm thinking maybe the creation of VarargsInvoker
>>> should be pushed behind SystemABI as well. Then windows can pass on
>>> which arguments are varargs in an independent implementation from the
>>> SysV ABI.
>>>
>>> With that, having varargs information being passed to SystemABI also
>>> makes sense, since SystemABI needs to decided if it wants to create a
>>> VarargsInvoker or not (and maybe which kind of VarargsInvoker).
>>>
>>> Jorn
>>>
>>> [1] :
>>> http://hg.openjdk.java.net/panama/dev/file/28b312ac55b5/src/java.base/share/classes/jdk/internal/foreign/memory/LayoutTypeImpl.java#l100
>>>
>>>> On the other hand, varargs method handle need to be composed at
>>>> higher
>>>> level, where I think Function + MethodType or FunctionType makes
>>>> more
>>>> sense. Should we have this in the SystemABI as a default method or
>>>> have it as an factory method in Symbol, I think I prefer Symbol.
>>>> Cheers,
>>>> Henry
>>>>> I guess that would achieve the same effects? That is, if we have a
>>>>> FunctionType then I would use it to simplify all signatures in the
>>>>> SystemABI interface, at which point creating a MH from a symbol is
>>>>> pretty straightforward.
>>>>> Maurizio
>>>>> On 16/11/2018 16:50, Jorn Vernee wrote:
>>>>>> Hi,
>>>>>> I like the idea of moving away from the dependency on
>>>>>> `j.l.r.Method`, I see that in a lot of places you've started using
>>>>>> `LayoutType<?> ret, LayoutType<?>... args`. I really like this,
>>>>>> and I think it could use it's own wrapper type. I had experimented
>>>>>> earlier with a draft of this:
>>>>>> public class FunctionType {
>>>>>> private final LayoutType<?> return;
>>>>>> private final LayoutType<?>[] args;
>>>>>> private final boolean isVararags;
>>>>>> private FunctionType(LayoutType<?> return, LayoutType<?>[]
>>>>>> args, boolean isVararags) {...}
>>>>>> public static FunctionType make(LayoutType<?> ret,
>>>>>> LayoutType<?>... args) {...}
>>>>>> public static FunctionType makeVarargs(LayoutType<?> ret,
>>>>>> LayoutType<?>... args) {...}
>>>>>> public static FunctionType ofMethod(Method m, Function layout)
>>>>>> {...}
>>>>>> // public static FunctionType ofAnnotatedMethod(Method m) {...}
>>>>>> // for later when we have per-method annotations
>>>>>> public LayoutType<?> returnType() {...}
>>>>>> public LayoutType<?>[] paramTypes() {...}
>>>>>> public boolean isVarargs() {...}
>>>>>> public MethodType toMethodType() {...}
>>>>>> public Function toFunction(){...}
>>>>>> }
>>>>>> This could maybe later also be used as a way to bind individual
>>>>>> symbols without having to create an interface to bind to:
>>>>>> class Library.Symbol {
>>>>>> ...
>>>>>> public MethodHandle bindAsFunction(FunctionType type) {...}
>>>>>> }
>>>>>> This doesn't work when using `Class`, since you lose generic
>>>>>> information; `Pointer<Byte>` -> `Pointer`, but `LayoutTypeImpl`
>>>>>> saves this information and makes it accessible through
>>>>>> `pointeeType()`, and the API for creating them is currently really
>>>>>> nice; `LayoutType<?> lt = NativeTypes.UINT8.pointer();` (similar
>>>>>> with Array as well).
>>>>>> What do you think?
>>>>>> Jorn
>>>>>> Maurizio Cimadamore schreef op 2018-11-16 11:06:
>>>>>>> Hi Henry,
>>>>>>> the work looks like a good initial step. But I think you didn't
>>>>>>> go all
>>>>>>> the way through with it. That is, the binder code
>>>>>>> (HeaderImplGenerator
>>>>>>> and ScopeImpl) still rely on UpcallHandler/NativeInvoker. I think
>>>>>>> it
>>>>>>> would be nice if we could completely decouple these things. After
>>>>>>> all,
>>>>>>> everything that HeaderImplGenerator needs is a way to get a
>>>>>>> method
>>>>>>> handle for a given downcall, and that can be done directly
>>>>>>> through
>>>>>>> SystemABI, now.
>>>>>>> For upcalls its a bit harder, given that for an upcall there's
>>>>>>> currently some info associated to it:
>>>>>>> * receiver object
>>>>>>> * address of the generated stub
>>>>>>> * target method handle
>>>>>>> plus a way to 'free' the stub generated by the VM.
>>>>>>> I think the best way to get there is to create an intermediate
>>>>>>> abstraction that holds these info (plus a free() method), call it
>>>>>>> 'UpcallHandle' (removed the final 'R') and then let the binder
>>>>>>> program
>>>>>>> against this. Then, a Callback object is essentially an
>>>>>>> UpcallHandle +
>>>>>>> a Java carrier (functional interface) + a scope.
>>>>>>> But UpcallHandler as it is now, is not a mere abstraction
>>>>>>> describing
>>>>>>> an upcall - it also embodies the strategy by which the upcall is
>>>>>>> performed (e.g. direct vs. universal) in the same way a
>>>>>>> NativeInvoker
>>>>>>> does. So I think that moving forward we would probably want to
>>>>>>> just
>>>>>>> have SystemABI select the right NativeInvoker/UpcallHandler to do
>>>>>>> the
>>>>>>> job - and these things don't have to be superclasses, or induce a
>>>>>>> hierarchy of any kind - e.g. it would be ok for a DirectInvoker
>>>>>>> to be
>>>>>>> completely unrelated from a UniversalInvoker. As long as the rest
>>>>>>> of
>>>>>>> the binder only interacts with SystemABI, that's ok.
>>>>>>> Maurizio
>>>>>>> On 15/11/2018 22:44, Henry Jen wrote:
>>>>>>>> Hi,
>>>>>>>> Please have a look at the webrev[1] refactor the SystemABI APIs
>>>>>>>> based on Mauricio’s proposal. SystemABI is suppose to
>>>>>>>> encapsulate platform-specific details, this patch make
>>>>>>>> NativeInvoker/UpcallHander and CalllingSequence to be
>>>>>>>> implementation details, and thus can further be
>>>>>>>> simplified/removed if we need to.
>>>>>>>> The binder suppose to only interact with SystemABI, that’s not
>>>>>>>> completed yet in this webrev, but follow up work should get us
>>>>>>>> there.
>>>>>>>> Concept of CallingConvention is introduced as an abstraction for
>>>>>>>> now and is currently simply support default.
>>>>>>>> Anyway, it’s just first step, and I would like to see if it make
>>>>>>>> sense to us.
>>>>>>>> Cheers,
>>>>>>>> Henry
>>>>>>>> [1]
>>>>>>>> http://cr.openjdk.java.net/~henryjen/panama/SystemABI/webrev/
More information about the panama-dev
mailing list