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

[John Cuthbertson]

Hi Bengt,

Submitted:

8009395     G1: CollectionSetChooser cleanup

to track the issue that you mentioned.

Thanks again!

JohnC

On 3/4/2013 3:59 AM, Bengt Rutisson wrote:
>
> 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
>>>
>>
>