[14] RFR (S): 8237079: gc/g1/mixedgc/TestLogging.java fails with "Pause Young (Mixed) (G1 Evacuation Pause) not found"

[Leo Korinth]

Hi Thomas,

This is not a review. This code is basically the same code as is 
duplicated at least three times in the test code. One of the 
duplications you can blame me for, *sorry*. I believe it should be moved 
to a common library method. I also believe the last fix you did in 
TestG1ParallelPhases.java makes that version look cleaner than what you 
propose here (it does not need the last allocation loop at all).

How about using the TestG1ParallelPhases.java version for all three test 
cases? If not, do the third version in TestOldGenCollectionUsage really 
work???

Thanks,
Leo

On 17/01/2020 15:11, Thomas Schatzl wrote:
> Hi all,
> 
>    can I have reviews for this small test fix to unclutter CI with 
> unnecessary failures?
> 
> So this attempts test checks the GC cycle, and in the logs this fails 
> because for some unknown reason (timing?) we get to-space exhaustion and 
> ultimately a full gc which prevents the expected mixed gc.
> 
> The problem (demonstrated with an even more heap-reduced test) is that 
> with 10m heap, 2 regions are already taken by archive regions, leaving 8 
> regions for allocation. Default policy allows g1 to use 4 regions of 
> eden straight away, meaning that if the right amount of fragmentation 
> occurs, we could expand these 4 eden regions in just a bit more than 4 
> destination regions, causing the evacuation failure.
> 
> The fix is to limit young gen size so that this situation can not occur 
> (verified that max number of regions used is significantly smaller than 
> before visually); I added another small fix to not rely on OOME 
> exception to trigger the mixed gcs we want to check for.
> 
> As I could never locally reproduce the issue with original VM settings, 
> I also added a bit more logging to the runs.
> 
> I would like to push this into 14 to avoid noise there too as it also 
> occurs there.
> 
> CR:
> https://bugs.openjdk.java.net/browse/JDK-8237079
> Webrev:
> http://cr.openjdk.java.net/~tschatzl/8237079/webrev
> Testing:
> 4k passed runs with the new test, local testing
> 
> Thanks,
>    Thomas