<html>
  <head>
    <meta content="text/html; charset=utf-8" http-equiv="Content-Type">
  </head>
  <body bgcolor="#FFFFFF" text="#000000">
    <br>
    <br>
    <div class="moz-cite-prefix">On 07/02/2015 10:58 AM, Stefan Karlsson
      wrote:<br>
    </div>
    <blockquote cite="mid:55957BCD.9020604@oracle.com" type="cite">
      <meta content="text/html; charset=utf-8" http-equiv="Content-Type">
      <div class="moz-cite-prefix">On 2015-07-02 19:43, Jon Masamitsu
        wrote:<br>
      </div>
      <blockquote cite="mid:5595784A.9050903@oracle.com" type="cite">
        <meta content="text/html; charset=utf-8"
          http-equiv="Content-Type">
        <br>
        <br>
        <div class="moz-cite-prefix">On 6/29/2015 2:38 AM, Stefan
          Karlsson wrote:<br>
        </div>
        <blockquote cite="mid:5591122F.2000704@oracle.com" type="cite">
          <meta content="text/html; charset=utf-8"
            http-equiv="Content-Type">
          Hi all,<br>
          <br>
          "8087322: Implement a Semaphore utility class" has now been
          pushed, so I've updated the patch to reflect the changes.<br>
          <br>
          <a moz-do-not-send="true" class="moz-txt-link-freetext"
            href="http://cr.openjdk.java.net/%7Estefank/8087324/webrev.01.delta">http://cr.openjdk.java.net/~stefank/8087324/webrev.01.delta</a><br>
          <a moz-do-not-send="true" class="moz-txt-link-freetext"
            href="http://cr.openjdk.java.net/%7Estefank/8087324/webrev.01">http://cr.openjdk.java.net/~stefank/8087324/webrev.01</a><br>
        </blockquote>
        <a moz-do-not-send="true" class="moz-txt-link-freetext"
href="http://cr.openjdk.java.net/%7Estefank/8087324/webrev.01/src/share/vm/gc/shared/workgroup.hpp.frames.html">http://cr.openjdk.java.net/~stefank/8087324/webrev.01/src/share/vm/gc/shared/workgroup.hpp.frames.html</a><br>
        <br>
        Are these used?<br>
        <br>
        <pre><span class="changed"> 194   void print_worker_started_task(AbstractGangTask* task, uint worker_id);</span>
<span class="changed"> 195   void print_worker_finished_task(AbstractGangTask* task, uint worker_id);
</span></pre>
      </blockquote>
      <br>
      No. I'll remove them.<br>
      <br>
      <blockquote cite="mid:5595784A.9050903@oracle.com" type="cite">
        <pre><span class="changed">

Rest looks good.  Just one question (just for my education).
 </span></pre>
        <a moz-do-not-send="true" class="moz-txt-link-freetext"
href="http://cr.openjdk.java.net/%7Estefank/8087324/webrev.01/src/share/vm/gc/shared/workgroup.cpp.frames.html">http://cr.openjdk.java.net/~stefank/8087324/webrev.01/src/share/vm/gc/shared/workgroup.cpp.frames.html</a><br>
        <br>
        <pre><span class="changed"> 336 void GangWorker::loop() {</span>
<span class="changed"> 337   while (true) {</span>
<span class="changed"> 338     WorkData data = wait_for_task();</span>
<span class="changed"> 339 </span>
<span class="changed"> 340     run_task(data);</span>
<span class="changed"> 341 </span>
<span class="changed"> 342     signal_task_done();</span>
 343   }
 344 }

Does this allow the same thread to execute more than 1
task ("data" here)?  Meaning if 2 threads are  requested but
1 thread is not scheduled to a cpu, will the other thread
do both chunks of work?</pre>
      </blockquote>
      <br>
      Yes, that's correct.<br>
    </blockquote>
    <br>
    That is really cool.  Thanks.<br>
    <br>
    Jon <br>
    <blockquote cite="mid:55957BCD.9020604@oracle.com" type="cite"> <br>
      Thanks for reviewing!<br>
      StefanK<br>
      <br>
      <blockquote cite="mid:5595784A.9050903@oracle.com" type="cite">
        <pre>
Jon


