RFR: 7903984: Inline upcallHandle method to reduce shared code across multiple generations [v2]
Jorn Vernee
jvernee at openjdk.org
Fri Apr 4 16:09:12 UTC 2025
On Fri, 4 Apr 2025 15:26:40 GMT, Nizar Benalla <nbenalla at openjdk.org> wrote:
>> Please review this patch to inline the upcallHandle method, reducing the amount of shared items across multiple jextract generations.
>>
>> Edit: all tests pass in CI, GitHub failure is unrelated
>>
>> TIA
>
> Nizar Benalla has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains three commits:
>
> - Merge branch 'master' into inline-upcallhandle
>
> # Conflicts:
> # src/main/java/org/openjdk/jextract/impl/HeaderFileBuilder.java
> - remove whitespace
> - inline method
Looks good.
To repeat some of the offline discussion: I don't think `ConstantBootstraps` is the right class to add a helper, since that class contains runtime support methods for implementing dynamic constants. However, if you wanted a MethodHandle constant, you could just use a `CONSTANT_MethodHandle_info` instead. I think if we added a more general API, there would also be a need to support ref kinds (not just invokevirtual).
src/main/java/org/openjdk/jextract/impl/FunctionalInterfaceBuilder.java line 94:
> 92: }
> 93:
> 94: private static final MethodHandle UP$MH = upcallHandle(%2$s.%3$s.class, "apply", $DESC);
Maybe you can inline the arguments into the implementation, since they seem to be constant?
Suggestion:
static MethodHandle upcallHandle() {
try {
return MethodHandles.lookup().findVirtual(%2$s.%3$s.class, "apply", $DESC.toMethodType());
} catch (ReflectiveOperationException ex) {
throw new AssertionError(ex);
}
}
private static final MethodHandle UP$MH = upcallHandle();
-------------
Marked as reviewed by jvernee (Committer).
PR Review: https://git.openjdk.org/jextract/pull/279#pullrequestreview-2743523594
PR Review Comment: https://git.openjdk.org/jextract/pull/279#discussion_r2029086110
More information about the jextract-dev
mailing list