RFR (S): 8243672: Short term pause time ratio calculation in G1 off

[stefan.johansson at oracle.com]

Hi Thomas,

On 2020-05-18 10:45, Thomas Schatzl wrote:
> Hi all,
> 
>    can I have reviews for this change that improves our calculation of 
> the short term pause time ratio calculation in G1?
> 
> Previously we calculated the short term pause time ratio by assuming 
> that the current pause occurred X times in the long term pause time 
> tracking interval spanning X garbage collections.
> 
> That, unfortunately, emphasizes the current gc pause, giving completely 
> off results particularly for pause time increases e.g. during mixed gc 
> (see the comment in the CR for a sample calculation), in turn tending to 
> affect heap sizing negatively in that the heap expansion is far higher 
> than expected.
> 
> (There is no real issue at the moment for too low results as we do not 
> use these calculated time ratios to shrink the heap yet).
> 
> Arguably now the short term gc time ratio will be more spiky now with 
> that change, but short term pause time ratio is not the only metric used 
> for heap expansion, single outliers are not relevant anyway, and there 
> are limits on the per-GC sizing too.
> So overall it mostly works out as before with significant improvements 
> in heap sizing behavior together with JDK-8244603 (which this change 
> will be pushed with.)
> 
> CR:
> https://bugs.openjdk.java.net/browse/JDK-8243672
> Webrev:
> http://cr.openjdk.java.net/~tschatzl/8243672/webrev/index.html

Looks good,

Just one thing:
src/hotspot/share/gc/g1/g1Policy.cpp
---
  122   double now = os::elapsedTime();
  123   _analytics->update_recent_gc_times(now, 0.0);

This is added to G1Policy::init() and two of the three things that are 
update by this call has already been set in the G1Analytics constructor. 
What do you think about adding _recent_gc_times_ms->add(0.0) to the 
constructor instead of this call?
---

Thanks,
Stefan

> Testing:
> hs-tier1-5, perf testing with JDK-8244603
> 
> Overall, with both changes on throughput benchmarks there is not much 
> difference than before as startup typically expands the heap similarly 
> to before and shrinking the heap at more places will be added only later.
> There are some significant performance regressions with OOB settings, at 
> significant reduced heap size though. If you set heap sizes manually, 
> there is *no* change.