[8u] RFR (S): Backport: JDK-8048556: Unnecessary GCLocker-initiated young GCs

[Hohensee, Paul]

Looks good. My previous comment on the test was misinformed: the one in the 14 patch depends on unified logging output, so can't be used as is.

Thanks,

Paul

On 10/17/19, 11:11 AM, "jdk8u-dev on behalf of John Cuthbertson" <jdk8u-dev-bounces at openjdk.java.net on behalf of johnc at azul.com> wrote:

    Hi Everyone,
    
    New webrev at http://cr.openjdk.java.net/~johnc/8048556/webrev.01/
    
    I went back and forth about whether to assert that setting the _full attribute using the max generation level matched the cause but decided to take the assert out to keep the change much closer to the original patch.
    
    Retested using the modified version of Kim’s test and TonyP’s test.
    
    Thanks,
    
    JohnC
    
    On Oct 15, 2019, at 12:16 PM, John Cuthbertson <johnc at azul.com<mailto:johnc at azul.com>> wrote:
    
    Hi Everyone,
    
    I’d like to contribute a back port of the changes for JDK-8048556 to jdk8u-dev/hotspot. You can find the webrev at http://cr.openjdk.java.net/~johnc/8048556/webrev.00/.
    
    Please bare with me since it’s been a long time since I contributed to OpenJDK. I apologize for any mis-steps in advance. Andrew has said in email that he has/had added jdk8u-fix-request label to the bug entry.
    
    As you would expect the import did not apply cleanly — mostly because of different paths so a lot had to manually back ported. The main places that needed some hand massaging were:
    
    * minor tweaks to the comments in g1CollectedHeap.cpp and genCollectedHeap.cpp,
    * setting the _full attribute in the VM_GenCollectFull operation (I suppose I should have followed the coding model in vmPSOperations.cpp and added a static is_cause_full() helper routine — I can do that if people wish),
    * changing Kim’s test to use GC MXBeans and output Before GC Eden information in a test local format along with some message if the Eden use was below or above 40% of max/committed and have the test grep for the bad message.
    
    I also manually tested using Tony P’s second test that was attached to the bug entry with the following combinations of options:
    
    ("-XX:+UseParNewGC"
                             "-XX:+UseSerialGC"
                             "-XX:+UseParallelGC"
                             "-XX:+UseConcMarkSweepGC"
                             "-XX:+UseG1GC"
                             "-XX:+UseParallelGC -XX:+UseParallelOldGC"
                             "-XX:+UseConcMarkSweepGC -XX:-UseParNewGC"
                             "-XX:+UseConcMarkSweepGC -XX:+GCLockerInvokesConcurrent"
                             "-XX:+UseG1GC -XX:+GCLockerInvokesConcurrent”)
    
    Please note that since CMS and G1 with -XX:+GCLockerInvokesConcurrent does not drop GCLocker initiated GCs we can still see GCs with under utilized Edens with these combinations.
    
    Once everything looks OK I think I’ll have to have someone else from Azul push the actual change.
    
    Thanks
    
    John Cuthbertson