<html>
  <head>
    <meta content="text/html; charset=utf-8" http-equiv="Content-Type">
  </head>
  <body text="#000000" bgcolor="#FFFFFF">
    <br>
    <br>
    <div class="moz-cite-prefix">On 2016-04-18 12:24, Nils Eliasson
      wrote:<br>
    </div>
    <blockquote cite="mid:5714B5CB.70705@oracle.com" type="cite">
      <meta content="text/html; charset=utf-8" http-equiv="Content-Type">
      Hi,<br>
      <br>
      <div class="moz-cite-prefix">On 2016-04-15 22:43, Christian
        Thalinger wrote:<br>
      </div>
      <blockquote
        cite="mid:B3A4C7A2-E9B6-403A-BF0A-8B64049F4338@oracle.com"
        type="cite">
        <meta http-equiv="Content-Type" content="text/html;
          charset=utf-8">
        <br class="">
        <div>
          <blockquote type="cite" class="">
            <div class="">On Apr 15, 2016, at 3:06 AM, Nils Eliasson
              <<a moz-do-not-send="true"
                class="moz-txt-link-abbreviated"
                href="mailto:nils.eliasson@oracle.com">nils.eliasson@oracle.com</a>>

              wrote:</div>
            <br class="Apple-interchange-newline">
            <div class="">
              <meta content="text/html; charset=utf-8"
                http-equiv="Content-Type" class="">
              <div text="#000000" bgcolor="#FFFFFF" class=""> Hi,<br
                  class="">
                <br class="">
                <div class="moz-cite-prefix">On 2016-04-14 20:45,
                  Christian Thalinger wrote:<br class="">
                </div>
                <blockquote
                  cite="mid:CCC9CCE9-1183-44DE-AA06-827F22B584E1@oracle.com"
                  type="cite" class="">
                  <meta http-equiv="Content-Type" content="text/html;
                    charset=utf-8" class="">
                  <br class="">
                  <div class="">
                    <blockquote type="cite" class="">
                      <div class="">On Apr 14, 2016, at 2:43 AM, Nils
                        Eliasson <<a moz-do-not-send="true"
                          class="moz-txt-link-abbreviated"
                          href="mailto:nils.eliasson@oracle.com">nils.eliasson@oracle.com</a>>


                        wrote:</div>
                      <br class="Apple-interchange-newline">
                      <div class="">
                        <div class="">I moved the reasons to
                          CompileTask.hpp and put it together with the
                          names list. Also changed the type from int to
                          CompileReason as Igor suggested.<br class="">
                          <br class="">
                          It gets verbose in the method declarations in
                          compileBroker</div>
                      </div>
                    </blockquote>
                    <div class=""><br class="">
                    </div>
                    Don’t worry about this.</div>
                  <div class=""><br class="">
                    <blockquote type="cite" class="">
                      <div class="">
                        <div class=""> and sometimes I think
                          CompileReason should be declared in
                          CompileBroker because it is mostly used there.
                          On the other hand, CompileTask is the keeper
                          of the CompileReason so it makes sense too.<br
                            class="">
                        </div>
                      </div>
                    </blockquote>
                    <div class=""><br class="">
                    </div>
                    Yes, that’s the right place.</div>
                  <div class=""><br class="">
                    <blockquote type="cite" class="">
                      <div class="">
                        <div class=""><br class="">
                          New webrev:<br class="">
                          <a moz-do-not-send="true"
                            href="http://cr.openjdk.java.net/%7Eneliasso/8153013/webrev.03/"
                            class="">http://cr.openjdk.java.net/~neliasso/8153013/webrev.03/</a><br
                            class="">
                        </div>
                      </div>
                    </blockquote>
                    <div class=""><br class="">
                    </div>
                    <pre style="background-color: rgb(238, 238, 238);" class=""><font class="" color="blue"><b class="">+   bool         can_become_stale() const          {</b></font>
<font class="" color="blue"><b class="">+     return !_is_blocking && (_compile_reason < Reason_Whitebox);</b></font>
<font class="" color="blue"><b class="">+   }</b></font></pre>
                    I’m not a fan of implicit contracts just defined by
                    comments.  This method doesn’t seem to be
                    performance critical so I would suggest to use a
                    switch-case.  An attribute on the enum would be much
                    better but we all know this isn’t Java.</div>
                </blockquote>
                <br class="">
                As you suggested:<br class="">
                <a moz-do-not-send="true" class="moz-txt-link-freetext"
