8244505: G1 pause time ratio calculation does not consider Remark/Cleanup pauses

src/hotspot/share/gc/g1/g1Analytics.cpp

@@ -151,7 +151,7 @@ void G1Analytics::report_alloc_rate_ms(double alloc_rate) {
 
 void G1Analytics::compute_pause_time_ratios(double end_time_sec, double pause_time_ms) {
   double long_interval_ms = (end_time_sec - oldest_known_gc_end_time_sec()) * 1000.0;
-  _long_term_pause_time_ratio = (_recent_gc_times_ms->sum() + pause_time_ms) / long_interval_ms;
+  _long_term_pause_time_ratio = _recent_gc_times_ms->sum() / long_interval_ms;
   _long_term_pause_time_ratio = clamp(_long_term_pause_time_ratio, 0.0, 1.0);
 
   double short_interval_ms = (end_time_sec - most_recent_gc_end_time_sec()) * 1000.0;

src/hotspot/share/gc/g1/g1Policy.cpp

@@ -752,7 +752,7 @@ void G1Policy::record_collection_pause_end(double pause_time_ms) {
 
   PauseKind this_pause = young_gc_pause_kind();
 
-  bool update_stats = !_g1h->evacuation_failed();
+  bool update_stats = should_update_gc_stats();
 
   record_pause(this_pause, start_time_sec, end_time_sec);
 
@@ -1304,17 +1304,22 @@ G1Policy::PauseKind G1Policy::young_gc_pause_kind() const {
   }
 }
 
-void G1Policy::update_gc_pause_time_ratios(PauseKind kind, double start_time_sec, double end_time_sec){
+bool G1Policy::should_update_gc_stats() {
+  return !_g1h->evacuation_failed();
+}
+
+void G1Policy::update_gc_pause_time_ratios(PauseKind kind, double start_time_sec, double end_time_sec) {
 
   double pause_time_sec = end_time_sec - start_time_sec;
   double pause_time_ms = pause_time_sec * 1000.0;
 
   _analytics->compute_pause_time_ratios(end_time_sec, pause_time_ms);
   _analytics->update_recent_gc_times(end_time_sec, pause_time_ms);
+
   if (kind == Cleanup || kind == Remark) {
     _analytics->append_prev_collection_pause_end_ms(pause_time_ms);
   } else {
-    _analytics->set_prev_collection_pause_end_ms(end_time_sec*1000*0);
+    _analytics->set_prev_collection_pause_end_ms(end_time_sec * 1000.0);
   }
 }
 
@@ -1323,10 +1328,11 @@ void G1Policy::record_pause(PauseKind kind, double start, double end) {
   if (kind != FullGC) {
     _mmu_tracker->add_pause(start, end);
   }
-  bool update_stats = !_g1h->evacuation_failed();
-  if (update_stats){
+
+  if (should_update_gc_stats()) {
     update_gc_pause_time_ratios(kind, start, end);
   }
+
   // Manage the mutator time tracking from concurrent start to first mixed gc.
   switch (kind) {
     case FullGC:

src/hotspot/share/gc/g1/g1Policy.hpp

@@ -285,6 +285,13 @@ class G1Policy: public CHeapObj<mtGC> {
   PauseKind young_gc_pause_kind() const;
   // Record the given STW pause with the given start and end times (in s).
   void record_pause(PauseKind kind, double start, double end);
+
+  // Evacuation failures skew the timing too much to be considered for statistics updates.
+  // We make the assumption that these are rare.
+  bool should_update_gc_stats();
+
+  void update_gc_pause_time_ratios(PauseKind kind, double start_sec, double end_sec);
+
   // Indicate that we aborted marking before doing any mixed GCs.
   void abort_time_to_mixed_tracking();
 
@@ -439,8 +446,6 @@ class G1Policy: public CHeapObj<mtGC> {
 
   void update_survivors_policy();
 
-  void update_gc_pause_time_ratios(PauseKind kind, double start_sec, double end_sec);
-
   virtual bool force_upgrade_to_full() {
     return false;
   }