RFR (XXS) JDK-8153578,Default NewRatio is ignored when UseConcMarkSweepGC is used as GC algorithm

[Joe Provino]

So it seems!

On 04/20/2016 03:36 PM, Jesper Wilhelmsson wrote:
> And, by the way, I object to the subject. Changes in argument parsing 
> are never XXS.  ;)
> /Jesper
>
> Den 20/4/16 kl. 21:35, skrev Jesper Wilhelmsson:
>> Hi Joe,
>>
>> If I understand the bug description correctly the problem is that 
>> NewSize
>> becomes too small. According to the bug the VM ignores the NewRatio 
>> setting.
>>
>> Your change removes the setting of MaxNewSize in the case where 
>> NewSize has the
>> default value. It's not obvious to me how that is related to the bug.
>>
>> There is an if statement enclosing the code you are changing. It has 
>> a comment
>> that I find interesting:
>>
>> 1755   // If either MaxNewSize or NewRatio is set on the command line,
>> 1756   // assume the user is trying to set the size of the young gen.
>> 1757   if (FLAG_IS_DEFAULT(MaxNewSize) && FLAG_IS_DEFAULT(NewRatio)) {
>>
>> The interesting part is that the comment says "MaxNewSize OR 
>> NewRatio" but the
>> code says "MaxNewSize AND NewRatio". This could be a typo in the 
>> comment, or it
>> could be related to your bug.
>>
>> I don't think the fix here is to ignore the calculated 
>> preferred_max_new_size,
>> but rather to figure out why it has the wrong value. 
>> preferred_max_new_size is
>> calculated a few lines up, and it is based on NewRatio. The number of 
>> threads
>> seems to be involved as well. Should it be? Usually things based on 
>> the number
>> of threads tend to be wrong...
>>
>> 1748     MIN2(max_heap/(NewRatio+1), 
>> ScaleForWordSize(young_gen_per_worker *
>> ParallelGCThreads));
>>
>> young_gen_per_worker is CMSYoungGenPerWorker which defaults to things 
>> like 16M
>> or 64M. ParallelGCThreads is usually just a handful, 8 on my machine. 
>> Since we
>> take the smallest number of this thread based thing and the NewRatio
>> calculation, I would guess the threads will limit the MaxNewSize 
>> quite a lot. I
>> wonder if this isn't the bug you are looking for. It would make more 
>> sense to me
>> if it was MAX of the two instead of MIN. You should probably consult 
>> whoever
>> wrote this code.
>>
>> /Jesper
>>
>>
>> Den 20/4/16 kl. 20:08, skrev Joseph Provino:
>>> Please review this tiny change.  It only affects ParNew.  Are there any
>>> unintended consequences?
>>>
>>> Passes JPRT.
>>>
>>> CR: JDK-8153578 Default NewRatio is ignored when UseConcMarkSweepGC 
>>> is used as
>>> GC algorithm <https://bugs.openjdk.java.net/browse/JDK-8153578>
>>>
>>> Webrev: http://cr.openjdk.java.net/~jprovino/8153578/webrev.00
>>>
>>>