RFR: 7903585: Revisit variadic support [v3]
Jorn Vernee
jvernee at openjdk.org
Fri Dec 1 14:53:02 UTC 2023
> Currently, we only support var args passed to a variadic function through an `Object[]`. From that, we infer the layouts of the arguments, and then link a variadic method handle on the fly.
>
> This is problematic because: 1. there's an ambiguity between structs and pointers, since they both use `MemorySegment` as a carrier. 2. from a performance perspective, re-linking the method handle every time is somewhat costly.
>
> The approach proposed by this patch is to add a variadic invoker factory method, which can, given a list of layouts, be used to create an instance of a variadic invoker (interface), that embeds a downcall handle specialized to a particular type of the variadic function. The user specifies the layout explicitly, so there is no ambiguity (1), and the user can cache the returned invoker and re-use it instead of re-linking the function on every invocation (2).
>
> The current layout inferring method can then be re-implemented on top of this factory.
>
> Question: I'm currently generating the doc comment on both the inferring method and the invoker factory. Is this good? Should it be on one or the other? Should it be on the invoker interface as well?
>
> ---
>
> There were some issues I ran into while developing this patch, that I fixed:
> - We had some methods in the tests that take no arguments, but were declared without a prototype, leading to them getting translated as variadic functions in the bindings. I've change those parameter lists to `(void)` (this popped out when I only had the invoker factory in an earlier iteration)
> - I noticed that jtreg was sometimes not recompiling a test class in the generator tests after the bindings changed, leading to weird NoSuchMethodErrors. I've fixed this by adding an explicit `@build` step to the tests.
> - I've added `-retain:fail,error` to the jtreg arguments, so that the generated sources for a failing test are not deleted immediately, and can be manually inspected after a test failure.
> - I noticed we weren't generating a layout for `C_BOOL`, because we don't treat that type as being supported. I think this is a latent issue. I've added Bool as a supported type to fix this.
>
> Also: I've ret-conned Test8244959 and renamed it to TestPrintf. And added new cases for the invoker.
Jorn Vernee has updated the pull request incrementally with one additional commit since the last revision:
drop holder class for base desc
-------------
Changes:
- all: https://git.openjdk.org/jextract/pull/149/files
- new: https://git.openjdk.org/jextract/pull/149/files/a8c387c8..603d829c
Webrevs:
- full: https://webrevs.openjdk.org/?repo=jextract&pr=149&range=02
- incr: https://webrevs.openjdk.org/?repo=jextract&pr=149&range=01-02
Stats: 4 lines in 1 file changed: 0 ins; 2 del; 2 mod
Patch: https://git.openjdk.org/jextract/pull/149.diff
Fetch: git fetch https://git.openjdk.org/jextract.git pull/149/head:pull/149
PR: https://git.openjdk.org/jextract/pull/149
More information about the jextract-dev
mailing list