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

[Bengt Rutisson]

On 2014-01-13 22:39, Jungwoo Ha wrote:
>
>
>     In CMSCollector there is still this code to change the value for
>     ConcGCThreads based on AdjustGCThreadsToCores.
>
>
>      639       if (AdjustGCThreadsToCores) {
>      640         FLAG_SET_DEFAULT(ConcGCThreads, ParallelGCThreads / 2);
>      641       } else {
>      642         FLAG_SET_DEFAULT(ConcGCThreads, (3 +
>     ParallelGCThreads) / 4);
>      643       }
>
>     Do you think that is needed or can we use the same logic in both
>     cases given that ParallelGCThreads has a different value if
>     AdjustGCThreadsToCores is enabled.
>
>
> I am happy to just use FLAG_SET_DEFAULT(ConcGCThreads, 
> ParallelGCThreads / 2);
> The original hotspot code used FLAG_SET_DEFAULT(ConcGCThreads, (3 + 
> ParallelGCThreads) / 4); which I think is somewhat arbitrary.
> Now that ParallelGCThreads will reduce on some configuration, dividing 
> it into 4 seems to make the ConcGCThreads too small.

Hm. Changing to FLAG_SET_DEFAULT(ConcGCThreads, ParallelGCThreads / 2) 
might be the way to go, but I think that should probably done as a 
separate change. That way we can performance test it more thoroughly.

>
>
>     Also, I don't fully understand the name AdjustGCThreadsToCores. In
>     VM_Version::calc_parallel_worker_threads() for x86 we simply
>     active_core_count with 2 if this flag is enabled. So, the flag
>     does not really adjust to the cores. It seems like it is reduces
>     the number of GC threads. How about calling the flag
>     ReduceGCThreads or something like that?
>
>
> The flag can be named better. However, ReduceGCThreads doesn't seem to 
> reflect what this flag does.
> I am pretty bad at naming, so let me summarize what this flag is 
> actually doing.
>
> The flag adjusts the GC threads to the number of "available" physical 
> cores reported by /proc filesystem and the CPU mask set by 
> sched_setaffinity.
> For example, ParallelGCThreads will remain the same regardless of 
> whether hyperthreading is turned on/off.
> Current hotspot code will have twice more GC threads if hyperthreading 
> is on.
> Usually, GC causes huge number of cache misses, thus having two GC 
> threads competing for the same physical core hurts the GC throughput.
> Current hotspot code doesn't consider CPU mask at all.
> For example, even though the machine has 64 cores, if CPU mask is set 
> for 2 cores, current hotspot calculates the number of GC threads based 
> on 64.
> Thus, this flag is actually evaluating the number of GC threads to the 
> number of physical cores available for the JVM process.

Right. In VM_Version::calc_parallel_worker_threads() we take the value 
of os::active_core_count() and divide it by 2. I guess this is to reduce 
the cache issues. But if the flag is called AdjustGCThreadsToCores I 
would have expected that we set the number of GC threads to be equal to 
the core count. That's why I suggested "Reduce" in the name.

Naming is hard and I am not particularly fond of the name 
ReduceGCThreads either. But maybe we can try to come up with something else?

>
>     I think I pointed this out earlier, but I don't feel comfortable
>     reviewing the changes in os_linux_x86.cpp. I hope someone from the
>     Runtime team can review that.
>
>
> Can you clarify what you meant? /proc & cpu mask is dependent on Linux 
> & x86, and I only tested on that platform.
> The assumptions I used here is based on the x86 cache architecture.

What I was trying to say was that I don't know enough about Linux to be 
confident that your implementation of os::active_core_count() is the 
simplest and most stable way to retrieve that information. I'm sure it 
is good, I am just not the right person to review this piece of the 
code. That's why I think it would be good if someone from the Runtime 
team looked at this.

Thanks,
Bengt


>
> Jungwoo
>

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