<html>
<head>
<meta content="text/html; charset=UTF-8" http-equiv="Content-Type">
</head>
<body bgcolor="#FFFFFF" text="#000000">
<div class="moz-cite-prefix"><br>
Hi Jungwoo,<br>
<br>
On 1/11/14 2:23 AM, Jungwoo Ha wrote:<br>
</div>
<blockquote
cite="mid:CA+n_jhhZK7X6f2EAv3kPOik_L657C=azwtBz1dzkaj19aKPLaA@mail.gmail.com"
type="cite">
<div dir="ltr">I attached the updated webrev.
<div>I reflected those comments from others except that making
os::active_core_count platform independent.</div>
<div>I think it is impossible to do it without using ifdefs on
os.cpp.</div>
<div>Let me know what else needs to be done.</div>
</div>
</blockquote>
<br>
Thanks for the update.<br>
<br>
In CMSCollector there is still this code to change the value for
ConcGCThreads based on AdjustGCThreadsToCores.<br>
<br>
639 if (AdjustGCThreadsToCores) {<br>
640 FLAG_SET_DEFAULT(ConcGCThreads, ParallelGCThreads / 2);<br>
641 } else {<br>
642 FLAG_SET_DEFAULT(ConcGCThreads, (3 + ParallelGCThreads)
/ 4);<br>
643 }<br>
<br>
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.<br>
<br>
Also, I don't fully understand the name AdjustGCThreadsToCores. In
<meta http-equiv="content-type" content="text/html; charset=UTF-8">
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?<br>
<br>
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.<br>
<br>
Bengt<br>
<br>
<br>
<br>
<blockquote
cite="mid:CA+n_jhhZK7X6f2EAv3kPOik_L657C=azwtBz1dzkaj19aKPLaA@mail.gmail.com"
type="cite">
<div dir="ltr">
<div><br>
</div>
</div>
<div class="gmail_extra"><br>
<br>
<div class="gmail_quote">On Thu, Jan 9, 2014 at 5:04 PM, Jungwoo
Ha <span dir="ltr"><<a moz-do-not-send="true"
href="mailto:jwha@google.com" target="_blank">jwha@google.com</a>></span>
wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0
.8ex;border-left:1px #ccc solid;padding-left:1ex">
<div dir="ltr">
<div class="gmail_extra">
<div class="gmail_quote">
<div class="im">
<blockquote class="gmail_quote" style="margin:0 0 0
.8ex;border-left:1px #ccc solid;padding-left:1ex">
<div text="#000000" bgcolor="#FFFFFF">
<div>
<blockquote type="cite">
<div dir="ltr">
<div class="gmail_extra">
<div class="gmail_quote"><br>
<blockquote class="gmail_quote"
style="margin:0 0 0
.8ex;border-left:1px #ccc
solid;padding-left:1ex">
<div bgcolor="#FFFFFF"
text="#000000">
<div> <br>
os_linux_x86.cpp<br>
<br>
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.<br>
<br>
<br>
Some code comments:<br>
<br>
Since core_count() returns 0 for
all platforms except linux_x86,
couldn't the following code be
moved to a platform independent
place?<br>
<br>
758 int os::active_core_count()
{<br>
759 if (os::core_count()
<= 0) {<br>
760
os::set_core_count(os::active_processor_count());<br>
761 }<br>
762 return os::core_count();<br>
763 }<br>
<br>
That way this code would not
have to be duplicated in all
other platforms:<br>
<br>
726 int os::active_core_count()
{<br>
727 return
os::active_processor_count();<br>
728 }<br>
<br>
We could just always use
os::active_core_count() and rely
on the value it returns.<br>
</div>
</div>
</blockquote>
<div><br>
</div>
<div>Sounds good to me.</div>
</div>
</div>
</div>
</blockquote>
<br>
</div>
Good.
<div><br>
</div>
</div>
</blockquote>
<div><br>
</div>
</div>
<div>I am now working on this update. If we pull
os::active_core_count() to os.cpp, where should the
linux_x86 version be?</div>
<span class="HOEnZb"><font color="#888888">
<div>
<br>
</div>
<div><br>
</div>
<div>Jungwoo</div>
</font></span></div>
</div>
</div>
</blockquote>
</div>
<br>
</div>
</blockquote>
<br>
</body>
</html>