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

[Jon Masamitsu]

Tony,

I've also run some GC locker type testing but (my bad) have
not analyzed the results fully yet. Nothing looks amiss with
your patch but, as I said, not done yet.  Also, my apologies, but
I won't be able to put more time on it for a bit.

Jon

On 9/9/2014 2:19 AM, Mikael Gerdin wrote:
> Tony,
>
> On Tuesday 02 September 2014 17.14.57 Tony Printezis wrote:
>> Mikael,
>>
>> Inline.
>>
>> On 9/1/14, 10:38 AM, Mikael Gerdin wrote:
>>> Hi Tony,
>>>
>>> On 2014-08-28 04:06, Tony Printezis wrote:
>>>> Hi all,
>>>>
>>>> I have a fix for the GCLocker causing unnecessary young GCs issue
>>>> (JDK-8048556). Here's the webrev:
>>>>
>>>> http://cr.openjdk.java.net/~tonyp/8048556/webrev.0/
>>>>
>>>> Any chance of a) getting some feedback on whether the fix is reasonable
>>>> and b) getting it through some testing (I did as much testing as I could
>>>> but you know how fragile the GCLocker is...)?
>>> I've started some testing on this fix (JPRT, GC test suite).
>> Thanks!
> So far my testing looks good.
>
> 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?
>
> /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