8287024: G1: Improve the API boundary between HeapRegionRemSet and G1CardSet

Reviewed-by: ayang, iwalulya

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

@@ -32,11 +32,14 @@
 #include "memory/allocation.inline.hpp"
 #include "runtime/atomic.hpp"
 #include "runtime/globals_extension.hpp"
+#include "runtime/java.hpp"
 #include "utilities/bitMap.inline.hpp"
 #include "utilities/concurrentHashTable.inline.hpp"
 #include "utilities/globalDefinitions.hpp"
 
 G1CardSet::ContainerPtr G1CardSet::FullCardSet = (G1CardSet::ContainerPtr)-1;
+uint G1CardSet::_split_card_shift = 0;
+size_t G1CardSet::_split_card_mask = 0;
 
 static uint default_log2_card_regions_per_region() {
   uint log2_card_regions_per_heap_region = 0;
@@ -367,6 +370,30 @@ G1CardSet::~G1CardSet() {
   _mm->flush();
 }
 
+void G1CardSet::initialize(MemRegion reserved) {
+  const uint BitsInUint = sizeof(uint) * BitsPerByte;
+  const uint CardBitsWithinCardRegion = MIN2((uint)HeapRegion::LogCardsPerRegion, G1CardSetContainer::LogCardsPerRegionLimit);
+
+  // Check if the number of cards within a region fits an uint.
+  if (CardBitsWithinCardRegion > BitsInUint) {
+    vm_exit_during_initialization("Can not represent all cards in a card region within uint.");
+  }
+
+  _split_card_shift = CardBitsWithinCardRegion;
+  _split_card_mask = ((size_t)1 << _split_card_shift) - 1;
+
+  // Check if the card region/region within cards combination can cover the heap.
+  const uint HeapSizeBits = log2i_exact(round_up_power_of_2(reserved.byte_size()));
+  if (HeapSizeBits > (BitsInUint + _split_card_shift + G1CardTable::card_shift())) {
+    FormatBuffer<> fmt("Can not represent all cards in the heap with card region/card within region. "
+                       "Heap %zuB (%u bits) Card set only covers %u bits.",
+                       reserved.byte_size(),
+                       HeapSizeBits,
+                       BitsInUint + _split_card_shift + G1CardTable::card_shift());
+    vm_exit_during_initialization(fmt, "Decrease heap size.");
+  }
+}
+
 uint G1CardSet::container_type_to_mem_object_type(uintptr_t type) const {
   assert(type == G1CardSet::ContainerArrayOfCards ||
          type == G1CardSet::ContainerBitMap ||
@@ -707,6 +734,28 @@ G1CardSetHashTableValue* G1CardSet::get_container(uint card_region) {
   return _table->get(card_region);
 }
 
+void G1CardSet::split_card(uintptr_t card, uint& card_region, uint& card_within_region) const {
+  card_region = (uint)(card >> _split_card_shift);
+  card_within_region = (uint)(card & _split_card_mask);
+  assert(card_within_region < _config->max_cards_in_region(), "must be");
+}
+
+G1AddCardResult G1CardSet::add_card(uintptr_t card) {
+  uint card_region;
+  uint card_within_region;
+  split_card(card, card_region, card_within_region);
+
+  return add_card(card_region, card_within_region, true /* increment_total */);
+}
+
+bool G1CardSet::contains_card(uintptr_t card) {
+  uint card_region;
+  uint card_within_region;
+  split_card(card, card_region, card_within_region);
+
+  return contains_card(card_region, card_within_region);
+}
+
 G1AddCardResult G1CardSet::add_card(uint card_region, uint card_in_region, bool increment_total) {
   G1AddCardResult add_result;
   ContainerPtr to_transfer = nullptr;
@@ -785,7 +834,12 @@ bool G1CardSet::contains_card(uint card_region, uint card_in_region) {
   return false;
 }
 
-void G1CardSet::print_info(outputStream* st, uint card_region, uint card_in_region) {
+void G1CardSet::print_info(outputStream* st, uintptr_t card) {
+  uint card_region;
+  uint card_in_region;
+
+  split_card(card, card_region, card_in_region);
+
   G1CardSetHashTableValue* table_entry = get_container(card_region);
   if (table_entry == nullptr) {
     st->print("NULL card set");

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

@@ -26,6 +26,7 @@
 #define SHARE_GC_G1_G1CARDSET_HPP
 
 #include "memory/allocation.hpp"
+#include "memory/memRegion.hpp"
 #include "utilities/concurrentHashTable.hpp"
 
 class G1CardSetAllocOptions;
@@ -185,11 +186,19 @@ public:
 class G1CardSet : public CHeapObj<mtGCCardSet> {
   friend class G1CardSetTest;
   friend class G1CardSetMtTestTask;
+  friend class G1CheckCardClosure;
 
   friend class G1TransferCard;
 
   friend class G1ReleaseCardsets;
 
+  // When splitting addresses into region and card within that region, the logical
+  // shift value to get the region.
+  static uint _split_card_shift;
+  // When splitting addresses into region and card within that region, the mask
+  // to get the offset within the region.
+  static size_t _split_card_mask;
+
   static G1CardSetCoarsenStats _coarsen_stats; // Coarsening statistics since VM start.
   static G1CardSetCoarsenStats _last_coarsen_stats; // Coarsening statistics before last GC.
 public:
@@ -294,6 +303,20 @@ private:
   uint8_t* allocate_mem_object(uintptr_t type);
   void free_mem_object(ContainerPtr container);
 
+  void split_card(uintptr_t card, uint& card_region, uint& card_within_region) const;
+
+  G1AddCardResult add_card(uint card_region, uint card_in_region, bool increment_total = true);
+
+  bool contains_card(uint card_region, uint card_in_region);
+
+  // Testing API
+  class CardClosure {
+  public:
+    virtual void do_card(uint region_idx, uint card_idx) = 0;
+  };
+
+  void iterate_cards(CardClosure& cl);
+
 public:
   G1CardSetConfiguration* config() const { return _config; }
 
@@ -301,13 +324,15 @@ public:
   G1CardSet(G1CardSetConfiguration* config, G1CardSetMemoryManager* mm);
   virtual ~G1CardSet();
 
+  static void initialize(MemRegion reserved);
+
   // Adds the given card to this set, returning an appropriate result.
   // If incremental_count is true and the card has been added, updates the total count.
-  G1AddCardResult add_card(uint card_region, uint card_in_region, bool increment_total = true);
+  G1AddCardResult add_card(uintptr_t card);
 
-  bool contains_card(uint card_region, uint card_in_region);
+  bool contains_card(uintptr_t card);
 
-  void print_info(outputStream* st, uint card_region, uint card_in_region);
+  void print_info(outputStream* st, uintptr_t card);
 
   // Returns whether this remembered set (and all sub-sets) have an occupancy
   // that is less or equal to the given occupancy.
@@ -352,17 +377,10 @@ public:
 
   class ContainerPtrClosure {
   public:
-    virtual void do_containerptr(uint region_idx, size_t num_occupied, ContainerPtr container) = 0;
+    virtual void do_containerptr(uint card_region_idx, size_t num_occupied, ContainerPtr container) = 0;
   };
 
   void iterate_containers(ContainerPtrClosure* cl, bool safepoint = false);
-
-  class CardClosure {
-  public:
-    virtual void do_card(uint region_idx, uint card_idx) = 0;
-  };
-
-  void iterate_cards(CardClosure& cl);
 };
 
 class G1CardSetHashTableValue {

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

@@ -44,36 +44,13 @@
 #include "utilities/growableArray.hpp"
 #include "utilities/powerOfTwo.hpp"
 
-uint HeapRegionRemSet::_split_card_shift = 0;
-size_t HeapRegionRemSet::_split_card_mask = 0;
 HeapWord* HeapRegionRemSet::_heap_base_address = nullptr;
 
 const char* HeapRegionRemSet::_state_strings[] =  {"Untracked", "Updating", "Complete"};
 const char* HeapRegionRemSet::_short_state_strings[] =  {"UNTRA", "UPDAT", "CMPLT"};
 
 void HeapRegionRemSet::initialize(MemRegion reserved) {
-  const uint BitsInUint = sizeof(uint) * BitsPerByte;
-  const uint CardBitsWithinCardRegion = MIN2((uint)HeapRegion::LogCardsPerRegion, G1CardSetContainer::LogCardsPerRegionLimit);
-
-  // Check if the number of cards within a region fits an uint.
-  if (CardBitsWithinCardRegion > BitsInUint) {
-    vm_exit_during_initialization("Can not represent all cards in a card region within uint.");
-  }
-
-  _split_card_shift = CardBitsWithinCardRegion + CardTable::card_shift();
-  _split_card_mask = ((size_t)1 << _split_card_shift) - 1;
-
-  // Check if the card region/region within cards combination can cover the heap.
-  const uint HeapSizeBits = log2i_exact(round_up_power_of_2(reserved.byte_size()));
-  if (HeapSizeBits > (BitsInUint + _split_card_shift)) {
-    FormatBuffer<> fmt("Can not represent all cards in the heap with card region/card within region. "
-                       "Heap %zuB (%u bits) Remembered set covers %u bits.",
-                       reserved.byte_size(),
-                       HeapSizeBits,
-                       BitsInUint + _split_card_shift);
-    vm_exit_during_initialization(fmt, "Decrease heap size.");
-  }
-
+  G1CardSet::initialize(reserved);
   _heap_base_address = reserved.start();
 }
 

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

@@ -51,12 +51,6 @@ class HeapRegionRemSet : public CHeapObj<mtGC> {
 
   HeapRegion* _hr;
 
-  // When splitting addresses into region and card within that region, the logical
-  // shift value to get the region.
-  static uint _split_card_shift;
-  // When splitting addresses into region and card within that region, the mask
-  // to get the offset within the region.
-  static size_t _split_card_mask;
   // Cached value of heap base address.
   static HeapWord* _heap_base_address;
 
@@ -95,6 +89,8 @@ public:
   // Coarsening statistics since VM start.
   static G1CardSetCoarsenStats coarsen_stats() { return G1CardSet::coarsen_stats(); }
 
+  inline uintptr_t to_card(OopOrNarrowOopStar from) const;
+
 private:
   enum RemSetState {
     Untracked,

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

@@ -115,11 +115,9 @@ inline void HeapRegionRemSet::iterate_for_merge(CardOrRangeVisitor& cl) {
   _card_set.iterate_containers(&cl2, true /* at_safepoint */);
 }
 
-void HeapRegionRemSet::split_card(OopOrNarrowOopStar from, uint& card_region, uint& card_within_region) const {
-  size_t offset = pointer_delta(from, _heap_base_address, 1);
-  card_region = (uint)(offset >> _split_card_shift);
-  card_within_region = (uint)((offset & _split_card_mask) >> CardTable::card_shift());
-  assert(card_within_region < G1CardSetContainer::cards_per_region_limit(), "must be");
+
+uintptr_t HeapRegionRemSet::to_card(OopOrNarrowOopStar from) const {
+  return pointer_delta(from, _heap_base_address, 1) >> CardTable::card_shift();
 }
 
 void HeapRegionRemSet::add_reference(OopOrNarrowOopStar from, uint tid) {
@@ -135,30 +133,15 @@ void HeapRegionRemSet::add_reference(OopOrNarrowOopStar from, uint tid) {
    return;
   }
 
-  uint card_region;
-  uint card_within_region;
-
-  split_card(from, card_region, card_within_region);
-
-  _card_set.add_card(card_region, card_within_region);
+  _card_set.add_card(to_card(from));
 }
 
 bool HeapRegionRemSet::contains_reference(OopOrNarrowOopStar from) {
-  uint card_region;
-  uint card_within_region;
-
-  split_card(from, card_region, card_within_region);
-
-  return _card_set.contains_card(card_region, card_within_region);
+  return _card_set.contains_card(to_card(from));
 }
 
 void HeapRegionRemSet::print_info(outputStream* st, OopOrNarrowOopStar from) {
-  uint card_region;
-  uint card_within_region;
-
-  split_card(from, card_region, card_within_region);
-
-  _card_set.print_info(st, card_region, card_within_region);
+  _card_set.print_info(st, to_card(from));
 }
 
 #endif // SHARE_VM_GC_G1_HEAPREGIONREMSET_INLINE_HPP