[foreign] RFR: simplify implementation classes

[Maurizio Cimadamore]

On 14/05/18 06:09, Sundararajan Athijegannathan wrote:
> 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).
Thanks for the comment - here's a revised webrev that used the 
SharedSecrets trick:

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

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