8295267: Improve G1 scan to merge ratio calculation

Reviewed-by: ayang, iwalulya

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

@@ -82,8 +82,8 @@ G1Analytics::G1Analytics(const G1Predictions* predictor) :
     _mixed_rs_length_diff_seq(new TruncatedSeq(TruncatedSeqLength)),
     _concurrent_refine_rate_ms_seq(new TruncatedSeq(TruncatedSeqLength)),
     _dirtied_cards_rate_ms_seq(new TruncatedSeq(TruncatedSeqLength)),
-    _young_card_merge_to_scan_ratio_seq(new TruncatedSeq(TruncatedSeqLength)),
-    _mixed_card_merge_to_scan_ratio_seq(new TruncatedSeq(TruncatedSeqLength)),
+    _young_card_scan_to_merge_ratio_seq(new TruncatedSeq(TruncatedSeqLength)),
+    _mixed_card_scan_to_merge_ratio_seq(new TruncatedSeq(TruncatedSeqLength)),
     _young_cost_per_card_scan_ms_seq(new TruncatedSeq(TruncatedSeqLength)),
     _mixed_cost_per_card_scan_ms_seq(new TruncatedSeq(TruncatedSeqLength)),
     _young_cost_per_card_merge_ms_seq(new TruncatedSeq(TruncatedSeqLength)),
@@ -112,7 +112,7 @@ G1Analytics::G1Analytics(const G1Predictions* predictor) :
   _concurrent_refine_rate_ms_seq->add(1/cost_per_logged_card_ms_defaults[0]);
   // Some applications have very low rates for logging cards.
   _dirtied_cards_rate_ms_seq->add(0.0);
-  _young_card_merge_to_scan_ratio_seq->add(young_card_merge_to_scan_ratio_defaults[index]);
+  _young_card_scan_to_merge_ratio_seq->add(young_card_merge_to_scan_ratio_defaults[index]);
   _young_cost_per_card_scan_ms_seq->add(young_only_cost_per_card_scan_ms_defaults[index]);
 
   _copy_cost_per_byte_ms_seq->add(cost_per_byte_ms_defaults[index]);
@@ -188,11 +188,11 @@ void G1Analytics::report_cost_per_card_merge_ms(double cost_per_card_ms, bool fo
   }
 }
 
