RFR(S):8151796: compiler/whitebox/BlockingCompilation.java fails due to method not compiled

Mon Mar 14 14:43:05 UTC 2016

Hi Volker,

On 2016-03-14 14:58, Volker Simonis wrote:
> Hi Nils,
>
> 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:
>
> java.lang.Exception: public static int BlockingCompilation.foo() 
> should be compiled at level 4(but is actually compiled at level 0)
> at BlockingCompilation.main(BlockingCompilation.java:104)
>
> 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:

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.

> 121 if (WB.getMethodCompilationLevel(m) != l*&& 
> **WB.getMethodCompilationLevel(m) != 0*) {
> Also, I don't understand the following code:
> 67 // Make sure no compilations can progress, blocking compiles will hang
> 68 WB.lockCompilation(); ...   78 // Normal compile on all levels
> 79 for (int l : levels) {
> 80 WB.enqueueMethodForCompilation(m, l);
>    81         }
>    82
> 83 // restore state
> 84 WB.unlockCompilation();
> 85 while (!WB.isMethodCompiled(m)) {
> 86 Thread.sleep(100);
> 87 }
> 88 WB.deoptimizeMethod(m);
>    89         WB.clearMethodState(m);
> 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.

Blocking compiles do not get stale any more - that is included in the patch.

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.

> 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:

It can only get on the queue once. It looks like this in the log:

Start of test - not blocking

     524  257       1       BlockingCompilation::foo (7 bytes)
     625  257       1       BlockingCompilation::foo (7 bytes)   made not entrant

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)

> 103 //Verify that it actuall is uncompiled
> 104 if (WB.isMethodCompiled(m)) {
> 105 throw new Exception("Should not be compiled after deoptimization");
> 106 }
> Finally some typos:
> 103 //Verify that it actuall*y* is uncompiled 111 // Add to queue 
> a*n*d make sure it actually went well
>
> Regards,
> Volker

Thanks for feedback,
Nils Eliasson

>
>
> On Mon, Mar 14, 2016 at 12:20 PM, Nils Eliasson 
> <nils.eliasson at oracle.com <mailto:nils.eliasson at oracle.com>> wrote:
>
>     Hi,
>
>     Summary:
>     The test wasn't as robust as expected.
>
>     Solution:
>     Change the way we verify that we are having a un-blocking compilation:
>     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.
>
>     Use this to test the blocking functionality in three steps:
>     1) Verify that we are not blocking on target method as described.
>     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.
>     3) Pop directive, and redo step one - verify that target method is
>     not blocking.
>
>     Bug: https://bugs.openjdk.java.net/browse/JDK-8151796
>     Webrev: http://cr.openjdk.java.net/~neliasso/8151796/werev.03/
>     <http://cr.openjdk.java.net/%7Eneliasso/8151796/werev.03/>
>
>     Regards,
>     Nils Eliasson
>
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/hotspot-compiler-dev/attachments/20160314/7618e93e/attachment.html>