[8u] RFR: Backport of JDK-8222670: pathological case of JIT recompilation and code cache bloat

Liu, Xin xxinliu at amazon.com
Sat May 18 17:20:37 UTC 2019


Hi, Andrew, 
Thanks for review it. Even though it's a backport patch, but the review workload as high as original one.  Thanks.
My comments are inline. 

On 5/16/19, 11:47 AM, "jdk8u-dev on behalf of Andrew John Hughes" <jdk8u-dev-bounces at openjdk.java.net on behalf of gnu.andrew at redhat.com> wrote:

    On 16/05/2019 18:23, Liu, Xin wrote:
    > Hi, Reviewers,
    > 
    > Could you review this backport patch?
    > 
    > BUG: https://bugs.openjdk.java.net/browse/JDK-8222670
    > Webrev: https://cr.openjdk.java.net/~xliu/8222670/webrev.8u-dev/
    > 
    > I can’t cleanly apply the original patch to jdk8u.  I made the following changes.
    > 
    >   1.  Include JDK-8223537.  It solved suspend problem in blocking mode. One line change.
    
    I think this should be applied as a pre-requisite clean backport. Just
    flag the bug with jdk8u-fix-request, no need for a separate review.
    
I added jdk8u-fix-request for JDK-8223537
The change can trivially apply on jdk8u-dev. Here is webrev.  The order does matter,  so let's call it  3/3 patch, or last one. 
https://cr.openjdk.java.net/~xliu/8223537/webrev.8u-dev/

    >   2.  Shorter wait period.  Otherwise, it’s easy to timeout for the  test.  It’s part of a JDK-8066770.
    > https://cr.openjdk.java.net/~xliu/8222670/webrev.8u-dev/test/compiler/whitebox/CompilerWhiteBoxTest.java.udiff.html
    > 
    
    8066770 relies on a lot of WhiteBox API that is not present in 8u so I'm
    ok with including just this segment.
    
    >   1.  +1 for Tier4MinInvocationThreshold and Tier4CompileThreshold. Otherwise, we have to backport JDK-8059604, which looks like complicated.
    > 
    > https://cr.openjdk.java.net/~xliu/8222670/webrev.8u-dev/src/share/vm/prims/whitebox.cpp.udiff.html
    
    JDK-8059604 looks backportable, but I'm ok with including this commented
    workaround for now.
>>
It is backportable. Actually, Paul suggested me do it. 
I think it's complicated because JDK-8059604 is only part of campaign(JDK-8050853). Taking 8059604 alone to jdk8u seems not be very useful. 
Further, I can't evaluate how much impact of changing thresholds  from > to >= here.  
https://hg.openjdk.java.net/jdk9/jdk9/hotspot/rev/ffe9c8c82350#l5.9

I'd like to +1 in whitebox.cpp because it's just a fixture of testing. It won't have runtime risk. 
    
    There seem to be two other changes being included here:
    
    1. compileBroker.cpp changes
    
    These aren't in the original JDK-8222670. In 9, the != NULL test is
    added by "8144964: JVMCI compilations need to be disabled until the
    module system is initialized". 8 has neither JVMCI or a module system,
    so not sure why this test is being added here. Why is this included?
>>    
I do need the "task != NULL" protection here. Otherwise,  remove(NULL/*task*/); will cash JVM. 
If duplication happens ,  this patch will ditch the task from select_task and return NULL. 

For JDK-8144964, it’s all about JVMCI. That's why I don't want to backport it.
 

    2. skiponTieredCompilation in CompilerWhiteBoxTest.java
    
    This is part of "8059575: JEP-JDK-8043304: Test task: Tiered Compilation
    level transition tests" which should also be backported as a pre-requisite.
>>    
I backported 8059575 to jdk8u-dev.  It's the 1/3 patch, or the first one to apply. 
Here is webrev: https://cr.openjdk.java.net/~xliu/8059575/webrev.01/

I was wrong at JDK-8059575. It need to polish a little bit to pass all tests. 
Here is my change: 
1) add compile_id in NMethod.java. 
I got lost from here. 
https://hg.openjdk.java.net/jdk9/jdk9/rev/c82ea5393dda
2) modify @library for LevelTransitionTest.java and ConstantGettersTransitionsTest.java. 
It's because auxiliary library locates differently. 


    > Thanks,
    > --lx
    > 
    
    Thanks,
    -- 
    Andrew :)
    
    Senior Free Java Software Engineer
    Red Hat, Inc. (http://www.redhat.com)
    
    PGP Key: ed25519/0xCFDA0F9B35964222 (hkp://keys.gnupg.net)
    Fingerprint = 5132 579D D154 0ED2 3E04  C5A0 CFDA 0F9B 3596 4222
    https://keybase.io/gnu_andrew
    
    



More information about the jdk8u-dev mailing list