<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=utf-8">
</head>
<body text="#000000" bgcolor="#FFFFFF">
Hi all,<br>
<br>
As webrev.0 is conflicting with webrev.0 of "8203319: JDK-8201487
disabled too much queue balancing"(out for review, but not yet
pushed), I'm posting webrev.1.<br>
<br>
<a class="moz-txt-link-freetext" href="http://cr.openjdk.java.net/~sangheki/8043575/webrev.1">http://cr.openjdk.java.net/~sangheki/8043575/webrev.1</a><br>
<a class="moz-txt-link-freetext" href="http://cr.openjdk.java.net/~sangheki/8043575/webrev.1_to_0">http://cr.openjdk.java.net/~sangheki/8043575/webrev.1_to_0</a><br>
<br>
Thanks,<br>
Sangheon<br>
<br>
<br>
<div class="moz-cite-prefix">On 5/30/18 9:43 PM,
<a class="moz-txt-link-abbreviated" href="mailto:sangheon.kim@oracle.com">sangheon.kim@oracle.com</a> wrote:<br>
</div>
<blockquote type="cite"
cite="mid:8af7594e-d5f4-ca0d-de73-6547ea131f3e@oracle.com">
<meta http-equiv="content-type" content="text/html; charset=utf-8">
Hi all,<br>
<br>
Could I have some reviews for this patch?<br>
<br>
This patch is suggesting ergonomically choosing worker thread
count from given reference count.<br>
We have ParallelRefProcEnabled command-line option which enables
to use ALL workers during reference processing however this option
has a drawback when there's limited number of references. i.e.
spends more time on thread start-up/tear-down than actual
processing time if there are less references. And also we use all
threads or single thread during reference processing which seems
less flexible on thread counts. This patch calculates the worker
counts from dividing reference count by ReferencesPerThread(newly
added experimental option). <br>
My suggestion for the default value of ReferencePerThread is 1000
as it showed good results from some benchmarks. <br>
<br>
Notes:<br>
1. CMS ParNew is excluded from this patch because:<br>
a) There is a separate CR for CMS (JDK-6938732).<br>
b) It is tricky to manage switching single <-> MT
processing inside of ReferenceProcessor class for ParNew. Tony
explained quite well about the reason here ( <a
class="moz-txt-link-freetext"
href="https://bugs.openjdk.java.net/browse/JDK-6938732?focusedCommentId=13932462&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-13932462"
moz-do-not-send="true">https://bugs.openjdk.java.net/browse/JDK-6938732?focusedCommentId=13932462&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-13932462</a>
). <br>
c) CMS will be obsoleted in the future so not motivated to fix
within this patch.<br>
2. JDK-8203951 is the CR for removing temporarily added
flag(ReferenceProcessor::_has_adjustable_queue from webrev.0) to
manage ParNew. So the flag should be removed when CMS is
obsoleted.<br>
3. Current logic of dividing by ReferencesPerThread would be
replaced with better implementation. e.g. time measuring
implementation etc. But I think current approach is simply and
good enough.<br>
4. This patch is based on JDK-8204094 and JDK-8204095, both are
not yet pushed so far.<br>
<br>
CR: <a class="moz-txt-link-freetext"
href="https://bugs.openjdk.java.net/browse/JDK-8043575"
moz-do-not-send="true">https://bugs.openjdk.java.net/browse/JDK-8043575</a><br>
Webrev: <a class="moz-txt-link-freetext"
href="http://cr.openjdk.java.net/%7Esangheki/8043575/webrev.0/"
moz-do-not-send="true">http://cr.openjdk.java.net/~sangheki/8043575/webrev.0/</a><br>
Testing: hs-tier 1~5 with/without ParallelRefProcEnabled<br>
<br>
Thanks,<br>
Sangheon<br>
</blockquote>
<br>
</body>
</html>