<div dir="ltr"><div dir="ltr">Hi,<div><br></div><div>Thanks for the review!</div><div>I moved those 3 functions and created a new WorkerPolicy class as suggested.</div><div>New webrev: <a href="https://cr.openjdk.java.net/~manc/8213224/webrev.01/">https://cr.openjdk.java.net/~manc/8213224/webrev.01/</a></div><div><br></div><div>There's a Sparc-specific heuristic for VM_Version::calc_parallel_worker_threads().</div>I added a function Abstract_VM_Version::parallel_worker_threads_denominator() in order to keep this behavior and minimize relevant code in VM_Version class.<div>I also changed "unsigned int" to "uint", for the functions from VM_Version.<br></div><div><br></div><div>Now the change in workerManager.hpp is only trivial code movement and improving #include statements. </div><div><div><br></div><div>Thanks,<br><div><div dir="ltr" class="gmail_signature"><div dir="ltr">Man</div></div></div><br></div></div></div></div><br><div class="gmail_quote"><div dir="ltr">On Mon, Nov 5, 2018 at 12:20 AM Per Liden <<a href="mailto:per.liden@oracle.com">per.liden@oracle.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Hi,<br>
<br>
On 11/1/18 6:49 PM, Man Cao wrote:<br>
> Hi all,<br>
> <br>
> Could anyone review this cleanup change?<br>
> In addition to straight code movement, it removes unnecessary dependency <br>
> on adaptiveSizePolicy.hpp in G1 code, and removed unnecessary include <br>
> statements in adaptiveSizePolicy.hpp/cpp.<br>
> <br>
> Webrev: <a href="https://cr.openjdk.java.net/~manc/8213224/webrev.00/" rel="noreferrer" target="_blank">https://cr.openjdk.java.net/~manc/8213224/webrev.00/</a><br>
> RFE: <a href="https://bugs.openjdk.java.net/browse/JDK-8213224" rel="noreferrer" target="_blank">https://bugs.openjdk.java.net/browse/JDK-8213224</a><br>
<br>
Removing these function from AdaptiveSizePolicy might be a good idea, <br>
but I'm not sure they fit very well into the WorkerManager class, which <br>
is a backend for AbstractWorkGang and GCTaskManager. These functions <br>
might fit better into a new WorkerPolicy class. Such a class could also <br>
be the new home for these function (which currently sit is an very odd <br>
place):<br>
<br>
Abstract_VM_Version::nof_parallel_worker_threads();<br>
Abstract_VM_Version::parallel_worker_threads();<br>
Abstract_VM_Version::calc_parallel_worker_threads();<br>
<br>
cheers,<br>
Per<br>
<br>
> Tested on submit repo with the help from JC (CCed).<br>
> <br>
> Thanks,<br>
> Man<br>
</blockquote></div>