8362278: G1: Consolidate functions for recording pause start time

Reviewed-by: tschatzl, ayang

Author: Ivan Walulya

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

@@ -1380,7 +1380,7 @@ void G1ConcurrentMark::remark() {
   }
 
   G1Policy* policy = _g1h->policy();
-  policy->record_concurrent_mark_remark_start();
+  policy->record_pause_start_time();
 
   double start = os::elapsedTime();
 
@@ -1516,7 +1516,7 @@ void G1ConcurrentMark::cleanup() {
   }
 
   G1Policy* policy = _g1h->policy();
-  policy->record_concurrent_mark_cleanup_start();
+  policy->record_pause_start_time();
 
   double start = os::elapsedTime();
 

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

@@ -180,7 +180,6 @@ void G1GCPhaseTimes::reset() {
   _cur_post_evacuate_cleanup_2_time_ms = 0.0;
   _cur_resize_heap_time_ms = 0.0;
   _cur_ref_proc_time_ms = 0.0;
-  _cur_collection_start_sec = 0.0;
   _root_region_scan_wait_time_ms = 0.0;
   _external_accounted_time_ms = 0.0;
   _recorded_prepare_heap_roots_time_ms = 0.0;

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

@@ -191,7 +191,6 @@ class G1GCPhaseTimes : public CHeapObj<mtGC> {
   double _cur_resize_heap_time_ms;
   double _cur_ref_proc_time_ms;
 
-  double _cur_collection_start_sec;
   // Not included in _gc_pause_time_ms
   double _root_region_scan_wait_time_ms;
 
@@ -367,10 +366,6 @@ class G1GCPhaseTimes : public CHeapObj<mtGC> {
     _recorded_prepare_for_mutator_time_ms = time_ms;
   }
 
-  void record_cur_collection_start_sec(double time_ms) {
-    _cur_collection_start_sec = time_ms;
-  }
-
   void record_verify_before_time_ms(double time_ms) {
     _cur_verify_before_time_ms = time_ms;
   }
@@ -387,10 +382,6 @@ class G1GCPhaseTimes : public CHeapObj<mtGC> {
     _recorded_prepare_heap_roots_time_ms = recorded_prepare_heap_roots_time_ms;
   }
 
-  double cur_collection_start_sec() {
-    return _cur_collection_start_sec;
-  }
-
   double cur_distribute_log_buffers_time_ms() {
     return _cur_distribute_log_buffers_time_ms;
   }

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

@@ -58,7 +58,7 @@ G1Policy::G1Policy(STWGCTimer* gc_timer) :
   _old_gen_alloc_tracker(),
   _ihop_control(create_ihop_control(&_old_gen_alloc_tracker, &_predictor)),
   _policy_counters(new GCPolicyCounters("GarbageFirst", 1, 2)),
-  _full_collection_start_sec(0.0),
+  _cur_pause_start_sec(0.0),
   _young_list_desired_length(0),
   _young_list_target_length(0),
   _eden_surv_rate_group(new G1SurvRateGroup()),
@@ -74,8 +74,6 @@ G1Policy::G1Policy(STWGCTimer* gc_timer) :
   _g1h(nullptr),
   _phase_times_timer(gc_timer),
   _phase_times(nullptr),
-  _mark_remark_start_sec(0),
-  _mark_cleanup_start_sec(0),
   _tenuring_threshold(MaxTenuringThreshold),
   _max_survivor_regions(0),
   _survivors_age_table(true)
@@ -572,7 +570,7 @@ void G1Policy::revise_young_list_target_length(size_t card_rs_length, size_t cod
 }
 
 void G1Policy::record_full_collection_start() {
-  _full_collection_start_sec = os::elapsedTime();
+  record_pause_start_time();
   // Release the future to-space so that it is available for compaction into.
   collector_state()->set_in_young_only_phase(false);
   collector_state()->set_in_full_gc(true);
@@ -605,7 +603,8 @@ void G1Policy::record_full_collection_end() {
 
   _old_gen_alloc_tracker.reset_after_gc(_g1h->humongous_regions_count() * G1HeapRegion::GrainBytes);
 
-  record_pause(G1GCPauseType::FullGC, _full_collection_start_sec, end_sec);
+  double start_time_sec = cur_pause_start_sec();
+  record_pause(G1GCPauseType::FullGC, start_time_sec, end_sec);
 }
 
 static void log_refinement_stats(const char* kind, const G1ConcurrentRefineStats& stats) {
@@ -650,7 +649,7 @@ void G1Policy::record_concurrent_refinement_stats(size_t pending_cards,
 
   // Record mutator's card logging rate.
   double mut_start_time = _analytics->prev_collection_pause_end_ms();
-  double mut_end_time = phase_times()->cur_collection_start_sec() * MILLIUNITS;
+  double mut_end_time = cur_pause_start_sec() * MILLIUNITS;
   double mut_time = mut_end_time - mut_start_time;
   // Unlike above for conc-refine rate, here we should not require a
   // non-empty sample, since an application could go some time with only
@@ -669,8 +668,13 @@ bool G1Policy::should_retain_evac_failed_region(uint index) const {
   return live_bytes < threshold;
 }
 
-void G1Policy::record_young_collection_start() {
+void G1Policy::record_pause_start_time() {
   Ticks now = Ticks::now();
+  _cur_pause_start_sec = now.seconds();
+}
+
+void G1Policy::record_young_collection_start() {
+  record_pause_start_time();
   // We only need to do this here as the policy will only be applied
   // to the GC we're about to start. so, no point is calculating this
   // every time we calculate / recalculate the target young length.
@@ -681,8 +685,6 @@ void G1Policy::record_young_collection_start() {
          max_survivor_regions(), _g1h->num_used_regions(), _g1h->max_num_regions());
   assert_used_and_recalculate_used_equal(_g1h);
 
-  phase_times()->record_cur_collection_start_sec(now.seconds());
-
   // do that for any other surv rate groups
   _eden_surv_rate_group->stop_adding_regions();
   _survivors_age_table.clear();
@@ -695,19 +697,12 @@ void G1Policy::record_concurrent_mark_init_end() {
   collector_state()->set_in_concurrent_start_gc(false);
 }
 
-void G1Policy::record_concurrent_mark_remark_start() {
-  _mark_remark_start_sec = os::elapsedTime();
-}
-
 void G1Policy::record_concurrent_mark_remark_end() {
   double end_time_sec = os::elapsedTime();
-  double elapsed_time_ms = (end_time_sec - _mark_remark_start_sec)*1000.0;
+  double start_time_sec = cur_pause_start_sec();
+  double elapsed_time_ms = (end_time_sec - start_time_sec) * 1000.0;
   _analytics->report_concurrent_mark_remark_times_ms(elapsed_time_ms);
-  record_pause(G1GCPauseType::Remark, _mark_remark_start_sec, end_time_sec);
-}
-
-void G1Policy::record_concurrent_mark_cleanup_start() {
-  _mark_cleanup_start_sec = os::elapsedTime();
+  record_pause(G1GCPauseType::Remark, start_time_sec, end_time_sec);
 }
 
 G1CollectionSetCandidates* G1Policy::candidates() const {
@@ -795,7 +790,7 @@ double G1Policy::logged_cards_processing_time() const {
 void G1Policy::record_young_collection_end(bool concurrent_operation_is_full_mark, bool allocation_failure) {
   G1GCPhaseTimes* p = phase_times();
 
-  double start_time_sec = phase_times()->cur_collection_start_sec();
+  double start_time_sec = cur_pause_start_sec();
   double end_time_sec = Ticks::now().seconds();
   double pause_time_ms = (end_time_sec - start_time_sec) * 1000.0;
 
@@ -1321,10 +1316,11 @@ void G1Policy::record_concurrent_mark_cleanup_end(bool has_rebuilt_remembered_se
   collector_state()->set_clear_bitmap_in_progress(true);
 
   double end_sec = os::elapsedTime();
-  double elapsed_time_ms = (end_sec - _mark_cleanup_start_sec) * 1000.0;
+  double start_sec = cur_pause_start_sec();
+  double elapsed_time_ms = (end_sec - start_sec) * 1000.0;
   _analytics->report_concurrent_mark_cleanup_times_ms(elapsed_time_ms);
 
-  record_pause(G1GCPauseType::Cleanup, _mark_cleanup_start_sec, end_sec);
+  record_pause(G1GCPauseType::Cleanup, start_sec, end_sec);
 }
 
 void G1Policy::abandon_collection_set_candidates() {

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

@@ -76,7 +76,7 @@ class G1Policy: public CHeapObj<mtGC> {
 
   GCPolicyCounters* _policy_counters;
 
-  double _full_collection_start_sec;
+  double _cur_pause_start_sec;
 
   // Desired young gen length without taking actually available free regions into
   // account.
@@ -134,6 +134,10 @@ class G1Policy: public CHeapObj<mtGC> {
     _card_rs_length = card_rs_length;
   }
 
+  double cur_pause_start_sec() const {
+    return _cur_pause_start_sec;
+  }
+
   double predict_base_time_ms(size_t pending_cards) const;
 
   double predict_base_time_ms(size_t pending_cards, size_t card_rs_length) const;
@@ -196,11 +200,6 @@ class G1Policy: public CHeapObj<mtGC> {
   // Lazily initialized
   mutable G1GCPhaseTimes* _phase_times;
 
-  // This set of variables tracks the collector efficiency, in order to
-  // determine whether we should initiate a new marking.
-  double _mark_remark_start_sec;
-  double _mark_cleanup_start_sec;
-
   // Updates the internal young gen maximum and target and desired lengths.
   // If no parameters are passed, predict pending cards, card set remset length and
   // code root remset length using the prediction model.
@@ -306,6 +305,7 @@ class G1Policy: public CHeapObj<mtGC> {
   bool about_to_start_mixed_phase() const;
 
   // Record the start and end of the actual collection part of the evacuation pause.
+  void record_pause_start_time();
   void record_young_collection_start();
   void record_young_collection_end(bool concurrent_operation_is_full_mark, bool allocation_failure);
 
@@ -316,12 +316,9 @@ class G1Policy: public CHeapObj<mtGC> {
   // Must currently be called while the world is stopped.
   void record_concurrent_mark_init_end();
 
-  // Record start and end of remark.
-  void record_concurrent_mark_remark_start();
   void record_concurrent_mark_remark_end();
 
   // Record start, end, and completion of cleanup.
-  void record_concurrent_mark_cleanup_start();
   void record_concurrent_mark_cleanup_end(bool has_rebuilt_remembered_sets);
 
   bool next_gc_should_be_mixed() const;

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

@@ -815,11 +815,11 @@ void G1YoungCollector::evacuate_next_optional_regions(G1ParScanThreadStateSet* p
 }
 
 void G1YoungCollector::evacuate_optional_collection_set(G1ParScanThreadStateSet* per_thread_states) {
-  const double collection_start_time_ms = phase_times()->cur_collection_start_sec() * 1000.0;
+  const double pause_start_time_ms = policy()->cur_pause_start_sec() * 1000.0;
 
   while (!evacuation_alloc_failed() && collection_set()->num_optional_regions() > 0) {
 
-    double time_used_ms = os::elapsedTime() * 1000.0 - collection_start_time_ms;
+    double time_used_ms = os::elapsedTime() * 1000.0 - pause_start_time_ms;
     double time_left_ms = MaxGCPauseMillis - time_used_ms;
 
     if (time_left_ms < 0 ||