Request for review (S): 8005396: Use ParNew with only one thread instead of DefNew as default for CMS on single CPU machines

[Bengt Rutisson]

Hi Jon,

Thanks for looking at this!

On 12/21/12 11:43 PM, Jon Masamitsu wrote:
> http://cr.openjdk.java.net/~brutisso/8005396/webrev.00/src/share/vm/runtime/arguments.cpp.frames.html 
>
>
> 1098       FLAG_SET_DEFAULT(ParallelGCThreads, par_gc_threads);
>
> I'd suggest using FLAG_SET_ERGO instead of FLAG_SET_DEFAULT.

Yes, I was thinking about that too. First I had FLAG_SET_ERGO, but then 
I thought that it is kind of the default value for ParallelGCThreads to 
be based on the number of CPUs in the system. Either way is fine with 
me. Would you like me to change it to ergo?

> http://cr.openjdk.java.net/~brutisso/8005396/webrev.00/src/share/vm/gc_implementation/parNew/parNewGeneration.cpp.frames.html 
>
>
> 1627 bool ParNewGeneration::in_use() {
> 1628   return UseParNewGC;
> 1629 }
>
> Is "in_use()" useful anymore?  It was invented to avoid a direct
> use of ParallelGCThreads.   Maybe just replace it with UseParNewGC?

Right. I wasn't quite sure if I should make that cleanup as part of this 
change too. But I think you are right. Here is an updated webrev:

http://cr.openjdk.java.net/~brutisso/8005396/webrev.01/

Thanks,
Bengt

>
> The rests looks good.
>
> Jon
>
> On 12/21/2012 5:42 AM, Bengt Rutisson wrote:
>>
>> Hi All,
>>
>> Can I have a couple of reviews for this change?
>>
>> http://cr.openjdk.java.net/~brutisso/8005396/webrev.00/
>>
>> Currently we use ParNew as default for the young generation when CMS 
>> is selected. But if the machine only has a single CPU we set the 
>> ParallelGCThreads to 0 and and select DefNew instead of ParNew.
>>
>> As part of another change, 8003820, we will deprecate the DefNew + 
>> CMS combination. Thus, it does not make sense anymore to have this 
>> selected by default. This fix is to make CMS always pick ParNew by 
>> default.
>>
>> The change also has the side effect that the, in my opinion, rather 
>> strange behavior that setting ParallelGCThreads=0 on the command line 
>> overrides the GC choice. I would expect this command line to give me 
>> ParNew, but it actually gives me DefNew:
>>
>> -XX:+UseParNewGC -XX:ParallelGCThreads=0
>>
>> After my proposed change you get ParNew with the above command line.
>>
>> I have done some performance testing to verify that ParNew with one 
>> thread is not slower than DefNew. The details are in the bug report:
>>
>> http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=8005396
>>
>> but as a summary it can be said that there is no noticeable difference.
>>
>> I am also running some more SPECjbb2005 runs and will analyze the gc 
>> times.
>>
>> Thanks,
>> Bengt