Better default for ParallelGCThreads and ConcGCThreads by using number of physical cores and CPU mask. (original) (raw)

Bengt Rutisson bengt.rutisson at oracle.com
Mon Jan 13 09:31:43 UTC 2014

Hi Jungwoo,

On 1/11/14 2:23 AM, Jungwoo Ha wrote:

I attached the updated webrev. I reflected those comments from others except that making os::activecorecount platform independent. I think it is impossible to do it without using ifdefs on os.cpp. Let me know what else needs to be done.

Thanks for the update.

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.

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?

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.

Bengt

On Thu, Jan 9, 2014 at 5:04 PM, Jungwoo Ha <jwha at google.com_ _<mailto:jwha at google.com>> wrote:

oslinuxx86.cpp This is where the important part of your change is. I don't know enough about Linux internals to validate the changes you have made there. I will leave that out of my review and let others (Runtime team?) with more Linux knowledge review it. Some code comments: Since corecount() returns 0 for all platforms except linuxx86, couldn't the following code be moved to a platform independent place? 758 int os::activecorecount() { 759 if (os::corecount() <= 0) { 760 os::setcorecount(os::activeprocessorcount()); 761 } 762 return os::corecount(); 763 } That way this code would not have to be duplicated in all other platforms: 726 int os::activecorecount() { 727 return os::activeprocessorcount(); 728 } We could just always use os::activecorecount() and rely on the value it returns. Sounds good to me. Good. I am now working on this update. If we pull os::activecorecount() to os.cpp, where should the linuxx86 version be? Jungwoo

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

More information about the hotspot-gc-dev mailing list