Integrated: 7903437: Improve translation strategy for function pointers

[Duncan Gittins]

Regarding the new class dependencies relating to callback interfaces
mentioned here, and the JIRA Jorn raised for functional interfaces for
function pointer types:

    https://mail.openjdk.org/pipermail/jextract-dev/2023-April/000742.html

https://mail.openjdk.org/pipermail/jextract-dev/2023-February/000675.html
    https://bugs.openjdk.org/browse/CODETOOLS-7903456

I have an application that uses IShellLinkWVtbl (and several others).
Currently, referencing IShellLinkWVtbl causes class load for every one of
the related callback class files: QueryInterface AddRef Release GetPath
GetIDList  SetIDList  GetDescription SetDescription GetWorkingDirectory
SetWorkingDirectory GetArguments SetArguments GetHotkey SetHotkey
GetShowCmd SetShowCmd GetIconLocation SetIconLocation SetRelativePath
Resolve SetPath

With the JIRA I will be able to exclude the callbacks I don't need
(QueryInterface AddRef Release already defined in IUnknown) leaving me with
the others. I build a jar from jextract code one but use it in different
applications. But on occasions they may only use the GetX calls, sometimes
GetX and SetX, sometimes SetX.

I think the jextract code generation should go one step further and handle
loading these functional interfaces for function pointer types on demand at
the time they are used, not loading all at first reference of parent class
IShellLinkWVtbl. Perhaps something like this for each callback interface -
this would encapsulate all the fields of the functional interface together
and mean any access to the parent class IShellLinkWVtbl doesn't load all
the other callbacks unless the caller uses them, and not at first reference
of parent class such as IShellLinkWVtbl:

public interface QueryInterface {


static final FunctionDescriptor QueryInterface$FUNC = FunctionDescriptor.of
(Constants$root.C_LONG$LAYOUT,

Constants$root.C_POINTER$LAYOUT,

Constants$root.C_POINTER$LAYOUT,

Constants$root.C_POINTER$LAYOUT

);

int apply(java.lang.foreign.MemorySegment _x0,
java.lang.foreign.MemorySegment _x1, java.lang.foreign.MemorySegment _x2);

static final MethodHandle UP$MH = RuntimeHelper.upcallHandle(QueryInterface.
class, "apply", QueryInterface$FUNC);

static MemorySegment allocate(QueryInterface fi, SegmentScope scope) {

return RuntimeHelper.upcallStub(QueryInterface.UP$MH, fi,
QueryInterface$FUNC, scope);

}

static final MethodHandle DOWN$MH = RuntimeHelper.downcallHandle(

QueryInterface$FUNC

);

static QueryInterface ofAddress(MemorySegment addr, SegmentScope scope) {

MemorySegment symbol = MemorySegment.ofAddress(addr.address(), 0, scope);

return (java.lang.foreign.MemorySegment __x0,
java.lang.foreign.MemorySegment __x1, java.lang.foreign.MemorySegment __x2)
-> {

try {

return (int)QueryInterface.DOWN$MH.invokeExact(symbol, __x0, __x1, __x2);

} catch (Throwable ex$) {

throw new AssertionError("should not reach here", ex$);

}

};

}

}


Kind regards

Duncan



On Fri, 17 Feb 2023 at 23:57, Maurizio Cimadamore <mcimadamore at openjdk.org>
wrote:

> On Fri, 17 Feb 2023 15:32:53 GMT, Maurizio Cimadamore <
> mcimadamore at openjdk.org> wrote:
>
> > The current translation stragegy for function pointers is not very
> efficient. Every time a lambda expression is turned into an upcall stub, a
> new method handle lookup is performed, which is quite a slow operation.
> >
> > This patch improves the translation strategy so that the lookup is only
> performed once. For instance, considering the following `typedef`:
> >
> >
> > typedef void (*cb)(int, int);
> >
> >
> > jextract will generate the following functional interface:
> >
> >
> > import static java.lang.foreign.ValueLayout.*;
> > /**
> >  * {@snippet :
> >  * void (*cb)(int,int);
> >  * }
> >  */
> > public interface cb {
> >
> >     void apply(int _x0, int _x1);
> >     static MemorySegment allocate(cb fi, Arena scope) {
> >         return RuntimeHelper.upcallStub(constants$0.cb_UP$MH, fi,
> constants$0.cb$FUNC, scope);
> >     }
> >     static cb ofAddress(MemorySegment addr, Arena arena) {
> >         MemorySegment symbol = addr.reinterpret(arena.scope(), null);
> >         return (int __x0, int __x1) -> {
> >             try {
> >                 constants$0.cb_DOWN$MH.invokeExact(symbol, __x0, __x1);
> >             } catch (Throwable ex$) {
> >                 throw new AssertionError("should not reach here", ex$);
> >             }
> >         };
> >     }
> > }
> >
> >
> > Where the constants are defined as follows:
> >
> >
> >     static final FunctionDescriptor cb$FUNC = FunctionDescriptor.ofVoid(
> >         Constants$root.C_INT$LAYOUT,
> >         Constants$root.C_INT$LAYOUT
> >     );
> >
> >     static final FunctionDescriptor cb_UP$FUNC =
> FunctionDescriptor.ofVoid(
> >         Constants$root.C_INT$LAYOUT,
> >         Constants$root.C_INT$LAYOUT
> >     );
> >
> >     static final MethodHandle cb_UP$MH =
> RuntimeHelper.upcallHandle(cb.class, "apply", constants$0.cb_UP$FUNC);
> >
> >     static final FunctionDescriptor cb_DOWN$FUNC =
> FunctionDescriptor.ofVoid(
> >         Constants$root.C_INT$LAYOUT,
> >         Constants$root.C_INT$LAYOUT
> >     );
> >
> >     static final MethodHandle cb_DOWN$MH = RuntimeHelper.downcallHandle(
> >         constants$0.cb_DOWN$FUNC
> >     );
> >
> >
> > And, where `RuntimeHelper::upcallHandler` is defined as follows:
> >
> >
> > static MethodHandle upcallHandle(Class<?> fi, String name,
> FunctionDescriptor fdesc) {
> >         try {
> >             return MH_LOOKUP.findVirtual(fi, name, fdesc.toMethodType());
> >         } catch (Throwable ex) {
> >             throw new AssertionError(ex);
> >         }
> >     }
> >
> >
> > This allows to perform the method handle lookup only once. This means
> that when creating an upcall stub, we only have to `bind` the functional
> interface instance to the method handle, and then create an upcall stub
> with the resulting handle. Both operations should be fast after the first
> time, because of caching.
>
> This pull request has now been integrated.
>
> Changeset: dfafbbe7
> Author:    Maurizio Cimadamore <mcimadamore at openjdk.org>
> URL:
> https://git.openjdk.org/jextract/commit/dfafbbe7d6ab3c190dae5519236bef718fd10755
> Stats:     41 lines in 4 files changed: 28 ins; 2 del; 11 mod
>
> 7903437: Improve translation strategy for function pointers
>
> Reviewed-by: jvernee
>
> -------------
>
> PR: https://git.openjdk.org/jextract/pull/109
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.org/pipermail/jextract-dev/attachments/20230411/ce9f3246/attachment.htm>