[prev in list] [next in list] [prev in thread] [next in thread] 

List:       openjdk-hotspot-runtime-dev
Subject:    Re: RFR(S): 8213000: Obsolete the IgnoreUnverifiableClassesDuringDump vm option
From:       Calvin Cheung <calvin.cheung () oracle ! com>
Date:       2018-10-31 21:05:27
Message-ID: 5BDA1917.2050607 () oracle ! com
[Download RAW message or body]

Thanks David and Jiangli.
I've pushed the change.

thanks,
Calvin

On 10/30/18, 9:00 PM, David Holmes wrote:
> Hi Calvin,
>
> On 31/10/2018 1:42 PM, Calvin Cheung wrote:
>> On 10/30/18, 6:29 PM, David Holmes wrote:
>>> Hi Calvin,
>>>
>>> Looks good.
>>>
>>> Only comment is whether the test should be looking for output that 
>>> confirms the non-verifiable class was found and elided from the 
>>> archive?
>> How about adding additional check on the output as follows?
>>
>>          // Unverifiable classes won't be included in the CDS archive.
>>          // Dumping should not fail.
>>          OutputAnalyzer output = TestCommon.dump(jar, appClasses);
>>          output.shouldHaveExitValue(0);
>> *        if (output.getStdout().contains("Loading clases to share")) {
>>              // last entry in appClasses[] is a verifiable class
>>              for (int i = 0; i < (appClasses.length - 1); i++) {
>>                  output.shouldContain("Verification failed for " + 
>> appClasses[i]);
>>                  output.shouldContain("Removed error class: " + 
>> appClasses[i]);
>>              }
>>          }*
>
> Seems reasonable.
>
> Thanks,
> David
>
>> thanks,
>> Calvin
>>>
>>> Thanks,
>>> David
>>>
>>> On 31/10/2018 9:58 AM, Calvin Cheung wrote:
>>>>
>>>>
>>>> On 10/30/18, 4:21 PM, Jiangli Zhou wrote:
>>>>> Hi Calvin,
>>>>>
>>>>> The change looks good, except the remaining use of 
>>>>> -IgnoreUnverifiableClassesDuringDump, which is also brought up in 
>>>>> the closed review.
>>>> Thanks for your review.
>>>> I've updated the VerifierTest.java test in the following updated 
>>>> webrev:
>>>>      http://cr.openjdk.java.net/~ccheung/8213000/webrev.01/
>>>>
>>>> thanks,
>>>> Calvin
>>>>>
>>>>> Thanks,
>>>>>
>>>>> Jiangli
>>>>>
>>>>>
>>>>> On 10/30/18 3:31 PM, Calvin Cheung wrote:
>>>>>> bug: https://bugs.openjdk.java.net/browse/JDK-8213000
>>>>>>
>>>>>> webrev: http://cr.openjdk.java.net/~ccheung/8213000/webrev.00/
>>>>>>
>>>>>> Please review this change for obsoleting the diagnostic 
>>>>>> IgoreUnverifiableClassesDuringDump vm option in JDK12.
>>>>>> With the change, if the option is specified in the command line, 
>>>>>> the user will see the following message:
>>>>>>
>>>>>> Java HotSpot(TM) 64-Bit Server VM warning: Ignoring option 
>>>>>> IgnoreUnverifiableClassesDuringDump; support was removed in 12.0
>>>>>>
>>>>>> Testing:
>>>>>>     Ran all CDS and AppCDS tests locally on linux-x64.
>>>>>>     hs-tier{1,2,3} testing in progress.
>>>>>>
>>>>>> thanks,
>>>>>> Calvin
>>>>>
[prev in list] [next in list] [prev in thread] [next in thread]