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