RFR: JDK-8148759: G1AllocRegion::_count inconsistently used if more than one context is active (original) (raw)

Bengt Rutisson bengt.rutisson at oracle.com
Fri Mar 18 11:14:41 UTC 2016

Hi Thomas,

Thanks for looking at this!

On 2016-03-18 11:14, Thomas Schatzl wrote:

Hi Bengt,

On Thu, 2016-03-17 at 18:04 +0100, Bengt Rutisson wrote: Hi everyone,

Could I have a couple of reviews for this change? http://cr.openjdk.java.net/~brutisso/8148759/webrev.00 https://bugs.openjdk.java.net/browse/JDK-8148759 The count value is per G1GCAllocRegion, so if we have more than one survivor alloc region we will notice too late that we have exceeded the maximum number of regions we had decided to use for survivors. The proposed patch uses younglist()->survivorlength() instead, which is a "global" value for all alloc regions. This value used to be updated when we retired a region, but to make proper use of it it needs to be updated when we pick a new survivor region. Thus, I had to move the call to younglist()->addsurvivorregion(). I've gone through and checked other users of the values updated by addsurvivorregion() and I haven't found anyone that is using this value inside of the actual evacuation path, which is where the difference would be noticeable. we talked about this offline a bit and I can kind of understand the reasoning of others to not have this check in G1CollectorPolicy. I would prefer to move the check into an extra method like generationisfull(InCSetState dest) that is checked instead of inlining it (so that it can be easily identified later to be moved in some kind of heap sizing/heap shaping or even evacuation policy).

Agreed. Here is an updated webrev: http://cr.openjdk.java.net/~brutisso/8148759/webrev.01/

And a diff compared to the last one: http://cr.openjdk.java.net/~brutisso/8148759/webrev.00-01.diff/

I've checked this version with Thomas, Stefan J and Jesper offline. So, I'll go ahead an push this now.

Thanks for the reviews everyone!

Bengt

Thanks, Thomas

More information about the hotspot-gc-dev mailing list