<html>
  <head>
    <meta content="text/html; charset=UTF-8" http-equiv="Content-Type">
  </head>
  <body text="#000000" bgcolor="#FFFFFF">
    Hello.<br>
    <br>
    Here is updated webrev<br>
    <br>
    <a class="moz-txt-link-freetext" href="http://cr.openjdk.java.net/~vkempik/8016302/webrev.03/">http://cr.openjdk.java.net/~vkempik/8016302/webrev.03/</a><br>
    <br>
    <br>
    <br>
    Thanks, Vladimir.<br>
    <div class="moz-cite-prefix">On 03.10.2013 22:41, Jon Masamitsu
      wrote:<br>
    </div>
    <blockquote cite="mid:524DBA6D.5090307@oracle.com" type="cite">
      <meta content="text/html; charset=UTF-8" http-equiv="Content-Type">
      <br>
      <div class="moz-cite-prefix">On 10/3/13 8:37 AM, Vladimir Kempik
        wrote:<br>
      </div>
      <blockquote cite="mid:524D8F38.2060304@oracle.com" type="cite">
        <meta content="text/html; charset=UTF-8"
          http-equiv="Content-Type">
        Hello<br>
        <br>
        <div class="moz-cite-prefix">On 03.10.2013 18:57, Jon Masamitsu
          wrote:<br>
        </div>
        <blockquote cite="mid:524D85F5.6030507@oracle.com" type="cite">
          <meta content="text/html; charset=UTF-8"
            http-equiv="Content-Type">
          <a moz-do-not-send="true" class="moz-txt-link-freetext"
href="http://cr.openjdk.java.net/%7Evkempik/8016302/webrev.02/src/share/vm/gc_implementation/g1/concurrentG1Refine.cpp.udiff.html">http://cr.openjdk.java.net/~vkempik/8016302/webrev.02/src/share/vm/gc_implementation/g1/concurrentG1Refine.cpp.udiff.html</a><br>
          <br>
          <meta http-equiv="content-type" content="text/html;
            charset=UTF-8">
          <pre>   for (int i = _n_threads - 1; i >= 0; i--) {

</pre>
          Did you consider using "uint i" instead of "int i"?  Maybe<br>
          adding an assert<br>
          <br>
          assert(_n_threads > 0, ...)<br>
          <br>
          In order to avoid the cast<br>
          <br>
          <meta http-equiv="content-type" content="text/html;
            charset=UTF-8">
          <pre><span class="new">+    ConcurrentG1RefineThread* t = new ConcurrentG1RefineThread(this, next, worker_id_offset, (uint)i);

</span><span class="new"></span></pre>
        </blockquote>
        But then   i >= 0 will always be true. The loop will look
        much uglier if I transform it to check against MAX_UINT<br>
      </blockquote>
      <br>
      OK.<br>
      <br>
      <blockquote cite="mid:524D8F38.2060304@oracle.com" type="cite"> <br>
        <blockquote cite="mid:524D85F5.6030507@oracle.com" type="cite">
          <a moz-do-not-send="true" class="moz-txt-link-freetext"
href="http://cr.openjdk.java.net/%7Evkempik/8016302/webrev.02/src/share/vm/gc_implementation/g1/concurrentMark.cpp.udiff.html">http://cr.openjdk.java.net/~vkempik/8016302/webrev.02/src/share/vm/gc_implementation/g1/concurrentMark.cpp.udiff.html</a><br>
          <br>
          I used to seeing all parameters on the same line or each
          parameter<br>
          on its own line.  Instead of<br>
          <br>
          <meta http-equiv="content-type" content="text/html;
            charset=UTF-8">
          <pre><span class="new">+    G1NoteEndOfConcMarkClosure g1_note_end(_g1h, &local_cleanup_list,</span>
                                            &old_proxy_set,
                                            &humongous_proxy_set,
                                            &hrrs_cleanup_task);


</pre>
          this style.<br>
          <br>
          <pre><span class="new">+    G1NoteEndOfConcMarkClosure g1_note_end(_g1h, 
                                            &local_cleanup_list,</span>
                                            &old_proxy_set,
                                            &humongous_proxy_set,
                                            &hrrs_cleanup_task);

</pre>
          I know, not really your code originally but since you touched
          it, I thought I<br>
          would mention it.<br>
          <br>
          <a moz-do-not-send="true" class="moz-txt-link-freetext"
href="http://cr.openjdk.java.net/%7Evkempik/8016302/webrev.02/src/share/vm/gc_implementation/g1/dirtyCardQueue.cpp.udiff.html">http://cr.openjdk.java.net/~vkempik/8016302/webrev.02/src/share/vm/gc_implementation/g1/dirtyCardQueue.cpp.udiff.html</a><br>
          <br>
        </blockquote>
        <br>
        I will update webrev to include these changes.<br>
        <br>
      </blockquote>
      Thanks.<br>
      <br>
      <blockquote cite="mid:524D8F38.2060304@oracle.com" type="cite"> <br>
        <blockquote cite="mid:524D85F5.6030507@oracle.com" type="cite">
          Is this cast necessary?<br>
          <br>
          <meta http-equiv="content-type" content="text/html;
            charset=UTF-8">
          <pre><span class="new">+                                  (uint) worker_i);

