RFR (M): 8231579: G1's incremental calculation of region elapsed time always uses the same age group for prediction

Thomas Schatzl thomas.schatzl at oracle.com
Fri Nov 22 13:00:41 UTC 2019


Hi Stefan,

On 20.11.19 14:43, Stefan Johansson wrote:
> 
> 
> On 2019-11-20 13:49, Thomas Schatzl wrote:
>> Hi Stefan,
>>
>> On 20.11.19 12:24, Stefan Johansson wrote:
>>> Hi Thomas,
>>>
[...]
>>>>
>>>> CR:
>>>> https://bugs.openjdk.java.net/browse/JDK-8231579
>>>> Webrev:
>>>> http://cr.openjdk.java.net/~tschatzl/8231579/webrev/
>>> Looks good, just some variable naming that I think should be updated:
>>> src/hotspot/share/gc/g1/g1CollectionSet.cpp
>>> ---
>>>   248   double old_elapsed_time_ms = hr->predicted_non_copy_time_ms();
>>>   249   double new_region_elapsed_time_ms = 
>>> predict_region_non_copy_time_ms(hr);
>>>   250   double non_copy_time_ms_diff = new_region_elapsed_time_ms - 
>>> old_elapsed_time_ms;
>>>   251   hr->set_predicted_non_copy_time_ms(new_region_elapsed_time_ms);
>>>   252   _inc_predicted_non_copy_time_ms_diff += non_copy_time_ms_diff;
>>>
>>> I think the local variables should change to reflect the new naming, 
>>> something like "old_non_copy_time" and "new_non_copy_time".
>>
>>   you are right. Fixed in
>>
>> http://cr.openjdk.java.net/~tschatzl/8231579/webrev.0_to_1/ (diff)
>> http://cr.openjdk.java.net/~tschatzl/8231579/webrev.1/ (full)
> Looks good.
> 
> Thanks,
> Stefan

   thanks for your review.

Thomas




More information about the hotspot-gc-dev mailing list