Fix for JDK-8048556 (GCLocker issues): feedback and some testing please?

[Tony Printezis]

Mikael,

Inline.

On 9/9/14, 5:19 AM, Mikael Gerdin wrote:
> I've started some testing on this fix (JPRT, GC test suite).
>> Thanks!
> So far my testing looks good.

Thanks. And thanks for the extra testing on this!!! Much appreciated.

> It looks like G1CollectedHeap::collect(GCCause::Cause cause) also reads the
> total_collections count and thus may trigger a superfluous evacuation pause.
> Why didn't you want to pass a total_collections count to G1's variant?

(I should have added a comment for this) I couldn't reproduce the issue 
with G1. It looks as if the issue had been discovered on G1 and 
Cuthbertson fixed it some time ago:

https://bugs.openjdk.java.net/browse/JDK-7143858

This is why I was conservative and left the G1 path unchanged. I had a 
look at G1CollectedHeap::collect(cause) and it wouldn't be trivial to 
move that functionality to collect_for_gc_locker(), mainly because of 
the need to handle GCLockerInvokesConcurrent (where is that helpful 
anyway?!?!?!). See should_do_concurrent_full_gc(cause) which is called 
from collect(cause).

We could decide to restructure the G1 code to make it more similar to 
what the other two GCs do wrt the handling of collect_for_gc_locker(). 
But, could we maybe do that on a separate CR?

Tony

> /Mikael
>
>>>> I didn't really want to change the CollectedHeap API. However, I can't
>>>> work out another way to pass the two GC counts to the VM op without
>>>> changes to the CollectedHeap. I could have expanded the collect() method
>>>> to also accept the count arguments. However, I don't think collect()
>>>> should really be used for the GCLocker GC and I feel that a separate
>>>> method is appropriate here.
>>> I agree that it's reasonable to have a separate collect_for_gc_locker.
>> Definitely less confusing in the long term...
>>
>>>> I also ended up renaming an argument to a c'tor from _cause to cause, as
>>>> the former should only be used for class members. Hope that's OK.
>>> Yep, that's always bothered me.
>> I aim to please. :-)
>>
>> Tony
>>
>>>> Thanks,
>>>>
>>>> Tony

-- 
Tony Printezis | JVM/GC Engineer / VM Team | Twitter

@TonyPrintezis
tprintezis at twitter.com