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

[Bengt Rutisson]

Hi John,

On 3/4/13 7:11 PM, John Cuthbertson wrote:
> Hi Bengt,
>
> Submitted:
>
> 8009395     G1: CollectionSetChooser cleanup
>
> to track the issue that you mentioned.

Great! Thanks!

Thanks for adding the comment about using RegionLists to the bug report. 
I agree that this is the way to go.

Bengt
>
> 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
>>>>
>>>
>>
>