[foreign] RFR 8212560 : jextract should generate a static forwarder class
Sundararajan Athijegannathan
sundararajan.athijegannathan at oracle.com
Mon Oct 22 02:36:56 UTC 2018
If you want to wait for good perf. numbers, please do so. But we still
need a test framework today - and jextract serves that purpose as of today.
PS. I think we should avoid RFR threads to discuss design/future etc.
That clutters reviews. We could always start a new thread referring to a
specific RFR thread if needed.
Thanks,
-Sundar
On 20/10/18, 6:44 AM, Samuel Audet wrote:
> 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