[foreign] RFR 8212560 : jextract should generate a static forwarder class
Samuel Audet
samuel.audet at gmail.com
Sat Oct 20 01:14:21 UTC 2018
Yes, I talked about that with John and Maurizio offline, which I
mentioned in the other thread trying to make a parallel with Dvorak vs
QWERTY keyboard layouts. I could, for example, have JavaCPP start
targeting the classes from Panama, but the issue here is that I don't
feel Panama will be able to provide a level of performance sufficiently
higher than JNI to justify the cost of switching over, or in maintaining
2 sets of bindings for Android, old versions of the JDK, etc. John says
it would be real easy to make it a lot faster than JNI (but why do we
still have basically nothing after over 4 years?), Maurizio seems less
certain... Who knows, really? Anyway, my point is if you could show me
good numbers, I could be the one to take over the testing here. Because,
as I also said before, that's basically what the JavaCPP Presets are,
they are a testbed for JavaCPP:
https://github.com/bytedeco/javacpp-presets
Samuel
On 10/19/2018 11:32 AM, Sundararajan Athijegannathan wrote:
> I think it has been already mentioned. But it is worth reiterating. Even
> if we're to leave at linkToNative + minimal API, we still need a tool to
> test it all! i.e., jextract is needed if we've to avoid "all manual"
> (and so difficult to maintain, error prone) tests.
>
> -Sundar
>
> On 19/10/18, 3:19 AM, Samuel Audet wrote:
>> 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