Looks good to me too. I'd suggest a one-line doc for the new getNextObject() method stating that<br>the argument to the method should be the address of a valid object. I don't think an assertion is<br>necessary in the method to check for that because, I am guessing, obj->size() will already have<br>
some form of sanity check/assertion that would assert if it's not the start address of a valid object.<br><br>-- ramki<br><br><div class="gmail_quote">On Fri, Mar 1, 2013 at 5:19 PM, Tao Mao <span dir="ltr"><<a href="mailto:tao.mao@oracle.com" target="_blank">tao.mao@oracle.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
  
    
  
  <div bgcolor="#FFFFFF" text="#000000">
    integrated the suggestion. A new webrev is updated.<br>
    <a href="http://cr.openjdk.java.net/%7Etamao/8008079/webrev.01/" target="_blank">http://cr.openjdk.java.net/~tamao/8008079/webrev.01/</a><br>
    <br>
    again, passed gc-test-suite for fastdebug build with stressing
    marking cycles<br>
    script:<br>
    ./run.sh /home/tamao/jdk1.8.0 --bits 64 --args -tamao -XX:+UseG1GC
    -XX:InitiatingHeapOccupancyPercent=5 -XX:+UnlockDiagnosticVMOptions
    -XX:+VerifyDuringGC<div><div class="h5"><br>
    <br>
    On 2/25/13 3:06 PM, John Cuthbertson wrote:
    <blockquote type="cite">
      
      Hi Tao,<br>
      <br>
      Looks fine except for the following nits....<br>
      <br>
      concurrentMark.hpp:100<br>
      Can you remove the line: <br>
      <br>
      // XXX Fix these so that offsets are size_t's...<br>
      <br>
      concurrentMark.hpp:111<br>
      It might be safer to return:<br>
      <br>
      return offsetToHeapWord(heapWordToOffset(addr + obj->size()));<br>
      <br>
      or assert something like:<br>
      <br>
      HeapWord* res =  <span>addr + obj->size();<br>
        assert(</span><span>offsetToHeapWord(heapWordToOffset(res))

        == res, "sanity");<br>
        return res;<br>
      </span><span><br>
        JohnC<br>
      </span>
      <pre><span></span></pre>
      <div>On 2/21/2013 6:16 PM, Tao Mao wrote:<br>
      </div>
      <blockquote type="cite">
        
        8008079 G1: Add nextObject routine to CMBitMapRO and replace
        nextWord<br>
        <a href="https://jbs.oracle.com/bugs/browse/JDK-8008079" target="_blank">https://jbs.oracle.com/bugs/browse/JDK-8008079</a><br>
        <br>
        webrev: <br>
        <a href="http://cr.openjdk.java.net/%7Etamao/8008079/webrev.00/" target="_blank">http://cr.openjdk.java.net/~tamao/8008079/webrev.00/</a><br>
        <br>
        changeset:<br>
        
        When concurrent marking scans an object, the task local finger
        is updated to point to the start of the object. If the marking
        task is asked to abort, the local finger is updated to the next
        word where another could possibly start. When the marking task
        is restarted, we restart scanning the marking bitmap from the
        updated local finger. <br>
        <br>
        This is a safe implementation but has not been fully exploited
        for efficiency because the contents of the marking bitmap should
        be all 0's until the offset associated with the actual start of
        the next object. When the marking task is restarted, we will
        scan these 0's looking for the first set bit. <br>
        <br>
        We can avoid this redundant scanning by updating the finger to
        and re-starting the scan at the actual offset where the next
        object starts.<br>
        <br>
        testing:<br>
        passed gc-test-suite with stressing marking cycles<br>
        script:<br>
        ./run.sh /home/tamao/jdk1.8.0 --bits 64 --args -tamao
        -XX:+UseG1GC -XX:InitiatingHeapOccupancyPercent=10
        -XX:+UnlockDiagnosticVMOptions -XX:+VerifyDuringGC<br>
      </blockquote>
      <br>
    </blockquote>
  </div></div></div>

</blockquote></div><br>