href="http://cr.openjdk.java.net/%7Eneliasso/8153013/webrev.04">http://cr.openjdk.java.net/~neliasso/8153013/webrev.04</a><br
                  class="">
              </div>
            </div>
          </blockquote>
          <div><br class="">
          </div>
          Thanks.  A space is missing and the closing } indent is wrong:</div>
        <div>
          <pre style="background-color: rgb(238, 238, 238);" class=""><font class="" color="blue"><b class="">+   bool         can_become_stale() const          {</b></font>
<font class="" color="blue"><b class="">+     switch(_compile_reason) {</b></font>
<font class="" color="blue"><b class="">+       case Reason_BackedgeCount:</b></font>
<font class="" color="blue"><b class="">+       case Reason_InvocationCount:</b></font>
<font class="" color="blue"><b class="">+       case Reason_Tiered:</b></font>
<font class="" color="blue"><b class="">+         return !_is_blocking;</b></font>
<font class="" color="blue"><b class="">+       }</b></font>
<font class="" color="blue"><b class="">+     return false;</b></font>
<font class="" color="blue"><b class="">+   }</b></font></pre>
        </div>
      </blockquote>
    </blockquote>
    And I fixed the indentation.<br>
    <br>
    Webrev: <a class="moz-txt-link-freetext" href="http://cr.openjdk.java.net/~neliasso/8153013/webrev.05/">http://cr.openjdk.java.net/~neliasso/8153013/webrev.05/</a><br>
    <br>
    Thanks!<br>
    Nils<br>
    <blockquote cite="mid:5714B5CB.70705@oracle.com" type="cite">
      <blockquote
        cite="mid:B3A4C7A2-E9B6-403A-BF0A-8B64049F4338@oracle.com"
        type="cite">
        <div> Also, what about:</div>
        <div>
          <pre style="background-color: rgb(238, 238, 238);" class=""><font class="" color="blue"><b class="">+       Reason_None,</b></font></pre>
        </div>
        <div>
          <pre style="background-color: rgb(238, 238, 238);" class=""><font class="" color="blue"><b class="">+       Reason_CTW,              // Compile the world</b></font>
<font class="" color="blue"><b class="">+       Reason_Replay,           // ciReplay</b></font></pre>
          These were covered before.</div>
      </blockquote>
      Reason_None - is only used for bounds checking together with
      Reason_Count.<br>
      Reason_Replay - if these compilations can get stale we can get
      indeterminism in replay.<br>
      Reason_CTW - CTW could silently drop compiles -> more
      indeterminism.<br>
    </blockquote>
    <blockquote cite="mid:5714B5CB.70705@oracle.com" type="cite"> <br>
      Regards,<br>
      Nils<br>
      <br>
      <blockquote
        cite="mid:B3A4C7A2-E9B6-403A-BF0A-8B64049F4338@oracle.com"
        type="cite">
        <div><br class="">
          <blockquote type="cite" class="">
            <div class="">
              <div text="#000000" bgcolor="#FFFFFF" class=""> <br
                  class="">
                Also made reasons CTW and Replay not stale-able. <br
                  class="">
                <br class="">
                Thanks!<br class="">
                Nils<br class="">
                <br class="">
                <blockquote
                  cite="mid:CCC9CCE9-1183-44DE-AA06-827F22B584E1@oracle.com"
                  type="cite" class="">
                  <div class=""><br class="">
                    <blockquote type="cite" class="">
                      <div class="">
                        <div class=""><br class="">
                          Thanks!<br class="">
                          Nils<br class="">
                          <br class="">
                          On 2016-04-13 23:34, Vladimir Kozlov wrote:<br
                            class="">
                          <blockquote type="cite" class="">Very nice, I
                            like it.<br class="">
                            <br class="">
                            One note. CompileReason (and its names)
                            should be CompileTask class where it is
                            recorded. Then
                            CompileTask::can_become_stale() can be in
                            header file so it is inlinined on all
                            platforms.<br class="">
                            <br class="">
                            Thanks,<br class="">
                            Vladimir<br class="">
                            <br class="">
                            On 4/13/16 5:59 AM, Nils Eliasson wrote:<br
                              class="">
                            <blockquote type="cite" class="">Hi,<br
                                class="">
                              <br class="">
                              New webrev:<br class="">
                              <a moz-do-not-send="true"
                                href="http://cr.openjdk.java.net/%7Eneliasso/8153013/webrev.02/"
                                class="">http://cr.openjdk.java.net/~neliasso/8153013/webrev.02/</a><br
                                class="">
                              <br class="">
                              Summary<br class="">
                              Introduced an enum CompileReason with
                              members matching all the old<br class="">
                              variants, and a table containing all the
                              unchanged strings. I see the<br class="">
                              possibility of
                              removing/changing/simplifying some
                              CompileReasons but<br class="">
                              have choosen not to do so in this change.<br
                                class="">
                              <br class="">
                              Only new logic is the
                              CompileTask::can_become_stale() method.<br
                                class="">
                              <br class="">
                              Testing:<br class="">
                              Running Testset hotspot on all platforms
                              and hotspot_all on one platform<br
                                class="">
                              <br class="">
                              Regards,<br class="">
                              Nils Eliawsson<br class="">
                              <br class="">
                              On 2016-04-12 18:55, Vladimir Kozlov
                              wrote:<br class="">
                              <blockquote type="cite" class="">On
                                4/12/16 6:30 AM, Nils Eliasson wrote:<br
                                  class="">
                                <blockquote type="cite" class="">Tasks
                                  get evicted from the compile_queue if
                                  their invocation counter<br class="">
                                  hasn't increased during
                                  TieredCompileTaskTimeout.<br class="">
