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

[Mikael Gerdin]

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