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