RFR: 8058568: GC cleanup phase can cause G1 skipping a System.gc()

[Bengt Rutisson]

Hi Stefan,

On 2014-10-20 13:08, Stefan Johansson wrote:
> Hi,
>
> Please review this fix for:
> https://bugs.openjdk.java.net/browse/JDK-8058568
>
> Webrev:
> http://cr.openjdk.java.net/~sjohanss/8058568/hotspot.00/

Looks good.

(Stefan and I discussed some refactoring that could be done to get rid 
of the default values of the VM_GC_Operation constructor. It seems like 
the default values make this code vulnerable to this type of mistakes. 
We discussed subclassing VM_GC_Operation to something like 
VM_Full_GC_Operation but this will require the skip_operation() method 
to be split up too. All in all it seemed like a too large change for the 
current problem. So, I prefer Stefan's small change for now.)

Bengt

>
> Summary:
> For G1, full GCs might be skipped if a young GC is executed after the 
> full GC is initiate but before it has been started. The cause for this 
> is that G1 Full GCs aren't marked as full collections when creating 
> the VM_GC_Operation. Changing this to actually mark it as a full 
> collection will cause it to do a second check in skip_operation() to 
> make sure it really should skip it. This check checks if the number of 
> full collections has changed since the operation was scheduled, and if 
> not continues.
>
> There was a second problem with this as well, the full gc before count 
> we passed in to the GC operation was not the correct one, we 
> previously passed in _old_marking_cycles_started which is not the same 
> as number of full collections. Changing this to instead pass in the 
> total_full_collections() will make the above check work as expected.
>
> Testing:
> * Verified that a JTREG test that previously failed now passes
> * Adhoc aurora run didn't show any regression
>
> Stefan