# HG changeset patch
# User tschatzl
# Date 1575019216 -3600
# Node ID 11ff4e4856708356dab530bd148b26b2fdfacf59
# Parent  9ee940f1de901e966c6daaa510eb9f34cf0e832e
8227434: G1 predictions may over/underflow with high variance input
Summary: Clamp G1 prediction output to sensible values.
Reviewed-by: lkorinth, sjohanss

diff -r 9ee940f1de90 -r 11ff4e485670 src/hotspot/share/gc/g1/g1Analytics.cpp
--- a/src/hotspot/share/gc/g1/g1Analytics.cpp	Fri Nov 29 10:20:14 2019 +0100
+++ b/src/hotspot/share/gc/g1/g1Analytics.cpp	Fri Nov 29 10:20:16 2019 +0100
@@ -125,12 +125,16 @@
   return seq->num() >= 3;
 }
 
-double G1Analytics::get_new_prediction(TruncatedSeq const* seq) const {
-  return _predictor->get_new_prediction(seq);
+double G1Analytics::get_new_unit_prediction(TruncatedSeq const* seq) const {
+  return _predictor->get_new_unit_prediction(seq);
 }
 
 size_t G1Analytics::get_new_size_prediction(TruncatedSeq const* seq) const {
-  return (size_t)get_new_prediction(seq);
+  return (size_t)get_new_lower_zero_bound_prediction(seq);
+}
+
+double G1Analytics::get_new_lower_zero_bound_prediction(TruncatedSeq const* seq) const {
+  return _predictor->get_new_lower_zero_bound_prediction(seq);
 }
 
 int G1Analytics::num_alloc_rate_ms() const {
@@ -225,50 +229,50 @@
 }
 
 double G1Analytics::predict_alloc_rate_ms() const {
-  return get_new_prediction(_alloc_rate_ms_seq);
+  return get_new_lower_zero_bound_prediction(_alloc_rate_ms_seq);
 }
 
 double G1Analytics::predict_concurrent_refine_rate_ms() const {
-  return get_new_prediction(_concurrent_refine_rate_ms_seq);
+  return get_new_lower_zero_bound_prediction(_concurrent_refine_rate_ms_seq);
 }
 
 double G1Analytics::predict_logged_cards_rate_ms() const {
-  return get_new_prediction(_logged_cards_rate_ms_seq);
+  return get_new_lower_zero_bound_prediction(_logged_cards_rate_ms_seq);
 }
 
 double G1Analytics::predict_young_card_merge_to_scan_ratio() const {
-  return get_new_prediction(_young_card_merge_to_scan_ratio_seq);
+  return get_new_unit_prediction(_young_card_merge_to_scan_ratio_seq);
 }
 
 size_t G1Analytics::predict_scan_card_num(size_t rs_length, bool for_young_gc) const {
   if (for_young_gc || !enough_samples_available(_mixed_card_merge_to_scan_ratio_seq)) {
-    return (size_t) (rs_length * predict_young_card_merge_to_scan_ratio());
+    return (size_t)(rs_length * predict_young_card_merge_to_scan_ratio());
   } else {
-    return (size_t) (rs_length * get_new_prediction(_mixed_card_merge_to_scan_ratio_seq));
+    return (size_t)(rs_length * get_new_unit_prediction(_mixed_card_merge_to_scan_ratio_seq));
   }
 }
 
 double G1Analytics::predict_card_merge_time_ms(size_t card_num, bool for_young_gc) const {
   if (for_young_gc || !enough_samples_available(_mixed_cost_per_card_merge_ms_seq)) {
-    return card_num * get_new_prediction(_young_cost_per_card_merge_ms_seq);
+    return card_num * get_new_lower_zero_bound_prediction(_young_cost_per_card_merge_ms_seq);
   } else {
-    return card_num * get_new_prediction(_mixed_cost_per_card_merge_ms_seq);
+    return card_num * get_new_lower_zero_bound_prediction(_mixed_cost_per_card_merge_ms_seq);
   }
 }
 
 double G1Analytics::predict_card_scan_time_ms(size_t card_num, bool for_young_gc) const {
   if (for_young_gc || !enough_samples_available(_mixed_cost_per_card_scan_ms_seq)) {
-    return card_num * get_new_prediction(_young_cost_per_card_scan_ms_seq);
+    return card_num * get_new_lower_zero_bound_prediction(_young_cost_per_card_scan_ms_seq);
   } else {
-    return card_num * get_new_prediction(_mixed_cost_per_card_scan_ms_seq);
+    return card_num * get_new_lower_zero_bound_prediction(_mixed_cost_per_card_scan_ms_seq);
   }
 }
 
 double G1Analytics::predict_object_copy_time_ms_during_cm(size_t bytes_to_copy) const {
   if (!enough_samples_available(_cost_per_byte_ms_during_cm_seq)) {
-    return (1.1 * bytes_to_copy) * get_new_prediction(_copy_cost_per_byte_ms_seq);
+    return (1.1 * bytes_to_copy) * get_new_lower_zero_bound_prediction(_copy_cost_per_byte_ms_seq);
   } else {
-    return bytes_to_copy * get_new_prediction(_cost_per_byte_ms_during_cm_seq);
+    return bytes_to_copy * get_new_lower_zero_bound_prediction(_cost_per_byte_ms_during_cm_seq);
   }
 }
 
@@ -276,32 +280,32 @@
   if (during_concurrent_mark) {
     return predict_object_copy_time_ms_during_cm(bytes_to_copy);
   } else {
-    return bytes_to_copy * get_new_prediction(_copy_cost_per_byte_ms_seq);
+    return bytes_to_copy * get_new_lower_zero_bound_prediction(_copy_cost_per_byte_ms_seq);
   }
 }
 
 double G1Analytics::predict_constant_other_time_ms() const {
-  return get_new_prediction(_constant_other_time_ms_seq);
+  return get_new_lower_zero_bound_prediction(_constant_other_time_ms_seq);
 }
 
 double G1Analytics::predict_young_other_time_ms(size_t young_num) const {
-  return young_num * get_new_prediction(_young_other_cost_per_region_ms_seq);
+  return young_num * get_new_lower_zero_bound_prediction(_young_other_cost_per_region_ms_seq);
 }
 
 double G1Analytics::predict_non_young_other_time_ms(size_t non_young_num) const {
-  return non_young_num * get_new_prediction(_non_young_other_cost_per_region_ms_seq);
+  return non_young_num * get_new_lower_zero_bound_prediction(_non_young_other_cost_per_region_ms_seq);
 }
 
 double G1Analytics::predict_remark_time_ms() const {
-  return get_new_prediction(_concurrent_mark_remark_times_ms);
+  return get_new_lower_zero_bound_prediction(_concurrent_mark_remark_times_ms);
 }
 
 double G1Analytics::predict_cleanup_time_ms() const {
-  return get_new_prediction(_concurrent_mark_cleanup_times_ms);
+  return get_new_lower_zero_bound_prediction(_concurrent_mark_cleanup_times_ms);
 }
 
 size_t G1Analytics::predict_rs_length() const {
-  return get_new_size_prediction(_rs_length_seq) + get_new_prediction(_rs_length_diff_seq);
+  return get_new_size_prediction(_rs_length_seq) + get_new_size_prediction(_rs_length_diff_seq);
 }
 
 size_t G1Analytics::predict_pending_cards() const {
diff -r 9ee940f1de90 -r 11ff4e485670 src/hotspot/share/gc/g1/g1Analytics.hpp
--- a/src/hotspot/share/gc/g1/g1Analytics.hpp	Fri Nov 29 10:20:14 2019 +0100
+++ b/src/hotspot/share/gc/g1/g1Analytics.hpp	Fri Nov 29 10:20:16 2019 +0100
@@ -84,8 +84,9 @@
   // The constant used is random but "small".
   bool enough_samples_available(TruncatedSeq const* seq) const;
 
-  double get_new_prediction(TruncatedSeq const* seq) const;
+  double get_new_unit_prediction(TruncatedSeq const* seq) const;
   size_t get_new_size_prediction(TruncatedSeq const* seq) const;
+  double get_new_lower_zero_bound_prediction(TruncatedSeq const* seq) const;
 
 public:
   G1Analytics(const G1Predictions* predictor);
diff -r 9ee940f1de90 -r 11ff4e485670 src/hotspot/share/gc/g1/g1ConcurrentMark.cpp
--- a/src/hotspot/share/gc/g1/g1ConcurrentMark.cpp	Fri Nov 29 10:20:14 2019 +0100
+++ b/src/hotspot/share/gc/g1/g1ConcurrentMark.cpp	Fri Nov 29 10:20:16 2019 +0100
@@ -2588,7 +2588,8 @@
   // and do_marking_step() is not being called serially.
   bool do_stealing = do_termination && !is_serial;
 
-  double diff_prediction_ms = _g1h->policy()->predictor().get_new_prediction(&_marking_step_diff_ms);
+  G1Predictions const& predictor = _g1h->policy()->predictor();
+  double diff_prediction_ms = predictor.get_new_lower_zero_bound_prediction(&_marking_step_diff_ms);
   _time_target_ms = time_target_ms - diff_prediction_ms;
 
   // set up the variables that are used in the work-based scheme to
diff -r 9ee940f1de90 -r 11ff4e485670 src/hotspot/share/gc/g1/g1IHOPControl.cpp
--- a/src/hotspot/share/gc/g1/g1IHOPControl.cpp	Fri Nov 29 10:20:14 2019 +0100
+++ b/src/hotspot/share/gc/g1/g1IHOPControl.cpp	Fri Nov 29 10:20:16 2019 +0100
@@ -113,6 +113,10 @@
     );
 }
 
