[foreign] RFR: simplify implementation classes

Maurizio Cimadamore maurizio.cimadamore at oracle.com
Fri May 11 17:15:24 UTC 2018


Hi,
it's spring cleaning in Panama-land :-)

Before moving forward with the revised design, I thought it would be 
better to consolidate the internal implementation a bit. So I started 
removing classes that didn't seem necessary - I ended up with a much 
bigger changeset than I anticipated, but I think this is not bad news, 
in fact I believe this is a big simplification over what we had. Here's 
some highlights:

* I removed the PLatform class and all its OS-dependent Host subclasses. 
The reality is that, at least for now, we only support SystemV ABI, and 
little endianness everywhere. So having all these intermediate steps 
seemed way too convoluted (and error prone as we learned earlier this 
week). Now SystemABI has a getInstance method which returns the default 
for that platform. For library loading, see below.

* Errno. This is also gone. Errno should be part of a wider Posix 
implementation effort; there's little value in having it there. It was 
only used by a test, (UnixSystem) which is easily fixable with 3-4 lines 
of code.

* LdLoader/LibraryLoader. This is probably the biggest change. As we've 
discussed we have a lot of custom logic that 'just' loads native 
libraries. In reality the JDK already has code to do this, but the 
problem is that it doesn't expose the library entry point back to the 
Java code. I looked at how we might massage the JDK implementation, and 
it's actually quite easy: ClassLoader stores loaded libraries in a class 
called NativeLibrary, which has a (native) method to lookup entries, 
etc. We can have this class implement our nicl/Library interface, and 
then have ClassLoader.loadLibrary returns one of these.

There's one caveat: when we load  a new library, we need to pass a class 
whose loader is used to 'hook' the native library (so that when the 
classloader is GCed the native library is unloaded). Runtime.loadLibrary 
achieves this with a @CallerSensitive, but that's only useful if the 
client calls that code. Here we also need the binder to call that code - 
so a saner approach is to pass a MethodHandles.Lookup - and use the 
lookup class as the class whose loader is hooked to the native lib. That 
works very well.

* As a result of the above class loader cleanup, we could get rid of 
UnixLibrary and UnixDynamicLibraries too

* BindingRegistry: this is not used, and if we need something like it, 
it will come back in a different shape

* ContainerSizeInfo: this is an interface which is only implemented by 
AbstractABI. Seems like a case of premature abstraction.

* UncheckedPointer: this is used to create pointers w/o bounds - this is 
just a factory method in BoundedPointer (which happened to be there 
already: createNativeVoidPointer).

In total, this patch removes 14 classes, and, crucially, it removes the 
duplication of the library loading logic. I suspect further cleanup is 
also possible in Libraries/LibrariesHelper, but I'll leave this to 
Sundar who's more familiar with that code and its interaction with 
jextract. (I suspect some of the SecurityManager checks are not needed 
anymore, since those would be carried out by the main class loader logic).

Webrev:

http://cr.openjdk.java.net/~mcimadamore/panama/cleanup/

Cheers
Maurizio



More information about the panama-dev mailing list