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

[mikhailo]

Thank you,

Consider it reviewed for my part; no need to post a new webrev for the 
added comment.


Misha


On 11/29/2017 03:29 PM, Ioi Lam wrote:
>
>
> 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
>>
>