Better default for ParallelGCThreads and ConcGCThreads by using number of physical cores and CPU mask.

[Bengt Rutisson]

On 11/22/13 6:48 PM, Jon Masamitsu wrote:
>
> On 11/22/2013 4:33 AM, Bengt Rutisson wrote:
>>
>>
>>>
>>>     Do we need the flag AdjustGCThreadsToCores? It seems like this
>>>     is a good change and it is always nice to reduce the number of
>>>     flags we have. If you feel unsure about the change it may be
>>>     good to be able to disable it, but in general I think it would
>>>     be better to decide that this is the default behavior and go
>>>     with it.
>>>
>>>
>>> I have no strong opinion. If you guys can confirm the improvements 
>>> and feel like you don't need the flag, that would be ideal.
>>
>> Yes, this is an interesting dilemma. Personally I don't think I have 
>> the cycles right now to do any performance testing of this fix. But I 
>> assume that you don't have access to enough machines and benchmarks 
>> to test this properly on all supported platforms. I'll bring this up 
>> internally to see if there is any process for handling this type of 
>> situation...
>>
>> Since your change is for Linux only and you have done the 
>> measurements there, I'm fine with your results. I would prefer 
>> skipping the AdjustGCThreadsToCores flag based on the data you 
>> provided. Especially since we can still set ParallelGCThreads 
>> explicitly on the command line if we need to work around a regression 
>> somewhere.
>
> I fully expect to need the AdjustGCThreadsToCores flag on Sparc so I 
> would prefer
> it be left in.

Can you explain why this will be particularly needed on Sparc? I assume 
you mean Linux/Sparc since the Solaris implementation is not changed by 
this patch. If we think this is not a good change for Sparc, then maybe 
we should not do it there...

> Make it an experimental flag to make it easier to remove later
> 1405experimental(bool, AdjustGCThreadsToCores, true,                               \
> 1406           "Create GC worker threads with respect to the number of physical "\
> 1407           "cores available from the CPU mask. Ignores hardware threads")    \
> 1408
>

Yes, this is a bit better, but I still don't fully understand why we 
want to keep the old behavior under a flag. More comments on that below.

> We do have the ParallelGCThreads flag to workaround regressions but 
> users don't
> know what value is currently set by the ergonomics so won't know what 
> value
> to use for ParallelGCThreads to workaround a regression.

It is not too hard to come up with the "old" default value for 
ParallelGCThreads. We know what the logic was so if just get processor 
information from the machine we can figure it out and in worst case we 
can ask a customer to run with an older version of the JDK and do 
"PrintFlagsFinal | grep ParallelGCThreads".

I'm not going to insist that we remove the AdjustGCThreadsToCores flag. 
We can keep it if you feel it is important. Personally I just think the 
cost of having it is higher than the benefit. We have extra code to 
support, more complex logic to take into account when debugging and we 
most likely will not test with this flag enabled so bugs may creep in 
without us noticing.

Bengt

>
> Jon

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.org/pipermail/hotspot-gc-dev/attachments/20131125/214255f4/attachment.htm>