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

[Jesper Wilhelmsson]

Stefan,

The new test looks fine.

My internal parser would prefer if you were consistent with spaces vs no spaces 
in expressions like 20*M. I like spaces. I could however live without the empty 
line after the call to restore_flags().

I also fail to parse the comment:
// if NewSize set on command line should take for both min
// and initial if less than min heap.

I think it should say:

// If NewSize is set on the command line, it should be used
// for both min and initial young size if less than min heap.

With a little cleanup, ship it!
/Jesper


Stefan Johansson skrev 7/2/14 1:31 PM:
> 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
>>
>