RFR: 8033426: Scale initial NewSize using NewRatio if not set on command line

[Jon Masamitsu]

Stefan,


1043   static void verify_scaled_initial(size_t initial_heap_size) {
1044     MarkSweepPolicy msp;
1045     msp.initialize_all();
1046
1047     size_t expected = msp.scale_by_NewRatio_aligned(initial_heap_size);
1048     assert(msp.initial_gen0_size() == expected, err_msg("%zu != %zu", msp.initial_gen0_size(), expected));
1049   }



Does the assertion work if you added a check that NewSize was set 
ergonomically?

assert(!FLAG_IS_ERGO(NewSize) || ...)

Jon


On 2/7/2014 4:31 AM, Stefan Johansson wrote:
> Hi again,
>
> I've looked at how to test this in a good way and come up with this 
> simple unit test to verify the basic rules for NewSize. This test 
> doesn't strive to cover all possible combinations of how NewSize 
> interacts with other flags, but instead focus on testing that a few 
> well defined cases work as expected without duplicating a lot of logic 
> from the product.
>
> Please review the updated webrev:
> http://cr.openjdk.java.net/~sjohanss/8033426/webrev.01/
>
> Cheers,
> Stefan
>
> On 2014-02-04 17:08, Stefan Johansson wrote:
>> Hi Jesper,
>>
>> On 2014-02-03 17:50, Jesper Wilhelmsson wrote:
>>> Hi Stefan,
>>>
>>> Have you looked at how this change plays with the ergonomics for 
>>> NewSize in Arguments::set_cms_and_parnew_gc_flags()? That code will 
>>> also use NewRatio to scale NewSize, but if running with a large 
>>> CMSYoungGenPerWorker it may end up with a larger NewSize. Looks like 
>>> your change will handle it well, but it would be nice with some test 
>>> to verify it going forward.
>>>
>> I hadn't looked into this in depth but I took a look now and it seems 
>> to be handled well as you say. If the NewSize is set to something the 
>> new code will never shrink it.
>>
>> Regarding testing, Erik H will soon push his fixes for one of the 
>> HeapSize tests. I will make sure nothing is broken with the improved 
>> test and also see if we can add some test case to it to test the 
>> NewSize.
>>
>>> Anyways, looks good to me.
>>> Ship it!
>> Thanks for reviewing 
>> thishttp://cr.openjdk.java.net/~sjohanss/8033426/webrev.01/.
>>
>> Stefan
>>> /Jesper
>>>
>>>
>>> Stefan Johansson skrev 3/2/14 4:40 PM:
>>>> Hi,
>>>>
>>>> Can I have a couple of reviews for this enhancement:
>>>> https://bugs.openjdk.java.net/browse/JDK-8033426
>>>>
>>>> Webrev:
>>>> http://cr.openjdk.java.net/~sjohanss/8033426/webrev.00/
>>>>
>>>> Summary:
>>>> Currently the initial young generation size is the same as the 
>>>> minimum. If not
>>>> using large pages this will be 1.5M for the default collector 
>>>> regardless of how
>>>> large the whole heap is. The proposed change is to scale the 
>>>> initial young size
>>>> the same way the maximum young size is scaled, using the NewRatio 
>>>> parameter.
>>>>
>>>> Note:
>>>> This change has been out on review as part of the fix for 
>>>> JDK-8028498, but after
>>>> discussions we have decided take another approach for that bug.
>>>>
>>>> Testing:
>>>> * JPRT
>>>> * GC tests in jtreg
>>>>
>>>> Thanks,
>>>> Stefan
>>
>