RFR(S): 7119908: G1: Cache CSet start region for each worker for subsequent reuse
john.cuthbertson at oracle.com
Tue Dec 13 10:31:59 PST 2011
Hi Bengt, Tony
First of all - Bengt, thanks for looking over the changes.
On 12/13/11 08:00, Tony Printezis wrote:
> 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 think the extra complexity is *that* complex - it is after all
just a simple cache with not too much overhead. Keeping it self
contained was the reason why I wanted to do this as a separate RFE.
Part of the motivation for this code, as Tony points out we may see more
benefit on HW with slow single thread performance. Also, as we discussed
earlier, a single threaded walk of the collection set to pre-calculate
the starting heap regions is out as that will penalize all threads -
especially on certain HW.
The other motivation was that we may have more and more parallel
iterations of the collection set in future (for example the removal of
self forwarding pointers) and with each additional iteration we add, the
complexity 'cost' decreases.
>> 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.
As Tony points out - an individual TS per worker is needed to avoid
clearing the array containing the starting regions during evacuation pauses.
>> Do all worker not participate in RSet scanning before they go on to
>> do complete_marking_in_collection_set() ?
Currently yes. But that may not always be the case and expanding the
code to handle differing numbers of threads should be straight forward.
More information about the hotspot-gc-dev