RFR: 8161604: TestNewSizeFlags fails with RuntimeException: max new size != MaxNewSize value

sangheon sangheon.kim at oracle.com
Wed Jul 27 19:14:07 UTC 2016


Hi Michail,

Sorry for the unclear comment.

On 07/27/2016 11:41 AM, Michail Chernov wrote:
> Hi Sangheon,
>
> Thanks you for reviewing this. I'm no sure that explanations are 
> needed in whitebox.cpp. I added the throwing of RuntimeException if we 
> try to invoke Parallel GC specific methods when use not Parallel GC. 
> Exception is more clear than 0 that was returned from methods. 
I meant adding the explanation near "private static final long 
PS_VIRTUAL_SPACE_ALIGNMENT = WB.psVirtualSpaceAlignment();" in 
TestNewSizeFlags.java. Because someone would be strange as we are 
calling it without checking GC type. But you explained that there's no 
harm with that code.

And I agree that adding something in whitebox.cpp is not necessary. Even 
the new exception. :)

Thanks,
Sangheon


> Existing tests which use that methods are passed with all collectors.
>
> Updated review:
>
> http://cr.openjdk.java.net/~mchernov/8161604/webrev.hotspot.01_to_00/
> http://cr.openjdk.java.net/~mchernov/8161604/webrev.hotspot.01/
>
> Thanks,
> Michail
>
>
> On 07/26/2016 11:19 PM, sangheon wrote:
>> Hi Michail,
>>
>> On 07/22/2016 12:21 PM, Michail Chernov wrote:
>>> Hi,
>>>
>>> Could I have a reviews for this change, please?
>>>
>>> https://bugs.openjdk.java.net/browse/JDK-8161604
>>> http://cr.openjdk.java.net/~mchernov/8161604/webrev.00/
>>> http://cr.openjdk.java.net/~mchernov/8161604/webrev.hotspot.00/
>>>
>>> The problem with test happens on host with huge pages size (64k). 
>>> This causes to 32Mb heap alignment.
>>>
>>> Test now uses heap alignment to check actual sizes of MaxHeapSize 
>>> and InitialHeapSize. In case if they are equal MaxNewSize and 
>>> NewSize are set to same value by the GC ergonomic.
>>>
>>> Added new method to WhiteBox to get heap alignment value. The 
>>> existing methods which are used in test and uses Parallel GC are 
>>> guarded by INCLUDE_ALL_GCS and executed real code only if 
>>> UseParallelGC is set.
>> I assume the test were okay.
>> Looks good and thanks for removing repeated codes.
>>
>> One minor comment is, how about adding above explanation of using 
>> psVirtualSpaceAlignment() is okay regardless of GC type?
>> I don't need a new webrev for this, if you like.
>>
>> Thanks,
>> Sangheon
>>
>>
>>>
>>> Test case for NewSize=0 was removed because it can cause to VM 
>>> initialization error (See 
>>> https://bugs.openjdk.java.net/browse/JDK-8162420).
>>>
>>> Added test case for MaxHeapSize == InitialHeapSize.
>>>
>>> Testing is in progress.
>>>
>>> Thanks,
>>> Michail
>>
>>
>




More information about the hotspot-gc-dev mailing list