RFR (S): 8138750: Clean up survivor rate group

[Tom Benson]

Hi Thomas,
Cleanup looks fine to me, too.  Just one comment you can choose whether 
to ignore:   it seems like the one-line body of 
should_update_surv_rate_group_predictors could just as easily be inline 
when the only use is  "bool update = should_update...... ", now in the 
same module.  Unless you envision other uses.
Tom

On 10/5/2015 10:29 AM, Mikael Gerdin wrote:
> Hi Thomas,
>
> On 2015-10-02 14:45, Thomas Schatzl wrote:
>> Hi all,
>>
>>    can I have reviews for the following cleanup (read deletion) of 
>> unused
>> code and some better naming/moving it to a better place for some method?
>>
>> In particular:
>> - Surv_rate_group::accum_surv_rate() is never called, so remove it and
>> all code referencing _accum_surv_rate.
>> - CollectorState has this method
>>
>>   124   bool should_propagate() const { // XXX should have a more
>> suitable state name or abstraction for this
>>
>> that is only used for updating the survivor rate group predictors in the
>> collector policy. Since it does not have anything to do with G1
>> collector state, I moved it to G1CollectorPolicy, and renamed it to
>> "should_update_surv_rate_group_predictors()" which is what it decides.
>>
>> There is a bug in that method: the current code reads
>>
>>    return _last_young_gc && !_in_marking_code;
>
> This should be !_in_marking_window I suppose.
>
>>
>> which has been introduced in JDK-7097567. It should actually be:
>>
>>    return _last_gc_was_young && !_in_marking_window;
>>
>> I will fix that in JDK-8138752 to keep the cleanup separate from the bug
>> fix unless someone objects.
>
> Splitting the change up sounds good to me.
>
>>
>> CR:
>> https://bugs.openjdk.java.net/browse/JDK-8138750
>> Webrev:
>> http://cr.openjdk.java.net/~tschatzl/8138750/webrev
>
> Looks good.
> /Mikael
>
>> Testing:
>> jprt
>>
>> Thanks,
>>    Thomas
>>
>>
>