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

[Hohensee, Paul]

Looks good overall.

As you suggest, please add is_cause_full() in gcVMOperations.hpp.

I expect your change to TestExcessGCLockerCollections.java is extensive enough for someone besides me to say it's worth doing in tip and backporting it. If you can use Kim's original code for this patch, I'd be happy to file an issue and handle it.

Thanks,

Paul

On 10/15/19, 12:17 PM, "jdk8u-dev on behalf of John Cuthbertson" <jdk8u-dev-bounces at openjdk.java.net on behalf of 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