RFR[S] 8191927 Enable AppCDS for custom loaders on all 64-bit Linux and AIX

[Ioi Lam]

On 11/29/17 2:15 PM, mikhailo wrote:
> Hi Ioi,
>
>   Overall the change looks good. I have just one comment:
>
> 1. In test/lib/Platform.java you note in the comment that the 
> condition should match one in 
> ClassListParser::load_class_from_source(). However, there is not 
> reverse reference. That is, there is no note/comment in 
> ClassListParser::load_class_from_source() to update 
> test.lib.Platform.areCustomLoadersSupportedForCDS() if the condition 
> changes in load_class_from_source().
>
> I can think of couple of ways to address this:
>
> A. Simple quick way, just add the comment in classListParser.cpp that 
> test library should be updated if this condition is updated.
>
> B. Add a white box API similar to WhiteBox.isCDSIncludedInVmBuild(); 
> and have a method in VM that would compute the result based on the 
> platform conditionals.
>
> The option B is more robust in my view, but I will not have strong 
> objection to option A either. Also, I understand that option B can 
> result in some sort of complex dependencies.
>

Hi Misha,

Option B will require that the VM has -XX:+WhiteBoxAPI and the 
whitebox.jar in the classpath, but the Platform class can be used 
without these conditions. So I don't think B is doable. I'll add the 
comments as you suggested in A.

Thanks
- Ioi

>
> Thank you,
>
> Misha
>
>
>
>
> On 11/27/2017 01:42 PM, Ioi Lam wrote:
>> Hi,
>>
>> Please review this change to support AppCDS on all 64-bit Linux 
>> platforms as well as AIX.
>>
>>     https://bugs.openjdk.java.net/browse/JDK-8191927
>> http://cr.openjdk.java.net/~iklam/jdk10/8191927-appcds-custom-loader-for-more-platforms.v01/ 
>>
>>
>> The patch must be applied on top of the AppCDS patch:
>>
>> http://cr.openjdk.java.net/~iklam/jdk10/8188791-open-appcds.v03/
>>
>> The patch was initially contributed by Volker Simonis. I modified it 
>> a little to avoid repeating the platform enumerations in all the test 
>> cases. Instead, I added a new test property, so the tests that 
>> require AppCDS support for custom loaders can be written as:
>>
>>     * @requires vm.cds.custom.loaders
>>
>> I've tested on Linux manually, and I am now running on all of the 
>> Oracle-supported platforms using our internal test harness.
>>
>> Thanks
>> - Ioi
>