RFR(S): 7119908: G1: Cache CSet start region for each worker for subsequent reuse

[Tony Printezis]

Bengt Rutisson wrote:
>
> Hi John,
>
> First a high level question: I think your changes are correct, but 
> this got a bit more complex than I thought it would be when I 
> suggested it in the review of 7112743. Is the extra complexity worth 
> the benefits of only having to do the iterations once instead of twice? 

(Small correction: once instead of three times). The thought had also 
crossed my mind. Maybe on some machines with not great single-threaded 
performance it will be worth it.

> I don't have a clear answer to this myself, I just wanted to bring the 
> question up. Clearly it is more efficient, but it is 3x as much code 
> to do the same thing.
>
> One detail for my understanding: why do we need separate time stamps 
> for each worker?

Each worker independently needs to know whether its starting region is 
valid or not. If John didn't have a timestamp per worker, he'd need to 
reset the table at the start or end of each GC.

Tony

> Do all worker not participate in RSet scanning before they go on to do 
> complete_marking_in_collection_set() ?
>
> Bengt
>
>
> On 2011-12-09 20:18, John Cuthbertson wrote:
>> Hi Everyone,
>>
>> Can I have a couple of volunteers to review the changes for this CR? 
>> The webrev can be found at: 
>> http://cr.openjdk.java.net/~johnc/7119908/webrev.0/
>>
>> Summary:
>> As part of the code review comments for 7112743 (G1: Reduce overhead 
>> of marking closure during evacuation pauses) it was suggested that 
>> instead of recalculating the starting heap region for each worker 
>> thread, we reuse the values calculated during RSet scanning. These 
>> changes address that review comment. In these changes I maintain a 
>> cache that is used and updated by 
>> G1CollectedHeap::start_cset_region_for_worker(). The first time this 
>> routine is called by a worker thread during an evacuation pause 
>> (currently during RSet scanning) the cached value for the worker gets 
>> set; when the routine is called subsequently the region that was 
>> cached for the worker is returned. I employ a simple stamp mechanism 
>> based upon the number of GCs that ensures the validity of the regions 
>> in the cache and makes clearing the cache unnecessary.
>>
>> Testing:
>> GC Test suite with a small marking threshold (10%) with and without 
>> verification; the configuration of SPECjbb used to test 7117423; 
>> Kitchensink.
>>
>> Thanks,
>>
>> JohnC
>>
>