+double G1AdaptiveIHOPControl::get_new_prediction(TruncatedSeq const* seq) const {
+  return _predictor->get_new_lower_zero_bound_prediction(seq);
+}
+
 bool G1AdaptiveIHOPControl::have_enough_data_for_prediction() const {
   return ((size_t)_marking_times_s.num() >= G1AdaptiveIHOPNumInitialSamples) &&
          ((size_t)_allocation_rate_s.num() >= G1AdaptiveIHOPNumInitialSamples);
@@ -120,8 +124,8 @@
 
 size_t G1AdaptiveIHOPControl::get_conc_mark_start_threshold() {
   if (have_enough_data_for_prediction()) {
-    double pred_marking_time = _predictor->get_new_prediction(&_marking_times_s);
-    double pred_promotion_rate = _predictor->get_new_prediction(&_allocation_rate_s);
+    double pred_marking_time = get_new_prediction(&_marking_times_s);
+    double pred_promotion_rate = get_new_prediction(&_allocation_rate_s);
     size_t pred_promotion_size = (size_t)(pred_marking_time * pred_promotion_rate);
 
     size_t predicted_needed_bytes_during_marking =
@@ -168,8 +172,8 @@
                       actual_target,
                       G1CollectedHeap::heap()->used(),
                       _last_unrestrained_young_size,
-                      _predictor->get_new_prediction(&_allocation_rate_s),
-                      _predictor->get_new_prediction(&_marking_times_s) * 1000.0,
+                      get_new_prediction(&_allocation_rate_s),
+                      get_new_prediction(&_marking_times_s) * 1000.0,
                       have_enough_data_for_prediction() ? "true" : "false");
 }
 
@@ -179,7 +183,7 @@
                                           actual_target_threshold(),
                                           G1CollectedHeap::heap()->used(),
                                           _last_unrestrained_young_size,
-                                          _predictor->get_new_prediction(&_allocation_rate_s),
-                                          _predictor->get_new_prediction(&_marking_times_s),
+                                          get_new_prediction(&_allocation_rate_s),
+                                          get_new_prediction(&_marking_times_s),
                                           have_enough_data_for_prediction());
 }