-void G1Analytics::report_card_merge_to_scan_ratio(double merge_to_scan_ratio, bool for_young_only_phase) {
-  if  (for_young_only_phase) {
-    _young_card_merge_to_scan_ratio_seq->add(merge_to_scan_ratio);
+void G1Analytics::report_card_scan_to_merge_ratio(double merge_to_scan_ratio, bool for_young_only_phase) {
+  if (for_young_only_phase) {
+    _young_card_scan_to_merge_ratio_seq->add(merge_to_scan_ratio);
   } else {
-    _mixed_card_merge_to_scan_ratio_seq->add(merge_to_scan_ratio);
+    _mixed_card_scan_to_merge_ratio_seq->add(merge_to_scan_ratio);
   }
 }
 
@@ -257,10 +257,10 @@ double G1Analytics::predict_dirtied_cards_rate_ms() const {
 }
 
 size_t G1Analytics::predict_scan_card_num(size_t rs_length, bool for_young_only_phase) const {
-  if  (for_young_only_phase || !enough_samples_available(_mixed_card_merge_to_scan_ratio_seq)) {
-    return (size_t)(rs_length * predict_in_unit_interval(_young_card_merge_to_scan_ratio_seq));
+  if (for_young_only_phase || !enough_samples_available(_mixed_card_scan_to_merge_ratio_seq)) {
+    return (size_t)(rs_length * predict_in_unit_interval(_young_card_scan_to_merge_ratio_seq));
   } else {
-    return (size_t)(rs_length * predict_in_unit_interval(_mixed_card_merge_to_scan_ratio_seq));
+    return (size_t)(rs_length * predict_in_unit_interval(_mixed_card_scan_to_merge_ratio_seq));
   }
 }
 

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

@@ -49,10 +49,10 @@ class G1Analytics: public CHeapObj<mtGC> {
   TruncatedSeq* _mixed_rs_length_diff_seq;
   TruncatedSeq* _concurrent_refine_rate_ms_seq;
   TruncatedSeq* _dirtied_cards_rate_ms_seq;
-  // The ratio between the number of merged cards and actually scanned cards, for
+  // The ratio between the number of scanned cards and actually merged cards, for
   // young-only and mixed gcs.
-  TruncatedSeq* _young_card_merge_to_scan_ratio_seq;
-  TruncatedSeq* _mixed_card_merge_to_scan_ratio_seq;
+  TruncatedSeq* _young_card_scan_to_merge_ratio_seq;
+  TruncatedSeq* _mixed_card_scan_to_merge_ratio_seq;
 
   // The cost to scan a card during young-only and mixed gcs in ms.
   TruncatedSeq* _young_cost_per_card_scan_ms_seq;
@@ -128,7 +128,7 @@ public:
   void report_dirtied_cards_rate_ms(double cards_per_ms);
   void report_cost_per_card_scan_ms(double cost_per_remset_card_ms, bool for_young_only_phase);
   void report_cost_per_card_merge_ms(double cost_per_card_ms, bool for_young_only_phase);
-  void report_card_merge_to_scan_ratio(double cards_per_entry_ratio, bool for_young_only_phase);
+  void report_card_scan_to_merge_ratio(double cards_per_entry_ratio, bool for_young_only_phase);
   void report_rs_length_diff(double rs_length_diff, bool for_young_only_phase);
   void report_cost_per_byte_ms(double cost_per_byte_ms, bool mark_or_rebuild_in_progress);
   void report_young_other_cost_per_region_ms(double other_cost_per_region_ms);

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

@@ -100,9 +100,7 @@ public:
 
   // Change Clean cards in a (large) area on the card table as Dirty, preserving
   // already scanned cards. Assumes that most cards in that area are Clean.
-  // Returns the number of dirtied cards that were not yet dirty. This result may
-  // be inaccurate as it does not perform the dirtying atomically.
-  inline size_t mark_range_dirty(size_t start_card_index, size_t num_cards);
+  inline void mark_range_dirty(size_t start_card_index, size_t num_cards);
 
   // Change the given range of dirty cards to "which". All of these cards must be Dirty.
   inline void change_dirty_cards_to(size_t start_card_index, size_t num_cards, CardValue which);

src/hotspot/share/gc/g1/g1CardTable.inline.hpp

@@ -43,12 +43,10 @@ inline bool G1CardTable::mark_clean_as_dirty(CardValue* card) {
   return false;
 }
 
-inline size_t G1CardTable::mark_range_dirty(size_t start_card_index, size_t num_cards) {
+inline void G1CardTable::mark_range_dirty(size_t start_card_index, size_t num_cards) {
   assert(is_aligned(start_card_index, sizeof(size_t)), "Start card index must be aligned.");
   assert(is_aligned(num_cards, sizeof(size_t)), "Number of cards to change must be evenly divisible.");
 
-  size_t result = 0;
-
   size_t const num_chunks = num_cards / sizeof(size_t);
 
   size_t* cur_word = (size_t*)&_byte_map[start_card_index];
@@ -57,7 +55,6 @@ inline size_t G1CardTable::mark_range_dirty(size_t start_card_index, size_t num_
     size_t value = *cur_word;
     if (value == WordAllClean) {
       *cur_word = WordAllDirty;
-      result += sizeof(value);
     } else if (value == WordAllDirty) {
       // do nothing.
     } else {
@@ -67,15 +64,12 @@ inline size_t G1CardTable::mark_range_dirty(size_t start_card_index, size_t num_
         CardValue value = *cur;
         if (value == clean_card_val()) {
           *cur = dirty_card_val();
-          result++;
         }
         cur++;
       }
     }
     cur_word++;
   }
-
-  return result;
 }
 
 inline void G1CardTable::change_dirty_cards_to(size_t start_card_index, size_t num_cards, CardValue which) {

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

@@ -112,14 +112,14 @@ class G1GCPhaseTimes : public CHeapObj<mtGC> {
     MergeRSHowlArrayOfCards,
     MergeRSHowlBitmap,
     MergeRSHowlFull,
-    MergeRSDirtyCards,
+    MergeRSCards,
     MergeRSContainersSentinel
   };
 
   static constexpr const char* GCMergeRSWorkItemsStrings[MergeRSContainersSentinel] =
     { "Merged Inline", "Merged ArrayOfCards", "Merged Howl", "Merged Full",
       "Merged Howl Inline", "Merged Howl ArrayOfCards", "Merged Howl BitMap", "Merged Howl Full",
-      "Dirty Cards" };
+      "Merged Cards" };
 
   enum GCScanHRWorkItems {
     ScanHRScannedCards,

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

@@ -802,12 +802,14 @@ void G1Policy::record_young_collection_end(bool concurrent_operation_is_full_mar
 
   double merge_hcc_time_ms = average_time_ms(G1GCPhaseTimes::MergeHCC);
   if (update_stats) {
-    size_t const total_log_buffer_cards = p->sum_thread_work_items(G1GCPhaseTimes::MergeHCC, G1GCPhaseTimes::MergeHCCDirtyCards) +
-                                          p->sum_thread_work_items(G1GCPhaseTimes::MergeLB, G1GCPhaseTimes::MergeLBDirtyCards);
-    // Update prediction for card merge; MergeRSDirtyCards includes the cards from the Eager Reclaim phase.
-    size_t const total_cards_merged = p->sum_thread_work_items(G1GCPhaseTimes::MergeRS, G1GCPhaseTimes::MergeRSDirtyCards) +
-                                      p->sum_thread_work_items(G1GCPhaseTimes::OptMergeRS, G1GCPhaseTimes::MergeRSDirtyCards) +
-                                      total_log_buffer_cards;
+    // Update prediction for card merge.
+    size_t const merged_cards_from_log_buffers = p->sum_thread_work_items(G1GCPhaseTimes::MergeHCC, G1GCPhaseTimes::MergeHCCDirtyCards) +
+                                                 p->sum_thread_work_items(G1GCPhaseTimes::MergeLB, G1GCPhaseTimes::MergeLBDirtyCards);
+    // MergeRSCards includes the cards from the Eager Reclaim phase.
+    size_t const merged_cards_from_rs = p->sum_thread_work_items(G1GCPhaseTimes::MergeRS, G1GCPhaseTimes::MergeRSCards) +
+                                        p->sum_thread_work_items(G1GCPhaseTimes::OptMergeRS, G1GCPhaseTimes::MergeRSCards);
+    size_t const total_cards_merged = merged_cards_from_rs +
+                                      merged_cards_from_log_buffers;
 
     if (total_cards_merged >= G1NumCardsCostSampleThreshold) {
       double avg_time_merge_cards = average_time_ms(G1GCPhaseTimes::MergeER) +
@@ -831,16 +833,14 @@ void G1Policy::record_young_collection_end(bool concurrent_operation_is_full_mar
 
     // Update prediction for the ratio between cards from the remembered
     // sets and actually scanned cards from the remembered sets.
-    // Cards from the remembered sets are all cards not duplicated by cards from
-    // the logs.
-    // Due to duplicates in the log buffers, the number of actually scanned cards
+    // Due to duplicates in the log buffers, the number of scanned cards
     // can be smaller than the cards in the log buffers.
-    const size_t from_rs_length_cards = (total_cards_scanned > total_log_buffer_cards) ? total_cards_scanned - total_log_buffer_cards : 0;
-    double merge_to_scan_ratio = 0.0;
-    if (total_cards_scanned > 0) {
-      merge_to_scan_ratio = (double) from_rs_length_cards / total_cards_scanned;
+    const size_t scanned_cards_from_rs = (total_cards_scanned > merged_cards_from_log_buffers) ? total_cards_scanned - merged_cards_from_log_buffers : 0;
+    double scan_to_merge_ratio = 0.0;
+    if (merged_cards_from_rs > 0) {
+      scan_to_merge_ratio = (double)scanned_cards_from_rs / merged_cards_from_rs;
     }
-    _analytics->report_card_merge_to_scan_ratio(merge_to_scan_ratio, is_young_only_pause);
+    _analytics->report_card_scan_to_merge_ratio(scan_to_merge_ratio, is_young_only_pause);
 
     const size_t recorded_rs_length = _collection_set->recorded_rs_length();
     const size_t rs_length_diff = _rs_length > recorded_rs_length ? _rs_length - recorded_rs_length : 0;

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

@@ -1116,8 +1116,8 @@ class G1MergeHeapRootsTask : public WorkerTask {
       _merged[tag]++;
     }
 
-    void inc_cards_dirty(size_t increment = 1) {
-      _merged[G1GCPhaseTimes::MergeRSDirtyCards] += increment;
+    void inc_remset_cards(size_t increment = 1) {
+      _merged[G1GCPhaseTimes::MergeRSCards] += increment;
     }
 
     size_t merged(uint i) const { return _merged[i]; }
@@ -1172,9 +1172,9 @@ class G1MergeHeapRootsTask : public WorkerTask {
 
     void mark_card(G1CardTable::CardValue* value) {
       if (_ct->mark_clean_as_dirty(value)) {
-        _stats.inc_cards_dirty();
         _scan_state->set_chunk_dirty(_ct->index_for_cardvalue(value));
       }
+      _stats.inc_remset_cards();
     }
 
   public:
@@ -1195,7 +1195,7 @@ class G1MergeHeapRootsTask : public WorkerTask {
 
     // Returns whether the given region actually needs iteration.
     bool start_iterate(uint const tag, uint const region_idx) {
-      assert(tag < G1GCPhaseTimes::MergeRSDirtyCards, "invalid tag %u", tag);
+      assert(tag < G1GCPhaseTimes::MergeRSCards, "invalid tag %u", tag);
       if (remember_if_interesting(region_idx)) {
         _region_base_idx = (size_t)region_idx << HeapRegion::LogCardsPerRegion;
         _stats.inc_card_set_merged(tag);
@@ -1205,8 +1205,8 @@ class G1MergeHeapRootsTask : public WorkerTask {
     }
 
     void do_card_range(uint const start_card_idx, uint const length) {
-      size_t num_dirtied = _ct->mark_range_dirty(_region_base_idx + start_card_idx, length);
-      _stats.inc_cards_dirty(num_dirtied);
+      _ct->mark_range_dirty(_region_base_idx + start_card_idx, length);
+      _stats.inc_remset_cards(length);
       _scan_state->set_chunk_range_dirty(_region_base_idx + start_card_idx, length);
     }
 

test/hotspot/jtreg/gc/g1/TestGCLogMessages.java

@@ -129,6 +129,7 @@ public class TestGCLogMessages {
         new LogMessageWithLevel("Hot Card Cache", Level.DEBUG),
         new LogMessageWithLevel("Log Buffers", Level.DEBUG),
         new LogMessageWithLevel("Dirty Cards", Level.DEBUG),
+        new LogMessageWithLevel("Merged Cards", Level.DEBUG),
         new LogMessageWithLevel("Skipped Cards", Level.DEBUG),
         // Evacuate Collection Set
         new LogMessageWithLevel("Ext Root Scanning", Level.DEBUG),