RFR(M): 8155634: Clean out old logging and dead code from SurvRateGroup

[Jon Masamitsu]

On 5/3/2016 12:58 AM, Mikael Gerdin wrote:
> Hi Jon,
>
> On 2016-05-02 20:58, Jon Masamitsu wrote:
>>
>> On 05/02/2016 05:40 AM, Mikael Gerdin wrote:
>>> Hi,
>>>
>>> Please review this change to clean out unused code from SurvRateGroup.
>>> The G1YoungSurvRateNumRegionsSummary flag is only accessible in
>>> notproduct builds and it is my opinion that the ability to print the
>>> predicted aggregate survivor rates for eden regions at JVM shutdown is
>>> not a particularly useful feature.
>>>
>>> Testing: Local testing with gc test suite.
>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8155634
>>> Webrev: http://cr.openjdk.java.net/~mgerdin/8155634/webrev.0/
>>
>> Mikael,
>>
>> Changes look good.
>
> Thanks.
>
>>
>> I don't have much of an opinion on whether the survivor rate summary is
>> useful but I note that
>>
>> http://cr.openjdk.java.net/~mgerdin/8155634/webrev.0/src/share/vm/gc/g1/g1CollectedHeap.cpp.udiff.html 
>>
>>
>>   void G1CollectedHeap::print_tracing_info() const {
>>     g1_rem_set()->print_summary_info();
>>     concurrent_mark()->print_summary_info();
>> - g1_policy()->print_yg_surv_rate_info();
>>   }
>>
>> the remembered set and concurrent marking summaries are being kept.
>> Can you explain why survivor rate is the exception?  All seems to
>> me averages over such an extended period that they all deserve the
>> same fate.
>
> The reason is simply that I wanted to clean up how SurvRateGroups were 
> used but understanding which parts of the class actually did some work 
> was extremely difficult due to the fact that 50% of the code was only 
> there to facilitate the logging.
>
> In general I think we should remove the other summary infos as well 
> but from my end this is more on an as-needed effort than a focused 
> cleanup of these summary info outputs.

Thanks for satisfying my curiosity.  Sounds reasonable.

>
> I don't think I have the cycles to get rid of the others before FC so 
> I expect that the remaining ones will have to wait for the next release.

Yes, indeed.  FC looms.

Jon

>
> /Mikael
>
>>
>> Thanks.
>>
>> Jon
>>
>>
>>>
>>> /Mikael
>>