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

Ivan Walulya ivan.walulya at oracle.com
Wed Jun 24 21:20:17 UTC 2020


Looks good to me!

//Ivan


> On 15 Jun 2020, at 11:23, Thomas Schatzl <thomas.schatzl at oracle.com> 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)
> 
> Thanks,
>  Thomas



More information about the hotspot-gc-dev mailing list