Fix for JDK-8048556 (GCLocker issues): feedback and some testing please?
Tony Printezis
tprintezis at twitter.com
Tue Sep 9 14:50:51 UTC 2014
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
More information about the hotspot-gc-dev
mailing list