Short feedback on the linker (CLinker)

[Maurizio Cimadamore]

Hi Johannes,
thanks a lot for the feedback, it's always great to get a pair of fresh 
eyes looking at how things are coming along!

On 21/12/2020 10:52, Johannes Kuhn wrote:
> So, I played a bit around with the CLinker, to see what is possible 
> and what is hard. I used a self-build JDK from the openjdk/jdk 
> repository.
>
> To do this, I picked 3 libraries I had installed on my system:
> * JNI
> * Tcl
> * Win32 api
>
> JNI:
> ----
> This is a bad idea, but someone will inevitably try it, so why 
> shouldn't that someone be me.
>
> Entry point is `JNI_GetCreatedJavaVMs`.
> Works so far, but the returned handles by JNI are immediately invalid. 
> (Somewhat expected, as the native call is over.)
> Interesting is that env->FindClass can throw NoClassDefFoundError, 
> which I could catch.
Interesting experiment, this is definitively an area which could use 
some polishing. While a JNI library is a shared library, its 
requirements are so specific that I think perhaps we should try harder 
to ban JNI library loading from Panama - that said, as always with 
shared library, there's not much we can do other than looking for 
special JNI names (OnLoad) - which might or might not be present. But 
seems an area where some warning triggered by an heuristic (a la 
JNI:check) might be helpful in avoiding mistakes.
>
> What seems to be missing is the ability to use a dynamic address for a 
> downcall to support vtables.
Yes, as discussed in a separate thread, this was on the table, we just 
didn't get enough time to get something in for 16 - but it's 
definitively something we want to get to, and thanks for reminding us of 
this important use case/generalization.
>
> Tcl:
> ----
> Works great so far.
>
> Tcl uses something called ClientData (just a typedef for void*) to 
> pass application specific structures around. It doesn't try to 
> dereference that pointer. The ClientData is only passed to upcalls.
>
> It might be nice to be able to get a "handle" for a java object - 
> otherwise you have to implement the bookkeeping yourself.
>
> It would avoid creating new downcall handles just because you changed one
We have in the past discussed ways to "pin" heap memory, so that heap 
addresses could stay stable across a native call. This would be useful 
to implement logic like the one currently implemented in JNI critical 
sections, but I think it would always help in your case. That said, 
pinning heap objects would need to be made an unsafe operation (e.g. 
foreign restricted), given that it has the potential to undermine the 
correct behavior of the garbage collector, if abused. But yes, obtaining 
pointers from Java objects is something that is on our radar - it's 
technically not too hard to do that - the safety implication are the 
difficult part (as in many other areas of this project).
>
> Win32API:
> ---------
>
> In the past, I often had problems with other FFI and the Win32 api:
> Something was clearing the last error, so a downcall to some function, 
> directly followed by an other downcall to GetLastError would result in 0.
>
> But with Panama, everything works fine - a call to GetLastError indeed 
> returns the error code.
Phew - Windows.h is a really big stress test for all things Panama - I'm 
glad it worked out ok so far (although I wouldn't be surprised if more 
issues would be discovered down the road :-) ).
>
> Conclusion:
> -----------
>
> Impressive work so far. There are a few areas that could get some 
> improvements (vtable support, handles for java objects).
Thanks for the honest feedback - being the first incubation of the 
linker API, we know we're not done - there's work to do in some of the 
areas you describe, as well as some performance work to do on upcalls - 
but I'm happy to hear that what we have seems to be working ok - at 
least for the use cases you have tried.
>
> I also noticed that I tend to declare a few classes with final 
> MemoryAddress as their sole instance field. I hope Project Valhalla 
> will reduce the cost of those type-safe wrappers.

Yeah - I suppose you had to do that for structs - while we could 
auto-generate these wrappers in jextract, for now our driving principle 
for jextract has been one to try not to add any overhead in the 
plumbings that are generated, so that clients could decide how to wire 
things up (and how much to pay for the extra safety provided). I too 
hope that Valhalla will make the cost of implementing such wrappers so 
negligible that at some point we might just be able to flip a switch and 
start generating struct wrappers by default. I've been doing some 
experiments few months ago and, while things look definitively solid (to 
the point I could even declare wrappers for primitive C types), we have 
also observed some performance cliffs (some of which have been fixed). 
So this is an area that we will keep monitoring going forward, as of 
course we'd like to make jextract generated code more useful, if we can 
do so at no performance cost.


Cheers
Maurizio

>
> - Johannes