<html>
  <head>
    <meta content="text/html; charset=utf-8" http-equiv="Content-Type">
  </head>
  <body text="#000000" bgcolor="#FFFFFF">
    Hi,<br>
    <br>
    On 2016-03-14 16:16, Volker Simonis wrote:<br>
    <blockquote
cite="mid:CA+3eh11pZbEaheX_2cqWR4CeqvsMCv-PqCkzmgXUjeMYG3MFXw@mail.gmail.com"
      type="cite">
      <div dir="ltr"><br>
        <div class="gmail_extra"><br>
          <div class="gmail_quote">On Mon, Mar 14, 2016 at 3:43 PM, Nils
            Eliasson <span dir="ltr"><<a moz-do-not-send="true"
                href="mailto:nils.eliasson@oracle.com" target="_blank">nils.eliasson@oracle.com</a>></span>
            wrote:<br>
            <blockquote class="gmail_quote" style="margin:0px 0px 0px
              0.8ex;border-left:1px solid
              rgb(204,204,204);padding-left:1ex">
              <div text="#000000" bgcolor="#FFFFFF"> Hi Volker,<span
                  class=""><br>
                  <br>
                  <div>On 2016-03-14 14:58, Volker Simonis wrote:<br>
                  </div>
                  <blockquote type="cite">
                    <div dir="ltr">
                      <div>Hi Nils,<br>
                        <br>
                        thanks for improving the test. I think your fix
                        solves some problems with regard to fast,
                        non-blocking compiles which are wrongly
                        interpreted as blocking by the test. But what
                        about the initial test failure:<br>
                        <br>
                        java.lang.Exception: public static int
                        BlockingCompilation.foo() should be compiled at
                        level 4(but is actually compiled at level 0)<br>
                        at
                        BlockingCompilation.main(BlockingCompilation.java:104)<br>
                        <br>
                        This is from the loop which does blocking
                        compilations. It seems that a method enqueued
                        for level 4 couldn't be compiled at all. I don't
                        know the exact reason, but one could be for
                        example that the code cache was full or that the
                        compiler bailed out because of another reason.
                        I'm not sure we can accurately handle this
                        situation in the test. Maybe we should tolerate
                        if a method couldn't be compiled at all:<br>
                      </div>
                    </div>
                  </blockquote>
                  <br>
                </span> This failure only happened on (slow) non-tiered
                platforms and the log looked like that as if the
                compiler even hadn't been put on the compile queue. In
                the first version of my rewrite I checked the return
                value from enqueueMethodForCompilation to make sure the
                compile was actually added. But then I changed my mind
                and focused on just testing the blocking functionality.
                <br>
                <span class=""> <br>
                  <blockquote type="cite">
                    <div dir="ltr">
                      <div>
                        <pre><span> 121                 if (WB.getMethodCompilationLevel(m) != l<b> && </b></span><span><b><span>WB.getMethodCompilationLevel(m) != 0</span></b>) {</span>
</pre>
                      </div>
                      Also, I don't understand the following code:<br>
                      <pre><span>  67         // Make sure no compilations can progress, blocking compiles will hang</span>
<span>  68         WB.lockCompilation();
  ...
</span> <span> 78         // Normal compile on all levels</span>
<span>  79         for (int l : levels) {</span>
<span>  80            WB.enqueueMethodForCompilation(m, l);</span>
  81         }
  82 
<a moz-do-not-send="true" name="-986868320_8"></a><span>  83         // restore state</span>
<span>  84         WB.unlockCompilation();</span>
<span>  85         while (!WB.isMethodCompiled(m)) {</span>
<span>  86           Thread.sleep(100);</span>
<span>  87         }</span>
<span>  88         WB.deoptimizeMethod(m);</span>
  89         WB.clearMethodState(m);
<span></span></pre>
                      You enqueue the methods on all levels (let's
                      assume 1,2,3,4). Then you wait until the method
                      gets compiled at a level (lets say at level 1). I
                      think this is already shaky, because these are
                      non-blocking compiles of a method which hasn't
                      been called before, so the requests can be easily
                      get stale. </div>
                  </blockquote>
                  <br>
                </span> Blocking compiles do not get stale any more -
                that is included in the patch.<br>
                <br>
                Only one item will actually be added to the compile
                queue - the rest will be dropped because the method is
                already enqueued. The loop makes the code work on all
                VM-flavours (client, serverm tiered) without worrying
                about compilation levels. The compilation-lock prevents
                any compilations from completing - so the all calls on
                enqueueMethodForCompilation() will be deterministic, and
                most important - we will get a deterministic result
                (hanged VM) if the method is blocking here.<span
                  class=""><br>
                  <br>
                  <blockquote type="cite">
                    <div dir="ltr">But lets say the method will be
                      compiled at level one. You then deoptimze and
                      clear the method state. But the queue can still
                      contain the compilation requests for the three
                      other levels which can lead to errors in the
                      following test:<br>
                    </div>
                  </blockquote>
                  <br>
                </span> It can only get on the queue once. It looks like
                this in the log:<br>
                <br>
                Start of test - not blocking <br>
                <pre style="color:rgb(0,0,0);font-style:normal;font-variant:normal;font-weight:normal;letter-spacing:normal;line-height:normal;text-align:start;text-indent:0px;text-transform:none;word-spacing:0px;word-wrap:break-word;white-space:pre-wrap">    524  257       1       BlockingCompilation::foo (7 bytes)
    625  257       1       BlockingCompilation::foo (7 bytes)   made not entrant
</pre>
              </div>
            </blockquote>
            <div>OK, but then the following loop is useless (and the
              comment misleading) because we actually only enqueue and
              compile on one level (the first one which is available):<br>
            </div>
          </div>
        </div>
      </div>
    </blockquote>
    I changed to compiling on the highest available comp level.<br>
    <br>
    <a class="moz-txt-link-freetext" href="http://cr.openjdk.java.net/~neliasso/8151796/webrev.07/">http://cr.openjdk.java.net/~neliasso/8151796/webrev.07/</a><br>
    <blockquote
cite="mid:CA+3eh11pZbEaheX_2cqWR4CeqvsMCv-PqCkzmgXUjeMYG3MFXw@mail.gmail.com"
      type="cite">
      <div dir="ltr">
        <div class="gmail_extra">
          <div class="gmail_quote">
            <div> <span class="">
                <pre> <span> 78         // Normal compile on all levels</span>
<span>  79         for (int l : levels) {</span>
<span>  80            WB.enqueueMethodForCompilation(m, l);</span>
  81         }

</pre>
                <pre><span style="font-family:arial,helvetica,sans-serif">Besides that, your changes look good!
</span></pre>
                <pre><span style="font-family:arial,helvetica,sans-serif">Regards,
Volker
</span></pre>
              </span></div>
          </div>
        </div>
      </div>
    </blockquote>
    <br>
    Thank you,<br>
    Nils<br>
    <br>
    <blockquote
cite="mid:CA+3eh11pZbEaheX_2cqWR4CeqvsMCv-PqCkzmgXUjeMYG3MFXw@mail.gmail.com"
      type="cite">
      <div dir="ltr">
        <div class="gmail_extra">
          <div class="gmail_quote">
            <div><span class="">
                <pre>
</pre>
              </span></div>
            <blockquote class="gmail_quote" style="margin:0px 0px 0px
              0.8ex;border-left:1px solid
              rgb(204,204,204);padding-left:1ex">
              <div text="#000000" bgcolor="#FFFFFF">
                <pre style="color:rgb(0,0,0);font-style:normal;font-variant:normal;font-weight:normal;letter-spacing:normal;line-height:normal;text-align:start;text-indent:0px;text-transform:none;word-spacing:0px;word-wrap:break-word;white-space:pre-wrap">Directive added, then blocking part where all levels are tested:

 1 compiler directives added
    626  258    b  1       BlockingCompilation::foo (7 bytes)
    627  258       1       BlockingCompilation::foo (7 bytes)   made not entrant
    627  259    b  2       BlockingCompilation::foo (7 bytes)
    628  259       2       BlockingCompilation::foo (7 bytes)   made not entrant
    629  260    b  3       BlockingCompilation::foo (7 bytes)
    630  260       3       BlockingCompilation::foo (7 bytes)   made not entrant
    630  261    b  4       BlockingCompilation::foo (7 bytes)
    632  261       4       BlockingCompilation::foo (7 bytes)   made not entrant


And finally the non-blocking part where only one level gets compiled:

 633  262       1       BlockingCompilation::foo (7 bytes)</pre>
                <span class=""> <br>
                  <blockquote type="cite">
                    <div dir="ltr">
                      <pre><span> 103                 //Verify that it actuall is uncompiled</span>
<span> 104                 if (WB.isMethodCompiled(m)) {</span>
<span> 105                     throw new Exception("Should not be compiled after deoptimization");</span>
<span> 106                 }</span>
<span></span></pre>
                      <div class="gmail_extra">Finally some typos:<br>
                        <pre><span> 103                 //Verify that it actuall<b>y</b> is uncompiled

</span><span> 111                 // Add to queue a<b>n</b>d make sure it actually went well</span>
<span></span><span></span></pre>
                        <br>
                      </div>
                      <div class="gmail_extra">Regards,<br>
                      </div>
                      <div class="gmail_extra">Volker<br>
                      </div>
                    </div>
                  </blockquote>
                  <br>
                </span> Thanks for feedback,<br>
                Nils Eliasson<span class=""><br>
                  <br>
                  <blockquote type="cite">
                    <div dir="ltr">
                      <div class="gmail_extra"><br>
                      </div>
                      <div class="gmail_extra"><br>
                        <div class="gmail_quote">On Mon, Mar 14, 2016 at
                          12:20 PM, Nils Eliasson <span dir="ltr"><<a
                              moz-do-not-send="true"
                              href="mailto:nils.eliasson@oracle.com"
                              target="_blank"><a class="moz-txt-link-abbreviated" href="mailto:nils.eliasson@oracle.com">nils.eliasson@oracle.com</a></a>></span>
                          wrote:<br>
                          <blockquote class="gmail_quote"
                            style="margin:0px 0px 0px
                            0.8ex;border-left:1px solid
                            rgb(204,204,204);padding-left:1ex">Hi,<br>
                            <br>
                            Summary:<br>
                            The test wasn't as robust as expected.<br>
                            <br>
                            Solution:<br>
                            Change the way we verify that we are having
                            a un-blocking compilation:<br>
                            First lock the compilation queue - no new
                            compiles will be completed. Enqueue method
                            for compilation. If the method is compiled
                            blockingly - the java thread will hang since
                            the compile can't complete as long as the
                            compile queue is locked.<br>
                            <br>
                            Use this to test the blocking functionality
                            in three steps:<br>
                            1) Verify that we are not blocking on target
                            method as described.<br>
                            2) Add compiler directive with instruction
                            to block on target method - verify that it
                            can be compiled on all levels. If it is not
                            blocking it will eventually be stalled for a
                            moment in the compiler queue and the test
                            will fail.<br>
                            3) Pop directive, and redo step one - verify
                            that target method is not blocking.<br>
                            <br>
                            Bug: <a moz-do-not-send="true"
                              href="https://bugs.openjdk.java.net/browse/JDK-8151796"
                              rel="noreferrer" target="_blank">https://bugs.openjdk.java.net/browse/JDK-8151796</a><br>
                            Webrev:  <a moz-do-not-send="true"
                              href="http://cr.openjdk.java.net/%7Eneliasso/8151796/werev.03/"
                              rel="noreferrer" target="_blank">http://cr.openjdk.java.net/~neliasso/8151796/werev.03/</a><br>
                            <br>
                            Regards,<br>
                            Nils Eliasson<br>
                          </blockquote>
                        </div>
                        <br>
                      </div>
                    </div>
                  </blockquote>
                  <br>
                </span></div>
            </blockquote>
          </div>
          <br>
        </div>
      </div>
    </blockquote>
    <br>
  </body>
</html>