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

[Stefan Johansson]

Thanks for the review Bengt.

Stefan


On 2014-10-20 13:57, Bengt Rutisson wrote:
>
> 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
>