<html>
<head>
<meta content="text/html; charset=utf-8" http-equiv="Content-Type">
</head>
<body bgcolor="#FFFFFF" text="#000000">
<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 class="moz-txt-link-freetext" href="http://cr.openjdk.java.net/~jmasa/6858051/webrev_delta_01_02/">http://cr.openjdk.java.net/~jmasa/6858051/webrev_delta_01_02/</a>
<br>
Full:<a class="moz-txt-link-freetext" href="http://cr.openjdk.java.net/~jmasa/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 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 class="moz-txt-link-freetext" href="http://cr.openjdk.java.net/~jmasa/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>
<br>
<br>
- Derek<br>
</body>
</html>