[foreign] RFR: simplify implementation classes

[Sundararajan Athijegannathan]

Quick comment on java.lang.Runtime change. We can avoid a new public API 
in Runtime by using SharedSecrets pattern 
(jdk.internal.misc.SharedSecrets). I think it is better to avoid 
exposing Libray/Symbol/Pointer etc. via j.l.Runtime.  For now, we could 
have the method in Runtime as pure implementation helper (though we may 
choose to expose a new API method in Runtime class later).

-Sundar

On 11/05/18, 10:45 PM, Maurizio Cimadamore wrote:
> 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
>