8222670 patch review: prevent downgraded tasks from recompiling

[Tobias Hartmann]

Hi,

in the bug description you state:
> CompileBroker::compile_method fails to detect the pre-existing nmethod because comp_level doesn't match

But why is that? If a downgraded compilation succeeded at level 2, shouldn't a re-compilation at the
same level be detected by CompileBroker::compilation_is_complete() in CompileBroker::compile_method()?

You need to update the copyright date in Level2RecompilationTest.java (should be 2019 only).

Thanks,
Tobias

On 26.04.19 09:36, Liu, Xin wrote:
> Gently ping. 
> 
> Bug:  https://bugs.openjdk.java.net/browse/JDK-8222670
> I got the new revision.
> https://cr.openjdk.java.net/~xliu/8222670/webrev.03/
> 
> I finish up test Level2RecompilationTest.java.   if you want to start a OSR compilation, you have to specify bci which points to the begin of a BB.
> Give them bci = 0 is good enough  for general cases. 
> 
> Thanks,
> --lx
> 
> 
> 
> 
> On 4/19/19, 11:19 PM, "Liu, Xin" <xxinliu at amazon.com> wrote:
> 
>     hi, Severin, 
>     
>     Thanks for reviewing. Yes, it's irrelevant.  I revert it.  please check it out. 
>     https://cr.openjdk.java.net/~xliu/8222670/webrev.02/
>     
>     Please note that I added an assertion InstanceKlass::add_osr_nmethod(nmethod* n) in this webrev.
>     In my understanding, it is a potential memleak of codecache.  If there's no higher level of osr compilation, those dups will  stay in codecache forever. 
>     
>     Further, it doesn’t make sense to recompile with the same level and same bci.  With this assertion, the following tests in tier1-test failed. 
>     test/hotspot/jtreg/compiler/intrinsics/unsafe/DirectByteBufferTest.java
>     test/hotspot/jtreg/compiler/intrinsics/unsafe/HeapByteBufferTest.java
>     test/jdk/java/util/stream/test/org/openjdk/tests/java/util/stream/ToArrayOpTest.java
>     test/jdk/tools/pack200/Pack200Test.java
>     test/jdk/java/util/Arrays/SortingNearlySortedPrimitive.java
>     
>     All crashes happen as I described in JDK-8222670. Eg.  duplicated OSR compilations occur for level2.    
>     
>     Program received signal SIGSEGV, Segmentation fault.
>     # To suppress the following error report, specify this argument
>     # after -XX: or in .hotspotrc:  SuppressErrorAt=/instanceKlass.cpp:2972
>     #
>     # A fatal error has been detected by the Java Runtime Environment:
>     #
>     #  Internal Error (/src/src/hotspot/share/oops/instanceKlass.cpp:2972), pid=8347, tid=8361
>     #  assert(prev == __null || !prev->is_in_use()) failed: redundunt OSR recompilation detected. memory leak in CodeCache!
>     #
>     # JRE version: OpenJDK Runtime Environment (13.0) (slowdebug build 13-internal+0-adhoc..src)
>     # Java VM: OpenJDK 64-Bit Server VM (slowdebug 13-internal+0-adhoc..src, mixed mode, sharing, tiered, compressed oops, g1 gc, linux-amd64)
>     # Problematic frame:
>     # V  [libjvm.so+0xb3dbb4]  InstanceKlass::add_osr_nmethod(nmethod*)+0xc4
>     #
>     # No core dump will be written. Core dumps have been disabled. To enable core dumping, try "ulimit -c unlimited" before starting Java again
>     #
>     # An error report file with more information is saved as:
>     # /build/JTwork/scratch/hs_err_pid8347.log
>     
>     Program received signal SIGSEGV, Segmentation fault.
>     Compiled method (c1)   19032  752 %     2       ByteBufferTest::stepUsingAccessors @ 382 (633 bytes)
>      total in heap  [0x00007fffd8f9ff90,0x00007fffd8fac628] = 50840
>      relocation     [0x00007fffd8fa0110,0x00007fffd8fa1388] = 4728
>      main code      [0x00007fffd8fa13a0,0x00007fffd8fa7f80] = 27616
>      stub code      [0x00007fffd8fa7f80,0x00007fffd8fa86c0] = 1856
>      oops           [0x00007fffd8fa86c0,0x00007fffd8fa86c8] = 8
>      metadata       [0x00007fffd8fa86c8,0x00007fffd8fa8800] = 312
>      scopes data    [0x00007fffd8fa8800,0x00007fffd8fa9ff8] = 6136
>      scopes pcs     [0x00007fffd8fa9ff8,0x00007fffd8fac408] = 9232
>      dependencies   [0x00007fffd8fac408,0x00007fffd8fac418] = 16
>      nul chk table  [0x00007fffd8fac418,0x00007fffd8fac628] = 528
>     Compiled method (c1)   19032  752 %     2       ByteBufferTest::stepUsingAccessors @ 382 (633 bytes)
>      total in heap  [0x00007fffd8f9ff90,0x00007fffd8fac628] = 50840
>      relocation     [0x00007fffd8fa0110,0x00007fffd8fa1388] = 4728
>      main code      [0x00007fffd8fa13a0,0x00007fffd8fa7f80] = 27616
>      stub code      [0x00007fffd8fa7f80,0x00007fffd8fa86c0] = 1856
>      oops           [0x00007fffd8fa86c0,0x00007fffd8fa86c8] = 8
>      metadata       [0x00007fffd8fa86c8,0x00007fffd8fa8800] = 312
>      scopes data    [0x00007fffd8fa8800,0x00007fffd8fa9ff8] = 6136
>      scopes pcs     [0x00007fffd8fa9ff8,0x00007fffd8fac408] = 9232
>      dependencies   [0x00007fffd8fac408,0x00007fffd8fac418] = 16
>      nul chk table  [0x00007fffd8fac418,0x00007fffd8fac628] = 528
>     
>     
>     Thanks, 
>     --lx
>     
>     On 4/19/19, 9:31 AM, "Severin Gehwolf" <sgehwolf at redhat.com> wrote:
>     
>         On Thu, 2019-04-18 at 19:46 +0000, Liu, Xin wrote:
>         > Hi, hotspot-compiler group,
>         >  
>         > Could you review this webrev for JDK-8222670?
>         > https://cr.openjdk.java.net/~xliu/8222670/webrev.01/
>         
>         +++ new/test/hotspot/jtreg/compiler/tiered/TieredLevelsTest.java	2019-04-18 12:18:38.000000000 -0700
>         @@ -89,7 +89,7 @@
>                          && actual == COMP_LEVEL_LIMITED_PROFILE) {
>                      // for simple method full_profile may be replaced by limited_profile
>                      if (IS_VERBOSE) {
>         -                System.out.printf("Level check: full profiling was replaced "
>         +                System.out.println("Level check: full profiling was replaced "
>                                  + "by limited profiling. Expected: %d, actual:%d",
>                                  expected, actual);
>         
>         This seems an unintended change, is it?
>         
>         Thanks,
>         Severin
>         
>         
>     
>     
>