<html>
  <head>
    <meta content="text/html; charset=utf-8" http-equiv="Content-Type">
  </head>
  <body bgcolor="#FFFFFF" text="#000000">
    <div class="moz-cite-prefix">On 24/06/15 13:39, Vitaly Davidovich
      wrote:<br>
    </div>
    <blockquote
cite="mid:CAHjP37E1Q-1NVu4Gq3C+F5T=pB4X65+CtQP+eM8Np3YgZwL4MQ@mail.gmail.com"
      type="cite">
      <p dir="ltr">Looks good Bengt (not a Reviewer though).</p>
    </blockquote>
    <br>
    Thanks, Vitaly!<br>
    <br>
    Bengt<br>
    <br>
    <blockquote
cite="mid:CAHjP37E1Q-1NVu4Gq3C+F5T=pB4X65+CtQP+eM8Np3YgZwL4MQ@mail.gmail.com"
      type="cite">
      <p dir="ltr">sent from my phone</p>
      <div class="gmail_quote">On Jun 24, 2015 6:29 AM, "Bengt Rutisson"
        <<a moz-do-not-send="true"
          href="mailto:bengt.rutisson@oracle.com">bengt.rutisson@oracle.com</a>>
        wrote:<br type="attribution">
        <blockquote class="gmail_quote" style="margin:0 0 0
          .8ex;border-left:1px #ccc solid;padding-left:1ex"><br>
          On 2015-06-24 11:27, Bertrand Delsart wrote:<br>
          <blockquote class="gmail_quote" style="margin:0 0 0
            .8ex;border-left:1px #ccc solid;padding-left:1ex">
            Looks good... and much simpler :-)<br>
          </blockquote>
          <br>
          Thanks, Bertrand!<br>
          <br>
          Bengt<br>
          <br>
          <blockquote class="gmail_quote" style="margin:0 0 0
            .8ex;border-left:1px #ccc solid;padding-left:1ex">
            <br>
            Bertrand.<br>
            <br>
            On 24/06/2015 10:31, Bengt Rutisson wrote:<br>
            <blockquote class="gmail_quote" style="margin:0 0 0
              .8ex;border-left:1px #ccc solid;padding-left:1ex">
              <br>
              <br>
              On 2015-06-24 10:34, Per Liden wrote:<br>
              <blockquote class="gmail_quote" style="margin:0 0 0
                .8ex;border-left:1px #ccc solid;padding-left:1ex">
                On 2015-06-24 10:15, Bengt Rutisson wrote:<br>
                <blockquote class="gmail_quote" style="margin:0 0 0
                  .8ex;border-left:1px #ccc solid;padding-left:1ex">
                  <br>
                  Hi Per,<br>
                  <br>
                  Thanks for looking at this!<br>
                  <br>
                  On 2015-06-24 10:09, Per Liden wrote:<br>
                  <blockquote class="gmail_quote" style="margin:0 0 0
                    .8ex;border-left:1px #ccc solid;padding-left:1ex">
                    Hi Bengt,<br>
                    <br>
                    On 2015-06-24 09:28, Bengt Rutisson wrote:<br>
                    <blockquote class="gmail_quote" style="margin:0 0 0
                      .8ex;border-left:1px #ccc solid;padding-left:1ex">
                      <br>
                      <br>
                      Hi Vitaly,<br>
                      <br>
                      On 2015-06-23 23:53, Vitaly Davidovich wrote:<br>
                      <blockquote class="gmail_quote" style="margin:0 0
                        0 .8ex;border-left:1px #ccc
                        solid;padding-left:1ex">
                        <br>
                        Naive question - could this be converted into a
                        numeric state field<br>
                        that indicates the lifecycle (e.g 1 = in
                        progress, 2 = started)?<br>
                        <br>
                      </blockquote>
                      <br>
                      Good question! That's a much simpler and more
                      stable solution.<br>
                      <br>
                      New webrev:<br>
                      <a moz-do-not-send="true"
                        href="http://cr.openjdk.java.net/%7Ebrutisso/8129626/webrev.02/"
                        rel="noreferrer" target="_blank">http://cr.openjdk.java.net/~brutisso/8129626/webrev.02/</a><br>
                    </blockquote>
                    <br>
                    This looks much nicer, and I can't think of any
                    reason why that<br>
                    wouldn't work. One little request though, can we
                    name the states to<br>
                    match the function names, like
                    Idle/Started/InProgress? And<br>
                    clear_in_progress() should probably be set_idle() to
                    align with the<br>
                    rest.<br>
                  </blockquote>
                  <br>
                  Yes, I was struggling a bit with the naming. What do
                  you think about<br>
                  this?<br>
                  <br>
                  <a moz-do-not-send="true"
                    href="http://cr.openjdk.java.net/%7Ebrutisso/8129626/webrev.03/"
                    rel="noreferrer" target="_blank">cr.openjdk.java.net/~brutisso/8129626/webrev.03/</a><br>
                </blockquote>
                <br>
                Looks good!<br>
              </blockquote>
              <br>
              Thanks Per!<br>
              <br>
              Bengt<br>
              <br>
              <blockquote class="gmail_quote" style="margin:0 0 0
                .8ex;border-left:1px #ccc solid;padding-left:1ex">
                <br>
                cheers,<br>
                /Per<br>
                <br>
                <blockquote class="gmail_quote" style="margin:0 0 0
                  .8ex;border-left:1px #ccc solid;padding-left:1ex">
                  <br>
                  Thanks,<br>
                  Bengt<br>
                  <blockquote class="gmail_quote" style="margin:0 0 0
                    .8ex;border-left:1px #ccc solid;padding-left:1ex">
                    <br>
                    cheers,<br>
                    /Per<br>
                    <br>
                    <blockquote class="gmail_quote" style="margin:0 0 0
                      .8ex;border-left:1px #ccc solid;padding-left:1ex">
                      <br>
                      Thanks,<br>
                      Bengt<br>
                      <br>
                      <br>
                      <br>
                      <blockquote class="gmail_quote" style="margin:0 0
                        0 .8ex;border-left:1px #ccc
                        solid;padding-left:1ex">
                        sent from my phone<br>
                        <br>
                        On Jun 23, 2015 5:42 PM, "bill pittore" <<a
                          moz-do-not-send="true"
                          href="mailto:bill.pittore@oracle.com"
                          target="_blank">bill.pittore@oracle.com</a><br>
                        <mailto:<a moz-do-not-send="true"
                          href="mailto:bill.pittore@oracle.com"
                          target="_blank">bill.pittore@oracle.com</a>>>
                        wrote:<br>
                        <br>
                        Â  Â  Generally when you have a storestore on the
                        write side you need a<br>
                        Â  Â  loadload on the read side to prevent the
                        second read from<br>
                        floating<br>
                        Â  Â  above the first one. The reading thread
                        could read in_progress as<br>
                        Â  Â  0 before it reads starting. Meanwhile the
                        write thread writes<br>
                        1 to<br>
                        Â  Â  in_progress, issues storestore, clears
                        starting. Reading thread<br>
                        Â  Â  then reads starting as 0. I don't know if
                        the CGC mutex somehow<br>
                        Â  Â  eliminates this issue as I'm not familiar
                        with the code in<br>
                        detail.<br>
                        <br>
                        Â  Â  bill<br>
                        <br>
                        Â  Â  On 6/23/2015 4:25 PM, Bengt Rutisson wrote:<br>
                        <blockquote class="gmail_quote" style="margin:0
                          0 0 .8ex;border-left:1px #ccc
                          solid;padding-left:1ex">
                          <br>
                          Â  Â  Hi everyone,<br>
                          <br>
                          Â  Â  Could I have a couple of reviews for this
                          change?<br>
                          <br>
                          Â  Â  <a moz-do-not-send="true"
                            href="https://bugs.openjdk.java.net/browse/JDK-8129626"
                            rel="noreferrer" target="_blank">https://bugs.openjdk.java.net/browse/JDK-8129626</a><br>
                          <a moz-do-not-send="true"
                            href="http://cr.openjdk.java.net/%7Ebrutisso/8129626/webrev.00/"
                            rel="noreferrer" target="_blank">http://cr.openjdk.java.net/~brutisso/8129626/webrev.00/</a><br>
                          <<a moz-do-not-send="true"
                            href="http://cr.openjdk.java.net/%7Ebrutisso/8129626/webrev.00/"
                            rel="noreferrer" target="_blank">http://cr.openjdk.java.net/%7Ebrutisso/8129626/webrev.00/</a>><br>
                          <br>
                          Â  Â  We need to add a barrier between the calls
                          to set_in_progress()<br>
                          Â  Â  and clear_started() to make sure that
                          other threads sees the<br>
                          Â  Â  correct value when they use<br>
                          ConcurrentMarkThread::during_cycle().<br>
                          <br>
                          Â  Â  Thanks to Per and Bertrand for helping out
                          identifying and<br>
                          Â  Â  sorting this out.<br>
                          <br>
                          Â  Â  From the bug report:<br>
                          <br>
                          Â  Â  ConcurrentMarkThread::during_cycle() is
                          implemented as:<br>
                          <br>
                          Â  Â  bool during_cycle() { return started() ||
                          in_progress(); }<br>
                          <br>
                          Â  Â  So, it checks both
                          ConcurrentMarkThread::_started and<br>
                          Â  Â  ConcurrentMarkThread::_in_progress and
                          they are meant to<br>
                          overlap.<br>
                          Â  Â  That is, we should not set _started to
                          false until after we have<br>
                          Â  Â  set _in_progress to true. This is done in<br>
                          sleepBeforeNextCycle():<br>
                          <br>
                          Â  Â  void
                          ConcurrentMarkThread::sleepBeforeNextCycle() {<br>
                          Â  Â  Â  // We join here because we don't want to
                          do the<br>
                          Â  Â  "shouldConcurrentMark()"<br>
                          Â  Â  Â  // below while the world is otherwise
                          stopped.<br>
                          Â  Â  Â  assert(!in_progress(), "should have been
                          cleared");<br>
                          <br>
                          Â  Â  Â  MutexLockerEx x(CGC_lock,
                          Mutex::_no_safepoint_check_flag);<br>
                          Â  Â  Â  while (!started() &&
                          !_should_terminate) {<br>
CGC_lock->wait(Mutex::_no_safepoint_check_flag);<br>
                          Â  Â  Â  }<br>
                          <br>
                          Â  Â  Â  if (started()) {<br>
                          Â  Â  Â  Â  set_in_progress();<br>
                          Â  Â  Â  Â  clear_started();<br>
                          Â  Â  Â  }<br>
                          Â  Â  }<br>
                          <br>
                          Â  Â  On non-TSO platforms there is a risk that
                          the write to<br>
                          Â  Â  _in_progress (from set_in_progress()) is
                          seen by other threads<br>
                          Â  Â  after the write to _started (in
                          clear_started()). In that case<br>
                          Â  Â  there is a window when during_cycle() may
                          return false even<br>
                          Â  Â  though we are in a concurrent cycle.<br>
                          <br>
                          <br>
                          Â  Â  Thanks,<br>
                          Â  Â  Bengt<br>
                        </blockquote>
                        <br>
                      </blockquote>
                      <br>
                    </blockquote>
                  </blockquote>
                  <br>
                </blockquote>
              </blockquote>
              <br>
            </blockquote>
            <br>
            <br>
          </blockquote>
          <br>
        </blockquote>
      </div>
    </blockquote>
    <br>
  </body>
</html>