Short feedback on the linker (CLinker)

[Johannes Kuhn]

On 21-Dec-20 12:32, Maurizio Cimadamore wrote:
> 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.

The problem is a bit bigger: Any native code could use JNI during the 
call. In my experiment, I obtained the entry point with

     LibraryLookup.ofDefault().lookup("JNI_GetCreatedJavaVMs")

There is not much you could do to guard against this - as this call 
might be down in the native code. Or just use `GetProcAddress` from 
win32 if you forbid looking up "JNI_*". Or it's linux equivalent.

The better option is: What guarantees do you make if someone does that?
A sentence in the spec that says "Using JNI using a downcall is not 
supported, it may or may not work." could help.

>>
>> 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.

Yes, saw that thread. For now, I cache an internal "Functions" object 
that has final MethodHandle fields. It works, but is clumsy.

>>
>> 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).

No objections to making it restricted.
There is just a few things to consider:
* The handle might have a long lifetime - possibly until the application 
exits.
* Tcl provides an upcall to "free" the data. In C, a function pointer to 
free is a good implementation (if the pointer came from malloc). This 
mode should be supported.

(This mode would be surprisingly easy to implement using JNI - with 
NewGlobalRef & DeleteGlobalRef.)

>>
>> 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 :-) ).

The problem sometimes are "non-errors". For example in named pipes. To 
figure out if the connection comes from a remote machine, you call 
"GetNamedPipeClientComputerName" - if there is no error, then it's 
remote. If there is a specific error (don't remember which), then it's 
local.

Haven't played much with it, basically just "I get the right error if I 
pass NULL to some function".

>>
>> 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.

Rarely structs. Mostly ordinary pointers.
Just to provide a type-safe, object oriented interface.
They have a few methods - they usually just call a MethodHandle with 
this.base as first argument. (And the usual allocation stuff for 
CStrings, out parameters, and error checking...)

- Johannes

> 
> 
> Cheers
> Maurizio
> 
>>
>> - Johannes

PS.: What is the best way to handle out parameters?
For now I allocate a new segment every time - is that fine?