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

[John Cuthbertson]

Thanks Paul.

JohnC

> On Oct 18, 2019, at 6:59 AM, Hohensee, Paul <hohensee at amazon.com> wrote:
> 
> 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
> 
> 
> 
> 
>