diff -r 9ee940f1de90 -r 11ff4e485670 src/hotspot/share/gc/g1/g1IHOPControl.hpp
--- a/src/hotspot/share/gc/g1/g1IHOPControl.hpp	Fri Nov 29 10:20:14 2019 +0100
+++ b/src/hotspot/share/gc/g1/g1IHOPControl.hpp	Fri Nov 29 10:20:16 2019 +0100
@@ -123,6 +123,9 @@
   // as there is no marking or mixed gc that could impact its size too much.
   size_t _last_unrestrained_young_size;
 
+  // Get a new prediction bounded below by zero from the given sequence.
+  double get_new_prediction(TruncatedSeq const* seq) const;
+
   bool have_enough_data_for_prediction() const;
 
   // The "actual" target threshold the algorithm wants to keep during and at the
diff -r 9ee940f1de90 -r 11ff4e485670 src/hotspot/share/gc/g1/g1Policy.cpp
--- a/src/hotspot/share/gc/g1/g1Policy.cpp	Fri Nov 29 10:20:14 2019 +0100
+++ b/src/hotspot/share/gc/g1/g1Policy.cpp	Fri Nov 29 10:20:16 2019 +0100
@@ -914,11 +914,7 @@
 double G1Policy::predict_yg_surv_rate(int age, SurvRateGroup* surv_rate_group) const {
   TruncatedSeq* seq = surv_rate_group->get_seq(age);
   guarantee(seq->num() > 0, "There should be some young gen survivor samples available. Tried to access with age %d", age);
-  double pred = _predictor.get_new_prediction(seq);
-  if (pred > 1.0) {
-    pred = 1.0;
-  }
-  return pred;
+  return _predictor.get_new_unit_prediction(seq);
 }
 
 double G1Policy::accum_yg_surv_rate_pred(int age) const {
diff -r 9ee940f1de90 -r 11ff4e485670 src/hotspot/share/gc/g1/g1Predictions.hpp
--- a/src/hotspot/share/gc/g1/g1Predictions.hpp	Fri Nov 29 10:20:14 2019 +0100
+++ b/src/hotspot/share/gc/g1/g1Predictions.hpp	Fri Nov 29 10:20:16 2019 +0100
@@ -57,6 +57,14 @@
   double get_new_prediction(TruncatedSeq const* seq) const {
     return seq->davg() + _sigma * stddev_estimate(seq);
   }
+
+  double get_new_unit_prediction(TruncatedSeq const* seq) const {
+    return clamp(get_new_prediction(seq), 0.0, 1.0);
+  }
+
+  double get_new_lower_zero_bound_prediction(TruncatedSeq const* seq) const {
+    return MAX2(get_new_prediction(seq), 0.0);
+  }
 };
 
 #endif // SHARE_GC_G1_G1PREDICTIONS_HPP
