RFR (L): 8244603 and 8238858: Improve young gen sizing

[stefan.johansson at oracle.com]

On 2020-06-15 11:23, Thomas Schatzl wrote:
> Hi Stefan,
> 
> On 11.06.20 09:51, stefan.johansson at oracle.com wrote:
>> Hi Thomas,
>>
>> Sorry for not getting to this sooner.
>>
>> On 2020-05-19 15:37, Thomas Schatzl wrote:
>>> Hi all,
>>>
>>>    can I have reviews for this change that improves young gen sizing 
>>> a lot to prepare for heap shrinking during young gc (JDK-8238687) ;)
> [...]>> Actually, in the future, when shrinking is implemented 
> (JDK-8238687),
>>> these may be more severe (in some benchmarks, actual gc usage is 
>>> still <2%). I will likely try to balance that with decreasing default 
>>> GCTimeRatio value in the future.
>>>
>>> CR:
>>> https://bugs.openjdk.java.net/browse/JDK-8244603
>>> https://bugs.openjdk.java.net/browse/JDK-8238858
>>> Webrev:
>>> http://cr.openjdk.java.net/~tschatzl/8244603/webrev/
>> Very nice change Thomas, really helpful with all comments.
>>
>> As I've mentioned to you offline I think we can re-structure the code 
>> a bit, to separate the updating of young length bounds from the 
>> returning of values. Here's a suggestion on how to do that:
>> http://cr.openjdk.java.net/~sjohanss/8244603/rev-1/
>>
>> src/hotspot/share/gc/g1/g1Analytics.cpp
>> ---
>>   226 double G1Analytics::predict_alloc_rate_ms() const {
>>   227   if (!enough_samples_available(_alloc_rate_ms_seq)) {
>>   228     return predict_zero_bounded(_alloc_rate_ms_seq);
>>   229   } else {
>>   230     return 0.0;
>>   231   }
>>   232 }
>>
>> As discussed, on line 227 the ! should be removed.
>> ---
>>
>> Apart from this I think it is all good. There are a few places in 
>> g1Policy.cpp where local variables could be either merged or skipped, 
>> but I think they add to the overall ease of understanding.
> 
> Applied all your comments.
> 
> New webrev:
> http://cr.openjdk.java.net/~tschatzl/8244603/webrev.0_to_1 (diff)
> http://cr.openjdk.java.net/~tschatzl/8244603/webrev.1 (full)
Looks good,
Stefan

> 
> Thanks,
>    Thomas