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

Thomas Schatzl thomas.schatzl at oracle.com
Mon Jun 15 09:23:20 UTC 2020


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)

Thanks,
   Thomas



More information about the hotspot-gc-dev mailing list