Integrated: 7903437: Improve translation strategy for function pointers
Maurizio Cimadamore
maurizio.cimadamore at oracle.com
Fri Apr 14 14:54:25 UTC 2023
Hi Duncan, as always thanks for the feedback.
Few observation on this topic:
* The PR in [1] will at least ameliorate the situation when it comes to
static initializers being too bloated (and slowing down startup)
* Adding filtering by functional interface seems to be at odds with the
way in which jextract filtering options work - you can filter a C entity
(e.g. a typedef or a function), but not a Java one. So that doesn't
strike me as the obviously correct solution
* There is a certain tension here between two goals: generating
higher-level bindings on the one hand (which led to add functional
interface accessors [2]) and stripping down a jarfile in an arbitrary
fashion. The more "info" is contained in the bindings, the less possible
this stripping process becomes. For instance, it is handy that all
pointers are modelled as "MemorySegment" - if we had `Pointer<Foo>` in
any of the fields, you would need to also carry "Foo" in the resulting
jar file
* I view all these attempts at "trying to reduce the size of the
generated classes" as transient solutions - at some point, when better
techonologies [3] will be available, _all_ of the constant classes
generated by jextract will disappear
* Perhaps jextract should do more to deduplicate functional interfaces
with the same C signature. Right now, since the name of e.g. C
variable/parameter is embedded into the functional interface name, this
is not possible. While this will remain the case for typedefs (which, on
the other hand can be filtered out on-demand), I see no reason as to why
"anonymous" function types shouldn't be translated using a more sensible
naming scheme which allows for deduplication. E.g. assuming in your case
most of the functional interfaces share the same signature, no stripping
down would be required, as jextract will only generate few _distinct_
functional interfaces.
Maurizio
[1] - https://git.openjdk.org/jextract/pull/118
[2] - https://git.openjdk.java.net/panama-foreign/pull/456
[3] - https://openjdk.org/jeps/8209964
On 11/04/2023 19:54, Duncan Gittins wrote:
> 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/20230414/bc73b140/attachment-0001.htm>
More information about the jextract-dev
mailing list