RFR(S): 7121496: G1: do the per-region evacuation failure handling work in parallel

[John Cuthbertson]

Hi Tony,

Thanks for looking over the code change.

On 12/28/11 04:03, Tony Printezis wrote:
> John,
>
> Thanks for doing this, it looks good. A few comments:
>
> g1CollectedHeap.cpp:
>
> 4102     assert(_g1h->g1_policy()->assertMarkedBytesDataOK(), "Should 
> be!");
>
> Since this will now be executed concurrently with other workers doing 
> the self-forward removal it might pick up inconsistent information 
> (not 100% sure that this will be the case, but I'd like be careful!). 
> Why don't you stop calling it per region and only call it once at the 
> start of remove_self_forwarding_pointers() (it's already called at the 
> end).

Sure. No problem.

> 4192   // Now reset the claim values in the regions in the collection 
> set.
> 4193   ResetClaimValuesClosure reset_cv_cl;
> 4194   collection_set_iterate(&reset_cv_cl);
>
> This is fine but we already have:
>
> reset_heap_region_claim_values()
> check_heap_region_claim_values()
>
> and
>
> check_cset_heap_region_claim_values()
>
> Can you maybe introduce reset_cset_heap_region_claim_values() for 
> consistency?

Again no problem.
>
> BTW, I liked that you declared the update_rset_cl once per task.

Thanks. It seemed the most natural way to do it.

I'll also follow  Igor's suggestion of moving these closures into a new 
.hpp file.

Thanks,

JohnC