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