RFR (S) JDK-8153578,Default NewRatio is ignored when UseConcMarkSweepGC is used as GC algorithm
Joseph Provino
joseph.provino at oracle.com
Wed May 25 16:19:22 UTC 2016
This is a simple change from MIN2 to MAX2 suggested by Jesper.
The code now matches the comment and seems to fix the problem.
Webrev: http://cr.openjdk.java.net/~jprovino/8153578/webrev.01
thanks.
joe
On 4/20/2016 6:29 PM, Jon Masamitsu wrote:
>
>
> On 04/20/2016 12:35 PM, Jesper Wilhelmsson wrote:
>> 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...
>
> The intent of this code was to control the young pause times by
> limiting the size of
> the young gen. The preferred size scaling with
>
> young_gen_per_worker * ParallelGCThreads
>
> was meant to take into account the fact you could have approximately the
> same pauses with larger heaps as the number of GC workers increases.
> I didn't add this code but I'm pretty sure it was added as a result of
> customer interaction.
>
> Jon
>
>>
>> 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
>>>
>>>
>
More information about the hotspot-dev
mailing list