<html>
  <head>
    <meta content="text/html; charset=ISO-8859-1"
      http-equiv="Content-Type">
  </head>
  <body bgcolor="#FFFFFF" text="#000000">
    Hi,<br>
    <br>
    Looks good in general. Two comments:<br>
    <br>
    * In compactibleFreeListSpace.cpp the code:<br>
    <meta http-equiv="content-type" content="text/html;
      charset=ISO-8859-1">
    <pre><span class="changed"> 630           MutexLockerEx x(parDictionaryAllocLock(),</span>
<span class="changed"> 631                           Mutex::_no_safepoint_check_flag);</span>
</pre>
    was previously executed for all values of ParallelGCThreads > 0,
    now it is not executed when ParallelGCThreads = 1. Seems legit, just
    want to make sure this is intentional. Are there any side effects to
    creating this lock that is expected to happen for PGCT = 1?<br>
    <br>
    * In g1HotCardCache you remove the variable
    _hot_cache_par_chunk_size and read directly from the flag in
    G1HotCardCache::drain()
    <meta http-equiv="content-type" content="text/html;
      charset=ISO-8859-1">
    . I would prefer to keep the variable and only read from the flag
    during initialization. I think it would be nice if we were moving
    towards not reading directly from command line flags during GC.
    There have been a few cases lately where we have been in need of
    changing settings for the GC at runtime (when making flags
    manageable for instance) and I think we will see more of this in the
    future if we want to see a GC that can behave differently in
    different situations. It is a lot easier to do this if the flags can
    be changed at any time without needing to worry about them being
    read during GC.<br>
    <br>
    Thanks!<br>
    /Jesper<br>
    <br>
    <br>
    <div class="moz-cite-prefix">Marcus Larsson skrev 8/10/14 09:56:<br>
    </div>
    <blockquote cite="mid:5434EE34.2090009@oracle.com" type="cite">Hi,
      <br>
      <br>
      Can I please have reviews for the following patch removing the
      special-case code used when ParallelGCThreads is 0. A recent
      change [1] forbidding this flag to be set to 0 turns this special
      handling into dead code, which should be removed.
      <br>
      <br>
      Functions with two interfaces for single threaded or parallel
      execution used exclusively in these cases have been merged to a
      single parallel version (for example G1RemSet's scrub and
      scrub_par).
      <br>
      <br>
      Webrev:
      <br>
      <a class="moz-txt-link-freetext" href="http://cr.openjdk.java.net/~mlarsson/6979279/webrev.00/">http://cr.openjdk.java.net/~mlarsson/6979279/webrev.00/</a>
      <br>
      <br>
      Bug:
      <br>
      <a class="moz-txt-link-freetext" href="https://bugs.openjdk.java.net/browse/JDK-6979279">https://bugs.openjdk.java.net/browse/JDK-6979279</a>
      <br>
      <br>
      Testing:
      <br>
      - jprt
      <br>
      - Aurora
      <br>
        - Weblogic
      <br>
        - runThese
      <br>
        - Kitchensink
      <br>
        - jtreg
      <br>
        - VM GC nightly
      <br>
      <br>
      Thanks,
      <br>
      Marcus
      <br>
      <br>
      [1]: <a class="moz-txt-link-freetext" href="https://bugs.openjdk.java.net/browse/JDK-8059527">https://bugs.openjdk.java.net/browse/JDK-8059527</a>
      <br>
    </blockquote>
    <br>
  </body>
</html>