RFR(S): 8007036: G1: Too many old regions added to last mixed GC

[Bengt Rutisson]

Hi John,

(Happened to send an empty mail just before. Here's what I wanted to write.)

Sorry for not reviewing this earlier!

Changes look good, ship it!

One question. You added a note to the _length field in CollectionSetChooser:

   // The number of candidate old regions added to the CSet chooser.
   // Note: this is not updated when removing a region using
   // remove_and_move_to_next() below.
   uint _length;

As far as I can tell we should really decrement the _length field in 
remove_and_move_to_next(). Would it maybe be better to fix this (as a 
separate change of course) than to add the note? I guess the code works 
since we don't use the length information after finlalize_cset() but it 
feels a bit fragile.

Bengt

On 2/26/13 7:28 PM, John Cuthbertson wrote:
> Hi Everyone,
>
> I've only received (and incorporated) comments from Vitaly Davidovich 
> so I'm pinging the list again. Any takers? It's quite small - the main 
> part of the fix is in G1CollectorPolicy::finalize_cset().
>
> Thanks,
>
> JohnC
>
> On 1/29/2013 1:35 PM, John Cuthbertson wrote:
>> Hi Everyone,
>>
>> Here's a new webrev based upon feedback from Vitaly: 
>> http://cr.openjdk.java.net/~johnc/8007036/webrev.1/
>>
>> JohnC
>>
>>
>> On 1/28/2013 1:59 PM, John Cuthbertson wrote:
>>> Hi Everyone,
>>>
>>> Can I have a couple of volunteers look over the changes for this CR? 
>>> The webrev is at: http://cr.openjdk.java.net/~johnc/8007036/webrev.0/
>>>
>>> Summary:
>>> When adding old regions to the collection set we don't take into 
>>> account whether the old regions added so far take us below the 
>>> G1HeapWastePercent. As a result we could end up adding (and 
>>> collecting) many more regions than we needed to. The actual number 
>>> added was the minimum between the number of candidate regions / 
>>> G1MixedGCCountTarget and 10% of the heap.
>>>
>>> Currently the calculation of the reclaimable bytes as a percentage 
>>> of the uses exact arithmetic. It might make sense, at some point in 
>>> the future, to use inexact arithmetic (rounding) in the decision on 
>>> whether to continue mixed GCs and use exact arithmetic when adding 
>>> regions.
>>>
>>> As part of this change I've also moved a couple routines from 
>>> CollectionSetChooser to G1CollectorPolicy. I think they "fit" better 
>>> in G1CollectorPolicy.
>>>
>>> Testing:
>>> GCOld with tenuring threshold = 1 and a marking threshold = 10.
>>>
>>> Many thanks to Monica for identifying the issue.
>>>
>>> Thanks,
>>>
>>> JohnC
>>
>