RFR (XS): 8233792: TestG1ParallelPhases.java fails with phase NonYoungFreeCSet not found (2)

[Leo Korinth]

On 11/11/2019 16:44, Thomas Schatzl wrote:
> Hi all,
> 
>    can I have reviews for this small test fix to avoid this test failing 
> sometimes?
> 
> The tests tries to force mixed gcs to wait for a particular JFR event 
> only sent at that time.
> The way it forces those is a bit wrong: in some cases it may happen that 
> at the time it starts a concurrent marking, another one just finished 
> without being able to clean out the old gen. This means that the initial 
> mark for this forced marking will be upgraded to a full gc (which also 
> ends that marking), and the following forced young collections are 
> young-only gcs only.
> 
> In total, no mixed gcs happen in that case, and so that JFR event is 
> never sent.
> 
> The fix is to make sure that before forcing mixed gc (which works) we 
> force the heap into a state where the upgrade to full gc may not happen 
> - by forcing a full gc.
> 
> Without the patch the fails like 6 times in 4000 runs, with the change 
> it does not fail after 3k runs.

Looks good, thanks for finding and fixing this Thomas!

After looking at your fix, I also took a look at TestLogging.java and 
TestOldGenCollectionUsage.java (from which I copied the code). Those 
test cases does, in addition to this test case, also allocate in a loop 
at the end and provokes a mixed gc by also setting 
-XX:G1MixedGCLiveThresholdPercent=100. Even if those tests do "work", I 
think those test cases ought to be cleaned up so that they do not fool 
more people like me. If you think that is a good idea, I will create an 
enhancement for cleanup of those test cases.

Thanks,
Leo

> CR:
> https://bugs.openjdk.java.net/browse/JDK-8233792
> Webrev:
> http://cr.openjdk.java.net/~tschatzl/8233792/webrev/
> Testing:
> see above.
> 
> Thanks,
>    Thomas