RFR (M): 8233588: Clean up SurvRateGroup

[Thomas Schatzl]

Hi,

On 19.11.19 07:19, Kim Barrett wrote:
>> On Nov 12, 2019, at 10:23 AM, Thomas Schatzl <thomas.schatzl at oracle.com> wrote:
>>
>> Hi all,
>>
>>   can I have some reviews for this change that cleans up the SurvRateGroup class. In particular, while working with it I found that it contains two members that are duplicates of others.
>>
>> This removed a few methods, which in turn made some others obsolete.
>>
[...]
> 
> Looks good.
> 
> One pre-existing issue:
> 
> ------------------------------------------------------------------------------
> src/hotspot/share/gc/g1/g1CollectionSet.cpp
>   343     if (r->age_in_surv_rate_group() < 0) {
> 
> [pre-existing]
> 
> A few lines before we checked r->has_surv_rate_group().  If it doesn't
> have one, should we really be checking the age?  Looks like we'd currently
> assert if we don't have one.
> 
> I think just making this an "else if” with the preceeding “has” check fixes it.
> 
> ------------------------------------------------------------------------------
> 

That does not work because the age_in_surv_rate_group() getter will 
already assert with a bogus HeapRegion::_age_index, i.e. what this code 
actually wants to check. I was not sure about just removing the whole 
verification in G1CollectionSet due to the many existing checks (I could 
still do that), but then left it in by introducing a 
has_valid_age_in_surv_rate() bool getter.

So fixed in

http://cr.openjdk.java.net/~tschatzl/8233588/webrev.0_to_1/
http://cr.openjdk.java.net/~tschatzl/8233588/webrev.1/

Passed jtreg gc/g1 testing

Thanks,
   Thomas