<html>
<head>
<meta content="text/html; charset=utf-8" http-equiv="Content-Type">
</head>
<body bgcolor="#FFFFFF" text="#000000">
No changes since webrev.04 but a webrev from an updated repository.<br>
<br>
<a class="moz-txt-link-freetext" href="http://cr.openjdk.java.net/~jmasa/6858051/webrev.05/">http://cr.openjdk.java.net/~jmasa/6858051/webrev.05/</a><br>
<br>
<div class="moz-cite-prefix">On 5/6/2016 12:15 PM, Jon Masamitsu
wrote:<br>
</div>
<blockquote
cite="mid:a05daecf-0c07-b7d4-dd5a-eea38b855069@oracle.com"
type="cite">
<meta content="text/html; charset=utf-8" http-equiv="Content-Type">
Testing with trace level logging found another bug.<br>
The logging tried to print information for GCTaskThreads<br>
that have not been created yet.<br>
<br>
Delta<br>
<a moz-do-not-send="true" class="moz-txt-link-freetext"
href="http://cr.openjdk.java.net/%7Ejmasa/6858051/webrev_delta_03_04/">http://cr.openjdk.java.net/~jmasa/6858051/webrev_delta_03_04/</a><br>
<br>
Complete<br>
<a moz-do-not-send="true" class="moz-txt-link-freetext"
href="http://cr.openjdk.java.net/%7Ejmasa/6858051/webrev.04/">http://cr.openjdk.java.net/~jmasa/6858051/webrev.04/</a><br>
<br>
Thanks.<br>
<br>
Jon<br>
<br>
<div class="moz-cite-prefix">On 5/5/2016 11:17 AM, Jon Masamitsu
wrote:<br>
</div>
<blockquote cite="mid:572B8E30.406@oracle.com" type="cite">
<meta content="text/html; charset=utf-8"
http-equiv="Content-Type">
<font face="Times New Roman, Times, serif">Derek,<br>
<br>
New webrev and webrev_delta. One comment in-line.<br>
<br>
<a moz-do-not-send="true" class="moz-txt-link-freetext"
href="http://cr.openjdk.java.net/%7Ejmasa/6858051/webrev.03/">http://cr.openjdk.java.net/~jmasa/6858051/webrev.03/</a><br>
<a moz-do-not-send="true" class="moz-txt-link-freetext"
href="http://cr.openjdk.java.net/%7Ejmasa/6858051/webrev_delta_02_03/">http://cr.openjdk.java.net/~jmasa/6858051/webrev_delta_02_03/</a><br>
<br>
Thanks.<br>
</font><br>
<div class="moz-cite-prefix">On 05/04/2016 02:45 PM, Derek White
wrote:<br>
</div>
<blockquote cite="mid:572A6D78.1090804@oracle.com" type="cite">
<meta content="text/html; charset=utf-8"
http-equiv="Content-Type">
<div class="moz-cite-prefix">Hi Jon,<br>
<br>
On 4/28/16 6:40 PM, Jon Masamitsu wrote:<br>
</div>
<blockquote cite="mid:57229179.9080606@oracle.com" type="cite">Changes:
<br>
<br>
In order to reduce the dependency of WorkGang on
AdaptiveSizePolicy, <br>
removed AdaptiveSizePolicy::initial_number_of_workers() and
duplicated the code <br>
in GCTaskManager::initialize() and
AbstractWorkGang::initialize_workers(). <br>
<br>
Moved AdaptiveSizePolicy::add_workers() to
WorkerManager::add_workers() in <br>
file shared/workerManager.hpp <br>
<br>
Removed guard DisableStartThread from add_workers() and
always start the <br>
workers. <br>
<br>
Removed the bool return value from
WorkerManager::add_workers (and let that rippled down <br>
to the callers). <br>
<br>
Changed WorkerManager::add_workers() to return the number of
created workers <br>
(removed inout on parameters) and let the callers of
WorkerManager::add_workers() <br>
adjust their _active_workers to be consistent with the
number of created workers. <br>
<br>
Changed number of workers created at initialization from 2
to 1. <br>
<br>
Please let me know if I forgot anything. <br>
<br>
Delta: <a moz-do-not-send="true"
class="moz-txt-link-freetext"
href="http://cr.openjdk.java.net/%7Ejmasa/6858051/webrev_delta_01_02/">http://cr.openjdk.java.net/~jmasa/6858051/webrev_delta_01_02/</a>
<br>
Full:<a moz-do-not-send="true" class="moz-txt-link-freetext"
href="http://cr.openjdk.java.net/%7Ejmasa/6858051/webrev.02/">http://cr.openjdk.java.net/~jmasa/6858051/webrev.02/</a>
<br>
<br>
Thanks. <br>
<br>
Jon <br>
<br>
<br>
<br>
On 04/26/2016 10:06 PM, Jon Masamitsu wrote: <br>
<blockquote type="cite">6858051: Create GC worker threads
dynamically <br>
<a moz-do-not-send="true" class="moz-txt-link-freetext"
href="https://bugs.openjdk.java.net/browse/JDK-6858051">https://bugs.openjdk.java.net/browse/JDK-6858051</a>
<br>
<br>
This change creates a minimal number of GC workers at JVM
initialization <br>
and then adds additional workers as they are needed. This
has been made part of the <br>
UseDynamicNumberOfGCThreads feature. When a parallel task
is about to <br>
be executed, the number of workers needed to execute the
task is calculated (call <br>
that number N). If that number of workers already exist,
then no additional workers <br>
are created. If fewer than N exists, then additional
workers are created. Workers <br>
are not destroyed if not needed for the task. <br>
<br>
The UseParallelGC way of executing parallel tasks is
different from the other <br>
collectors so it has a somewhat separate implementation. <br>
<br>
<a moz-do-not-send="true" class="moz-txt-link-freetext"
href="http://cr.openjdk.java.net/%7Ejmasa/6858051/webrev.00/">http://cr.openjdk.java.net/~jmasa/6858051/webrev.00/</a>
<br>
<br>
Testing: gc_test_suite with and without
UseDynamicNumberOfGCThreads. <br>
Performance testing with the prototype did not show any
regression with <br>
UseDynamicNumberOfGCThreads turned on. RBT testing for
hotspot_gc <br>
is in progress. <br>
<br>
Thanks. <br>
<br>
Jon <br>
</blockquote>
</blockquote>
<br>
Great change!<br>
I have a few comments, mostly minor. The last one <i>may</i>
be more serious.<br>
<br>
------------------------------<br>
gc/shared/workgroup.cpp<br>
<br>
- Line 38, update comment about
AbstractWorkGang::initialize_workers() returning a boolean.<br>
<br>
------------------------------<br>
gc/shared/workerManager.hpp<br>
<br>
- Line 33: "dynamic_workers" -> "active_workers"<br>
Lines 38, 39, similarly<br>
<br>
------------------------------<br>
gc/shared/adaptiveSizePolicy.hpp<br>
<br>
Line 34, 42, 43:<br>
- Are these changes still needed?<br>
<br>
Line 355: Does anyone call initial_number_of_workers()?<br>
<br>
------------------------------<br>
gc/parallel/gcTaskThread.hpp<br>
<br>
Line 69: I think start() is no longer called?<br>
- WorkerManager::add_workers() calls os::start_thread()
directly. <br>
<br>
------------------------------<br>
gc/parallel/gcTaskThread.cpp<br>
<br>
Line 57: I think start() is no longer called?<br>
- WorkerManager::add_workers() calls os::start_thread()
directly.<br>
<br>
------------------------------<br>
gc/parallel/gcTaskManager.hpp<br>
Line 1: Copyright.<br>
<br>
------------------------------<br>
gc/parallel/gcTaskManager.cpp<br>
<br>
Line 430: The threads get started here now, not at the end of<br>
initialize(). Is there any danger due to threads starting
before:<br>
443 reset_busy_workers();<br>
444 set_unblocked();<br>
445 for (uint w = 0; w < workers(); w += 1) {<br>
446 set_resource_flag(w, false);<br>
447 }<br>
448 reset_delivered_tasks();<br>
449 reset_completed_tasks();<br>
450 reset_barriers();<br>
451 reset_emptied_queue();<br>
</blockquote>
<br>
The set_resources_flag() call at least should precede the
starting<br>
of the threads. Probably also the set_unblocked(). I don't
think<br>
anything would crash. No threads would try to do work until
after tasks are<br>
added to the work queue, but there would likely have been some<br>
unexpected thread activity at startup that would have been
confusing.<br>
In general that block of code should be executed before starting<br>
threads.<br>
<br>
Jon<br>
<blockquote cite="mid:572A6D78.1090804@oracle.com" type="cite">
<br>
<br>
- Derek<br>
</blockquote>
<br>
</blockquote>
<br>
</blockquote>
<br>
</body>
</html>