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

[Stefan Johansson]

Thanks Jesper,

I will do the suggested cleanup before pushing but won't post an update 
webrev for it.

Cheers,
Stefan

On 2014-02-07 14:56, Jesper Wilhelmsson wrote:
> 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
>>>
>>