</span><span class="new"></span></pre>
        </blockquote>
        <br>
        worker_i is defined as size_t in that function, it's almost
        same, but not always (on every platform we have), and that code
        is compiled with  -WError, so I've decided to make it clear for
        compiler and not produce any warnings.<br>
      </blockquote>
      <br>
      Ok.<br>
      <br>
      Jon<br>
      <blockquote cite="mid:524D8F38.2060304@oracle.com" type="cite"> <br>
        <blockquote cite="mid:524D85F5.6030507@oracle.com" type="cite">
          Rest looks good.<br>
          <br>
          Jon<br>
          <pre><span class="new"></span></pre>
          <br>
          <br>
          <pre><span class="new"></span></pre>
          <br>
          <div class="moz-cite-prefix">On 10/3/13 3:32 AM, Vladimir
            Kempik wrote:<br>
          </div>
          <blockquote cite="mid:524D47CA.4040708@oracle.com" type="cite">Hello



            <br>
            <br>
            Can I have two reviewers to looks at it please ? <br>
            <br>
            Thanks, Vladimir. <br>
            On 26.09.2013 11:41, Per Liden wrote: <br>
            <blockquote type="cite">Looks good! <br>
              <br>
              cheers, <br>
              /Per <br>
              <br>
              On 2013-09-25 12:47, Vladimir Kempik wrote: <br>
              <blockquote type="cite">Hello <br>
                <br>
                Thanks for comments, updated webrev - <a
                  moz-do-not-send="true" class="moz-txt-link-freetext"
                  href="http://cr.openjdk.java.net/%7Evkempik/8016302/webrev.02/">http://cr.openjdk.java.net/~vkempik/8016302/webrev.02/</a>
                <br>
                <br>
                <br>
                On 24.09.2013 17:22, Thomas Schatzl wrote: <br>
                <blockquote type="cite">Hi, <br>
                  <br>
                  On Thu, 2013-09-19 at 16:22 +0200, Vladimir Kempik
                  wrote: <br>
                  <blockquote type="cite">Hello <br>
                    <br>
                    Thanks for comments, here is updated webrev - <br>
                    <a moz-do-not-send="true"
                      class="moz-txt-link-freetext"
                      href="http://cr.openjdk.java.net/%7Evkempik/8016302/webrev.01/">http://cr.openjdk.java.net/~vkempik/8016302/webrev.01/</a>
                    <br>
                    <br>
                  </blockquote>
                  - the comments in
                  DirtyCardQueueSet::mut_process_buffer() should be <br>
                  adapted that we use uints now. They talk of worker_i
                  of -1. <br>
                  <br>
                  E.g. <br>
                  <br>
                  130  // If worker_i is not -1 then the thread has
                  already claimed <br>
                  131  // a par_id. We make note of it using the
                  already_claimed value <br>
                  <br>
                  - in G1GCPhaseTimes::print_stats() we need to use
                  UINT32_FORMAT instead <br>
                  of %d as the printf() specifier for uints. <br>
                  <br>
                  - same in ScanRSClosure::printCard(). <br>
                  <br>
                  Thanks, <br>
                     Thomas <br>
                  <br>
                </blockquote>
                <br>
              </blockquote>
              <br>
            </blockquote>
            <br>
          </blockquote>
          <br>
        </blockquote>
        <br>
      </blockquote>
      <br>
    </blockquote>
    <br>
  </body>
</html>