jextract generated code

[Duncan Gittins]

Thanks Maurizio

> If building a project jar from multiple jextract
> runs there are multiple RuntimeHelper+VariadicInvoker per project jar so
> avoiding System.loadLibrary() would also make it easier to plan for a
> single definition of these support classes.

Having re-read my comments, I feel that the paragraph above about having
multiple jextract runs share RuntimeHelper wouldn't be worth the effort
especially if it adds any complexity to the tidy way jextract
currently runs. After all, I could always collate my 5x separate library
jextracts as one to achieve single RuntimeHelper.

Kind regards

Duncan


On Mon, 16 May 2022 at 10:07, Maurizio Cimadamore <
maurizio.cimadamore at oracle.com> wrote:

> (cc'ing jextract-dev)
>
> Thanks Duncan,
> I filed this:
>
> https://bugs.openjdk.java.net/browse/CODETOOLS-7903190
>
> And linked to your email.
>
> I think all your suggestions are valid and can be implemented (hopefully
> w/o too much hassle).
>
> Some observations below:
>
> > 1) RuntimeHelper.java - downcallHandle(String name, FunctionDescriptor
> > fdesc, boolean variadic).  The isVariadic flag is known at generate time
> > (for my case), so why is this flag passed in for runtime evaluation
> instead
> > of generating a call directly to a variadic-only downcall handler ?
> I think this can be done. I also don't get why, in the virtual downcall
> case (no lookup), we accept a variadic flag, even though we then throw
> if the downcall is variadic. It seems like we can simplify here.
> > 2) RuntimeHelper.java looks like it should be a pure support class but it
> > contains generated code. This makes it difficult to swap out with a
> > compatible version, say one with project specific logging classes. Any
> > reason why not move the System.loadLibrary() calls to a fully generated
> > class such as Constants$root.java (which appears to be referenced before
> > RuntimeHelper is used)? If building a project jar from multiple jextract
> > runs there are multiple RuntimeHelper+VariadicInvoker per project jar so
> > avoiding System.loadLibrary() would also make it easier to plan for a
> > single definition of these support classes.
> If we move lookup stuff to Constants$root, then we will have to tweak
> some of the methods in RuntimeHelper to take a SymbolLookup, as this
> will need to be provided from jextracted code.
>
> This is doable. In the specific case you mention though, e.g. with
> multiple jextract runs, wouldn't doing this just move the problem of
> multiple library loading from RuntimeHelper to Constants$root? (I think
> your argument here is that Constants$root is smaller, so less of an issue?)
>
> Another way to do this would be:
>
> * move loading code and lookup to the main header class. After all, I
> think it makes sense for a library header class to have its own lookup
> object.
> * if we do this, then RuntimeHelper and Constant$root become just dumb
> method/constant holder
> * add a way for a jextract run to re-use helpers and constants from a
> previous jextract round (e.g. --with-helper-path)
>
> Another option would be to put the shared stuff in a jar which is
> created at jextract build time, some sort of jextract runtime API. But
> this is going to make deployment of jextracted applications a bit more
> convoluted I think.
>
> > 3) Generated header file classes - XYZ_h.java    Below is a fragment of
> > generated code for Windows OLE library  (-lole32 --include-function
> > CoCreateInstance with XYZ.h "#include <objbase.h>"). Is there a reason to
> > duplicate requireNonNull(xxxx$MH) calls, when instead CoCreateInstance()
> > could just call CoCreateInstance$MH() ?
>
> >
> > public static MethodHandle CoCreateInstance$MH() {
> > return
> >
> RuntimeHelper.requireNonNull(constants$2.CoCreateInstance$MH,"CoCreateInstance");
> > }
> > public static int CoCreateInstance ( Addressable rclsid,  Addressable
> > pUnkOuter,  int dwClsContext,  Addressable riid,  Addressable ppv) {
> > var mh$ = RuntimeHelper.requireNonNull(constants$2.CoCreateInstance$MH,
> > "CoCreateInstance");
> > ...
> > }
>
> This seems redundant, I agree.
>
>
> Maurizio
>
> >
> > Kind regards
> >
> > Duncan
>