(AdvancedThresholdPolicy::is_stale(...)).<br class="">
                                  <br class="">
                                  I'll do a proper fix, it is the right
                                  thing to do and should be pretty<br
                                    class="">
                                  quick. I'll change the comment to an
                                  enum that represent who submitted<br
                                    class="">
                                  the compile, and add a table for the
                                  comments. This could be useful in<br
                                    class="">
                                  other settings to.<br class="">
                                </blockquote>
                                <br class="">
                                Sounds good.<br class="">
                                <br class="">
                                Thanks,<br class="">
                                Vladimir<br class="">
                                <br class="">
                                <blockquote type="cite" class=""><br
                                    class="">
                                  Regards,<br class="">
                                  Nils<br class="">
                                  <br class="">
                                  On 2016-04-08 19:09, Vladimir Kozlov
                                  wrote:<br class="">
                                  <blockquote type="cite" class="">What
                                    do you mean "stale"?<br class="">
                                    I would prefer to see the real fix
                                    as you suggested to avoid removing<br
                                      class="">
                                    WB comp tasks from queue. Adding
                                    timeout is not reliable.<br class="">
                                    <br class="">
                                    Thanks,<br class="">
                                    Vladimir<br class="">
                                    <br class="">
                                    On 4/8/16 5:27 AM, Nils Eliasson
                                    wrote:<br class="">
                                    <blockquote type="cite" class="">Hi,<br
                                        class="">
                                      <br class="">
                                      Please review this small fix of
                                      the BlockingCompilation test.<br
                                        class="">
                                      <br class="">
                                      Summary:<br class="">
                                      Add method enqueued for
                                      compilation with WB API may be
                                      removed from<br class="">
                                      the compile queue as stale.<br
                                        class="">
                                      <br class="">
                                      Solution:<br class="">
                                      Add
                                      -XX:TieredCompileTaskTimeout=60000
                                      to make sure nothing gets<br
                                        class="">
                                      stale while the test is running.
                                      (Also added some extra<br class="">
                                      checks that may spare us from
                                      waiting until timeout for
                                      failing.)<br class="">
                                      <br class="">
                                      This is an workaround but we
                                      should consider fixing something<br
                                        class="">
                                      permanent for WB API compiles -
                                      like tagging the compile<br
                                        class="">
                                      task with info about the origin of
                                      the compile. The comment field has<br
                                        class="">
                                      this information - but then it
                                      needs to be<br class="">
                                      converted to an enum.<br class="">
                                      <br class="">
                                      Bug: <a moz-do-not-send="true"
                                        class="moz-txt-link-freetext"
                                        href="https://bugs.openjdk.java.net/browse/JDK-8153013">https://bugs.openjdk.java.net/browse/JDK-8153013</a><br
                                        class="">
                                      Webrev: <a moz-do-not-send="true"
                                        class="moz-txt-link-freetext"
                                        href="http://cr.openjdk.java.net/%7Eneliasso/8153013/webrev.01/">http://cr.openjdk.java.net/~neliasso/8153013/webrev.01/</a><br
                                        class="">
                                      <br class="">
                                      Best regards,<br class="">
                                      Nils Eliasson<br class="">
                                      <br class="">
                                      <br class="">
                                      <br class="">
                                      <br class="">
                                    </blockquote>
                                  </blockquote>
                                  <br class="">
                                </blockquote>
                              </blockquote>
                              <br class="">
                            </blockquote>
                          </blockquote>
                          <br class="">
                        </div>
                      </div>
                    </blockquote>
                  </div>
                  <br class="">
                </blockquote>
                <br class="">
              </div>
            </div>
          </blockquote>
        </div>
        <br class="">
      </blockquote>
      <br>
    </blockquote>
    <br>
  </body>
</html>