[foreign] RFR 8212560 : jextract should generate a static forwarder class

Samuel Audet samuel.audet at gmail.com
Thu Oct 18 21:49:01 UTC 2018 

Here I'm concerned about usability. Having to look at a bunch of 
irrelevant Offset annotations and/or weird layouts specification is a 
usability issue, but not a big one. The problem with jextract is that it 
needs to generate them by /parsing/ the header files, which is an 
error-prone process, especially for function-like macros and C++, as you 
tell me those are still goals. It doesn't make sense to want to parse 
random files when it's possible to specify manually but more easily 
everything in the target language. It just complicates things! (BTW, Go 
or Swift don't do function-like macros or C++ so they can afford to tell 
everyone to just parse everything). If jextract could have 2 mode of 
operations, one where it parses the header files, and another one, where 
it can generate the annotations at runtime, that's fine, but that's not 
how it works currently. (Which reminds me, we can probably do all that 
stuff at build time the way Lombok does it: https://projectlombok.org/ )

This is especially relevant for C++ template libraries like Boost, 
Eigen, Ceres Solver, or Thrust. Just writing out the few declarations 
manually is often easier than figuring out whatever hacks we can come up 
to specify some types and how to map them, for example:
https://github.com/bytedeco/javacpp/wiki/Interface-Thrust-and-CUDA

This really depends on the priorities of the project though. I really 
think Panama should concentrate on getting things like linkToNative 
ready in the JDK and leave things like jextract and Lombok-like 
functionality to others because we can do it outside the JDK, but that's 
just my opinion.

Samuel

On 10/18/2018 11:45 PM, Maurizio Cimadamore wrote:
> Hi Samuel,
> I'm really confused here; if the various annotated interfaces/artifacts 
> are generated in a jar bundle by jextract, what is the problem with 
> layout annotations? It's not like the user will see them?
> 
> So, if you are concerned about usabilty (e.g. what the client see, as in 
> your 'main' method) using a different kind of annotation won't change 
> anything.
> 
> If you are concerned with the (one time) runtime cost to parse the 
> layout annotations, we have already covered how the best way to address 
> that concern is by running jextract as a jlink at module install time 
> (as we do for other similar tasks in the JDK build itself). That will 
> create binding implementations for all the required interfaces 
> _statically_ which should help with startup.
> 
> Maurizio
> 
> On 17/10/2018 19:02, Samuel Audet wrote:
>> And while we're at it, I think the runtime component could also take 
>> care of abstracting away the details of the layouts, so gettimeofday() 
>> could look something like this, again without having to parse anything 
>> for simple cases:
>>
>>     @Struct
>>     interface Timeval {
>>         @SizeT long tv_sec();
>>         Timeval tv_sec(long tv_sec);
>>
>>         long tv_usec();
>>         Timeval tv_usec(long tv_usec);
>>     }
>>
>>     interface Dummy {
>>         int gettimeofday(Timeval tv, Pointer timezone);
>>     }
>>
>>     import static Dummy.*;
>>
>>     public static void main(String[] args) {
>>     InitMagicBindings(Dummy.class, Timeval.class);
>>         Timeval t = Timeval.create()
>>     gettimeofday(t, null);
>>     }
>> }
>>
>> Now that would be awesome :)
>>
>> Samuel
>>
>> On 10/18/2018 09:16 AM, Samuel Audet wrote:
>>> That looks good, thanks Sundar! Though, eventually, it would be great 
>>> to have those generated at runtime somehow, so that we don't need to 
>>> start parsing stuff when we just need to call single functions, like 
>>> we can do with JNA or JavaCPP, for example:
>>> https://gist.github.com/saudet/1bf14a000e64c245675cf5d4e9ad6e69#file-nativebenchmark-java 
>>>
>>>
>>> Samuel
>>>
>>> On 10/18/2018 02:35 AM, Sundararajan Athijegannathan wrote:
>>>> Updated: 
>>>> http://cr.openjdk.java.net/~sundar/8212560/webrev.02/index.html
>>>>
>>>> Added --static-forwarder boolean option (default true)
>>>>
>>>> -Sundar
>>>>
>>>> On 17/10/18, 9:55 PM, Sundararajan Athijegannathan wrote:
>>>>>
>>>>> comments below..
>>>>>
>>>>> On 17/10/18, 9:13 PM, Jorn Vernee wrote:
>>>>>> Sundararajan Athijegannathan schreef op 2018-10-17 14:37:
>>>>>>> On option: do we need an explicit option? given that one file extra
>>>>>>> and that too only when -l is specified. (too many options 
>>>>>>> already? ;)
>>>>>>> )
>>>>>>
>>>>>> Maybe having an option is nice, but have it be turned on by 
>>>>>> default? That way if someone really wanted to not generate the 
>>>>>> forwarder they could, and for the general use case they don't have 
>>>>>> to bother with setting an option every time.
>>>>>>
>>>>> okay.
>>>>>
>>>>>>> On naming: we've derived automatic names elsewhere. I think we could
>>>>>>> revisit all naming options by another round - perhaps even java
>>>>>>> convention for header interface name.
>>>>>>>
>>>>>>> On subclassing: Nice suggestion. I made it as AsmCodeFactoryExt 
>>>>>>> class
>>>>>>> and extended it from AsmCodeFactory. visit methods now return 
>>>>>>> Boolean
>>>>>>> to tell if the particular tree was handled or not. This is needed so
>>>>>>> that subclass can avoid the same tree if super class avoided it (for
>>>>>>> eg. function-like macros, repeated definitions..)
>>>>>>
>>>>>> I'm wondering if this runs into trouble later on when adding more 
>>>>>> code generation extensions/options. Let's say you add `foo` and 
>>>>>> `bar` options later on. You'd have to make an extension subclass 
>>>>>> for every combination of options (StaticFooBar, StaticFoo, 
>>>>>> StaticBar, FooBar, Static, Foo, Bar).
>>>>>>
>>>>>> Maybe you could have something like a `CodeFactory`, which 
>>>>>> consists of an `AsmCodeFactory` and a list of `CodeFactoryExt`, of 
>>>>>> which the static forwarder generator would be one. And have this 
>>>>>> kind of pattern:
>>>>>>
>>>>>> ```
>>>>>> @Override
>>>>>> public Void visitVar(VarTree varTree, JType jt) {
>>>>>>     if(asmCodeFactory.visitVar(varTree, jt)) {
>>>>>>         for(CodeFactoryExt cfe : extensions) {
>>>>>>             cfe.visitVar(varTree, jt);
>>>>>>         }
>>>>>>     }
>>>>>>     return null;
>>>>>> }
>>>>>> ```
>>>>>>
>>>>>> I think that also allows you to get rid of the if/else statements 
>>>>>> in the extension class.
>>>>>>
>>>>>> It's not really needed right now since there is only 1 extension, 
>>>>>> but it might be nice idea for the future.
>>>>>
>>>>> Right. We don't want to go there yet - but yes, if we happened to 
>>>>> add one more, we can revisit & refactor the code.
>>>>>
>>>>> -Sundar
>>>>>>
>>>>>> Jorn
>>>>>>
>>>>>>> Updated: 
>>>>>>> http://cr.openjdk.java.net/~sundar/8212560/webrev.01/index.html
>>>>>>>
>>>>>>> Thanks,
>>>>>>> -Sundar
>>>>>>>
>>>>>>> On 17/10/18, 3:25 PM, Maurizio Cimadamore wrote:
>>>>>>>> I like it! Few questions/comments:
>>>>>>>>
>>>>>>>> * should it be enabled/disabled with explicit option
>>>>>>>> * should the name of the statics class be customizable
>>>>>>>> * I like the code organization - have you thought of pushing it 
>>>>>>>> further and make StaticForwarderGenerator a _subclass_ of 
>>>>>>>> AsmCodeFactory - each visitor could maybe delegate to 
>>>>>>>> super.visitXyz() first and then do its own bit? Then when you 
>>>>>>>> setup the pipeline, depending on what info is available on the 
>>>>>>>> context (e.g. presence of -l) you can decide whether to use a 
>>>>>>>> 'bare' ASMCodeFactory or the 'embellished' one. That should 
>>>>>>>> remove all the 'if staticFwd != null ...' sections from 
>>>>>>>> AsmCodeFactory.
>>>>>>>>
>>>>>>>> Maurizio
>>>>>>>>
>>>>>>>>
>>>>>>>> On 17/10/2018 06:35, Sundararajan Athijegannathan wrote:
>>>>>>>>> Please review.
>>>>>>>>>
>>>>>>>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8212560
>>>>>>>>> Webrev: 
>>>>>>>>> http://cr.openjdk.java.net/~sundar/8212560/webrev.00/index.html
>>>>>>>>>
>>>>>>>>> Thanks,
>>>>>>>>> -Sundar
>>>
>>

More information about the panama-dev mailing list