RFR(S):8151796: compiler/whitebox/BlockingCompilation.java fails due to method not compiled
Nils Eliasson
nils.eliasson at oracle.com
Tue Mar 15 09:25:24 UTC 2016
Hi,
On 2016-03-14 16:16, Volker Simonis wrote:
>
>
> On Mon, Mar 14, 2016 at 3:43 PM, Nils Eliasson
> <nils.eliasson at oracle.com <mailto:nils.eliasson at oracle.com>> wrote:
>
> 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
>
> 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):
I changed to compiling on the highest available comp level.
http://cr.openjdk.java.net/~neliasso/8151796/webrev.07/
> 78 // Normal compile on all levels
> 79 for (int l : levels) {
> 80 WB.enqueueMethodForCompilation(m, l);
> 81 }
>
> Besides that, your changes look good!
> Regards, Volker
Thank you,
Nils
> 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/20160315/b02813f0/attachment-0001.html>
More information about the hotspot-compiler-dev
mailing list