diff -r 9ee940f1de90 -r 11ff4e485670 src/hotspot/share/gc/g1/survRateGroup.cpp
--- a/src/hotspot/share/gc/g1/survRateGroup.cpp	Fri Nov 29 10:20:14 2019 +0100
+++ b/src/hotspot/share/gc/g1/survRateGroup.cpp	Fri Nov 29 10:20:16 2019 +0100
@@ -116,8 +116,7 @@
   double accum = 0.0;
   double pred = 0.0;
   for (size_t i = 0; i < _stats_arrays_length; ++i) {
-    pred = predictor.get_new_prediction(_surv_rate_pred[i]);
-    if (pred > 1.0) pred = 1.0;
+    pred = predictor.get_new_unit_prediction(_surv_rate_pred[i]);
     accum += pred;
     _accum_surv_rate_pred[i] = accum;
   }
diff -r 9ee940f1de90 -r 11ff4e485670 test/hotspot/gtest/gc/g1/test_g1Predictions.cpp
--- a/test/hotspot/gtest/gc/g1/test_g1Predictions.cpp	Fri Nov 29 10:20:14 2019 +0100
+++ b/test/hotspot/gtest/gc/g1/test_g1Predictions.cpp	Fri Nov 29 10:20:16 2019 +0100
@@ -26,6 +26,8 @@
 #include "gc/g1/g1Predictions.hpp"
 #include "unittest.hpp"
 
+#include "utilities/ostream.hpp"
+
 static const double epsilon = 1e-6;
 
 // Some basic formula tests with confidence = 0.0
@@ -96,3 +98,51 @@
   double p4 = predictor.get_new_prediction(&s);
   ASSERT_GT(p4, p3) << "Fourth prediction must be greater than third";
 }
+
+// Some tests to verify bounding between [0 .. 1]
+TEST_VM(G1Predictions, unit_predictions) {
+  G1Predictions predictor(0.5);
+  TruncatedSeq s;
+
+  double p0 = predictor.get_new_unit_prediction(&s);
+  ASSERT_LT(p0, epsilon) << "Initial prediction of empty sequence must be 0.0";
+
+  s.add(100.0);
+  double p1 = predictor.get_new_unit_prediction(&s);
+  ASSERT_NEAR(p1, 1.0, epsilon);
+
+  // Feed the sequence additional positive values to test the high bound.
+  for (int i = 0; i < 3; i++) {
+    s.add(2.0);
+  }
+  ASSERT_NEAR(predictor.get_new_unit_prediction(&s), 1.0, epsilon);
+
+  // Feed the sequence additional large negative value to test the low bound.
+  for (int i = 0; i < 4; i++) {
+    s.add(-200.0);
+  }
+  ASSERT_NEAR(predictor.get_new_unit_prediction(&s), 0.0, epsilon);
+}
+
+// Some tests to verify bounding between [0 .. +inf]
+TEST_VM(G1Predictions, lower_bound_zero_predictions) {
+  G1Predictions predictor(0.5);
+  TruncatedSeq s;
+
+  double p0 = predictor.get_new_lower_zero_bound_prediction(&s);
+  ASSERT_LT(p0, epsilon) << "Initial prediction of empty sequence must be 0.0";
+
+  s.add(100.0);
+  // Feed the sequence additional positive values to see that the high bound is not
+  // bounded by e.g. 1.0
+  for (int i = 0; i < 3; i++) {
+    s.add(2.0);
+  }
+  ASSERT_GT(predictor.get_new_lower_zero_bound_prediction(&s), 1.0);
+
+  // Feed the sequence additional large negative value to test the low bound.
+  for (int i = 0; i < 4; i++) {
+    s.add(-200.0);
+  }
+  ASSERT_NEAR(predictor.get_new_lower_zero_bound_prediction(&s), 0.0, epsilon);
+}