</pre>
        <blockquote cite="mid:5591122F.2000704@oracle.com" type="cite">
          <br>
          - The IMPLEMENTS_SEMAPHORE_CLASS define was removed, since all
          platforms need to provide a Semaphore implementation.<br>
          <br>
          - Removed the need to pass down "max number of workers" to the
          Semaphore constructor.<br>
          <br>
          - Updated semaphore.hpp include path<br>
          <br>
          Thanks,<br>
          StefanK<br>
          <br>
          <br>
          <div class="moz-cite-prefix">On 2015-06-12 16:52, Stefan
            Karlsson wrote:<br>
          </div>
          <blockquote cite="mid:557AF21B.2090102@oracle.com" type="cite">
            <meta http-equiv="content-type" content="text/html;
              charset=utf-8">
            Hi all,<br>
            <br>
            The current implementation to distribute tasks to GC worker
            threads often cause long latencies (multiple milliseconds)
            when the threads are started and stopped. <br>
            <br>
            The main reason is that the worker threads have to fight
            over the Monitor lock when they are woken up from the call
            to Monitor::wait. Another reason is that all worker threads
            call notify_all when they finish a task and there wakes all
            all sleeping worker threads, which will yet again force the
            worker threads to fight over the lock. <br>
            <br>
            I propose that we use semaphores instead, so that the worker
            threads don't have to fight over a lock when they are woken
            up.<br>
            <br>
            <br>
            The patches build upon the following patch which introduces
            a Semaphore utility class. This patch will sent out for
            review on the hotspot-dev, since it affects non-GC parts of
            the code:<br>
             <a moz-do-not-send="true" class="moz-txt-link-freetext"
              href="http://cr.openjdk.java.net/%7Estefank/8087322/webrev.00/">http://cr.openjdk.java.net/~stefank/8087322/webrev.00/</a><br>
             <a moz-do-not-send="true" class="moz-txt-link-freetext"
              href="https://bugs.openjdk.java.net/browse/JDK-8087322">https://bugs.openjdk.java.net/browse/JDK-8087322</a><br>
            <br>
            <br>
            The first patch that I would like to get reviewed is:<br>
             <a moz-do-not-send="true" class="moz-txt-link-freetext"
              href="http://cr.openjdk.java.net/%7Estefank/8087323/webrev.00/">http://cr.openjdk.java.net/~stefank/8087323/webrev.00/</a><br>
             <a moz-do-not-send="true" class="moz-txt-link-freetext"
              href="https://bugs.openjdk.java.net/browse/JDK-8087323">https://bugs.openjdk.java.net/browse/JDK-8087323</a>
            - Unify and split the work gang classes <br>
            <br>
            It prepares for JDK-8087324, by separating the generic
            WorkGang implementation from the more elaborate
            YieldingFlexibleWorkGang (CMS) implementation. By having
            this part as a separate patch, I hope it will be easier to
            review JDK-8087324. The patch changes the work gang
            inheritance from:<br>
            <br>
            AbstractWorkGang<br>
             WorkGang<br>
              FlexibleWorkGang<br>
               YieldingFlexibleWorkGang<br>
            <br>
            to:<br>
            <br>
            AbstractWorkGang<br>
             WorkGang<br>
             YieldingFlexibleWorkGang<br>
            <br>
            Parts of the FlexibleWorkGang and WorkGang code that is
            going to be used by both concrete work gang classes, has
            been moved into AbstractWorkGang. I've duplicated some code
            in WorkGang and YieldingFlexibleWorkGang, but that code will
            be removed from WorkGang in the following patch.<br>
            <br>
            <br>
            The second patch I'd like to get reviewed is:<br>
             <a moz-do-not-send="true" class="moz-txt-link-freetext"
              href="http://cr.openjdk.java.net/%7Estefank/8087324/webrev.00/">http://cr.openjdk.java.net/~stefank/8087324/webrev.00/</a><br>
             <a moz-do-not-send="true" class="moz-txt-link-freetext"
              href="https://bugs.openjdk.java.net/browse/JDK-8087324">https://bugs.openjdk.java.net/browse/JDK-8087324</a>
            - Use semaphores when starting and stopping GC task threads
            <br>
            <br>
            It first simplifies the way we distribute the tasks to the
            GC worker threads. For example, the coordinator thread
            dispatches a task to a specific number of workers, and then
            waits for all work to be completed. There's no risk that
            multiple tasks will be scheduled simultaneously, so there's
            no need for the sequences number that is used in the current
            implementation.<br>
            <br>
            The patch contains two task dispatch / thread
            synchronization implementations:<br>
            <br>
            The first implementation uses Monitors, similar to what we
            did before the patch, but with a slightly lower overhead
            since the code calls notify_all less often. It still suffers
            from the "thundering heard" problem. When the coordinator
            thread signals that the worker threads should start, they
            all wake up from Monitor::wait and they all try to lock the
            Monitor.<br>
            <br>
            The second, and the more interesting, implementation uses
            semaphores. When the worker threads wake up from the
            semaphore wait, they don't have to serialize the execution
            by taking a lock. This greatly decreases the time it takes
            to start and stop the worker threads.<br>
            <br>
            The semaphore implementation is used on all platforms where
            the Semaphore class has been implemented in JDK-8087322. So,
            on some OS:es the code will revert to the Monitor-based
            solution until a Semaphore class has been implemented for
            that OS. So, porters might want to consider implementing the
            Sempahore class.<br>
            <br>
            There's also a diagnostic vm option
            (-XX:+/-UseSemaphoreGCThreadsSynchronization) to turn off
            the Semaphore-based implementation, which can be used to
            debug this new code. It's mainly targeted towards support
            and sustaining engineering.<br>
            <br>
            <br>
            The patches have been performance tested on Linux, Solaris,
            OSX, and Windows.<br>
            <br>
            The effects of the patch can be seen by running benchmarks
            with small young gen sizes, which triggers frequent and
            short GCs.<br>
            <br>
            For example, here are runs from the SPECjvm2008
            xml.transform benchmark with:<br>
            -Xmx1g -Xms1g -Xmn64m -XX:+PrintGC -XX:+UseG1GC -jar
            SPECjvm2008.jar -ikv xml.transform -it 30 -wt 30<br>
            <br>
            I got the following GC times:<br>
            <br>
            <tt>            Average    Median    99.9 percentile   Max</tt><tt><br>
            </tt><tt>Baseline:   </tt>
            <meta http-equiv="content-type" content="text/html;
              charset=utf-8">
            <title></title>
            <meta name="generator" content="LibreOffice 4.4.0.3 (Linux)">
            <style type="text/css">
                body,div,table,thead,tbody,tfoot,tr,th,td,p { font-family:"Liberation Sans"; font-size:x-small }
        </style><tt> </tt>
            <meta http-equiv="content-type" content="text/html;
              charset=utf-8">
            <title></title>
            <meta name="generator" content="LibreOffice 4.4.0.3 (Linux)">
            <style type="text/css">
                body,div,table,thead,tbody,tfoot,tr,th,td,p { font-family:"Liberation Sans"; font-size:x-small } </style><tt>8.76</tt><tt>
              ms    8.44 ms   25.9 ms           </tt><tt>3</tt><tt>4.7
              ms</tt><tt><br>
            </tt><tt>Monitor:</tt><tt>    6.1</tt><tt>7 </tt><tt>ms   
            </tt><tt>5.88 ms   26.0 ms           </tt><tt>49.1 ms</tt><tt><br>
            </tt><tt> </tt>
            <meta http-equiv="content-type" content="text/html;
              charset=utf-8">
            <title></title>
            <meta name="generator" content="LibreOffice 4.4.0.3 (Linux)">
            <style type="text/css">
                body,div,table,thead,tbody,tfoot,tr,th,td,p { font-family:"Liberation Sans"; font-size:x-small }
        </style><tt>Semaphore:  </tt><tt>3.</tt><tt>43 ms    </tt><tt>3.26 </tt><tt>ms





                13.4 ms           33.4 ms</tt><tt><br>
            </tt><br>
            If I run an empty GC task 10 times per GC, by running the
            following code:<br>
            <a moz-do-not-send="true" class="moz-txt-link-freetext"
              href="http://cr.openjdk.java.net/%7Estefank/8087324/timedTask/">http://cr.openjdk.java.net/~stefank/8087324/timedTask/</a><br>
            <br>
            I get the following numbers to complete the empty GC tasks:<br>
            <br>
            <tt>            Average    Median    99.9 percentile   Max</tt><tt><br>
            </tt><tt>Baseline:   </tt><tt>1.43 ms    0.92 ms   3.43
              ms           9.30</tt><tt> ms</tt><tt><br>
              Monitor</tt><tt>:</tt><tt>    0.75</tt><tt> </tt><tt>ms   

            </tt><tt>0.72 ms   1.74 ms           2.78</tt><tt> ms</tt><tt><br>
            </tt><tt> </tt><tt>Semaphore:  </tt><tt>0.</tt><tt>07
              ms    </tt><tt>0.07 </tt><tt>ms   0.17 ms           0.26
              ms</tt><tt><br>
            </tt><br>
            <br>
            <br>
            The code has been tested with JPRT and our nightly testing
            suites. <br>
            <br>
            I've created a unit test to run a small test with both the
            semaphore implementation and the monitor implementation:<br>
            <a moz-do-not-send="true" class="moz-txt-link-freetext"
              href="http://cr.openjdk.java.net/%7Estefank/8087324/workgangTest/">http://cr.openjdk.java.net/~stefank/8087324/workgangTest/</a><br>
            <br>
            But since we currently don't have code to shutdown worker
            threads after they have been started, I don't want to push
            this test (or clean it up) until we have that in place. I
            created this bug for that:<br>
            <a moz-do-not-send="true" class="moz-txt-link-freetext"
              href="https://bugs.openjdk.java.net/browse/JDK-8087340">https://bugs.openjdk.java.net/browse/JDK-8087340</a><br>
            <br>
            Thanks,<br>
            StefanK<br>
            <br>
            <br>
            <br>
          </blockquote>
          <br>
        </blockquote>
        <br>
      </blockquote>
      <br>
    </blockquote>
    <br>
  </body>
</html>