8370807: G1: Improve region attribute table method naming

Reviewed-by: ayang, sjohanss, iwalulya

Author: Thomas Schatzl

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

@@ -3159,9 +3159,9 @@ G1HeapRegion* G1CollectedHeap::new_gc_alloc_region(size_t word_size, G1HeapRegio
       young_regions_cset_group()->add(new_alloc_region);
     } else {
       new_alloc_region->set_old();
+      update_region_attr(new_alloc_region);
     }
     _policy->remset_tracker()->update_at_allocate(new_alloc_region);
-    register_region_with_region_attr(new_alloc_region);
     G1HeapRegionPrinter::alloc(new_alloc_region);
     return new_alloc_region;
   }

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

@@ -645,16 +645,17 @@ class G1CollectedHeap : public CollectedHeap {
                               size_t word_size,
                               bool update_remsets);
 
-  // We register a region with the fast "in collection set" test. We
-  // simply set to true the array slot corresponding to this region.
-  void register_young_region_with_region_attr(G1HeapRegion* r) {
-    _region_attr.set_in_young(r->hrm_index(), r->has_pinned_objects());
-  }
+  // The following methods update the region attribute table, i.e. a compact
+  // representation of per-region information that is regularly accessed
+  // during GC.
+  inline void register_young_region_with_region_attr(G1HeapRegion* r);
   inline void register_new_survivor_region_with_region_attr(G1HeapRegion* r);
-  inline void register_region_with_region_attr(G1HeapRegion* r);
-  inline void register_old_region_with_region_attr(G1HeapRegion* r);
+  inline void register_old_collection_set_region_with_region_attr(G1HeapRegion* r);
   inline void register_optional_region_with_region_attr(G1HeapRegion* r);
 
+  // Updates region state without overwriting the type in the region attribute table.
+  inline void update_region_attr(G1HeapRegion* r);
+
   void clear_region_attr(const G1HeapRegion* hr) {
     _region_attr.clear(hr);
   }

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

@@ -191,18 +191,26 @@ void G1CollectedHeap::register_humongous_candidate_region_with_region_attr(uint
   _region_attr.set_humongous_candidate(index);
 }
 
+void G1CollectedHeap::register_young_region_with_region_attr(G1HeapRegion* r) {
+  assert(!is_in_cset(r), "should not already be registered as in collection set");
+  _region_attr.set_in_young(r->hrm_index(), r->has_pinned_objects());
+}
+
 void G1CollectedHeap::register_new_survivor_region_with_region_attr(G1HeapRegion* r) {
-  _region_attr.set_new_survivor_region(r->hrm_index());
+  assert(!is_in_cset(r), "should not already be registered as in collection set");
+  _region_attr.set_new_survivor_region(r->hrm_index(), r->has_pinned_objects());
 }
 
-void G1CollectedHeap::register_region_with_region_attr(G1HeapRegion* r) {
+void G1CollectedHeap::update_region_attr(G1HeapRegion* r) {
   _region_attr.set_remset_is_tracked(r->hrm_index(), r->rem_set()->is_tracked());
   _region_attr.set_is_pinned(r->hrm_index(), r->has_pinned_objects());
 }
 
-void G1CollectedHeap::register_old_region_with_region_attr(G1HeapRegion* r) {
+void G1CollectedHeap::register_old_collection_set_region_with_region_attr(G1HeapRegion* r) {
+  assert(!is_in_cset(r), "should not already be registered as in collection set");
+  assert(r->is_old(), "must be");
   assert(r->rem_set()->is_complete(), "must be");
-  _region_attr.set_in_old(r->hrm_index(), true);
+  _region_attr.set_in_old(r->hrm_index(), true, r->has_pinned_objects());
   _rem_set->exclude_region_from_scan(r->hrm_index());
 }
 

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

@@ -126,8 +126,7 @@ void G1CollectionSet::add_old_region(G1HeapRegion* hr) {
 
   assert(!hr->rem_set()->has_cset_group(), "Should have already uninstalled group remset");
 
-  assert(!hr->in_collection_set(), "should not already be in the collection set");
-  _g1h->register_old_region_with_region_attr(hr);
+  _g1h->register_old_collection_set_region_with_region_attr(hr);
 
   assert(_regions_cur_length < _regions_max_length, "Collection set now larger than maximum size.");
   _regions[_regions_cur_length++] = hr->hrm_index();
@@ -736,7 +735,7 @@ void G1CollectionSet::abandon_optional_collection_set(G1ParScanThreadStateSet* p
       // Clear collection set marker and make sure that the remembered set information
       // is correct as we still need it later.
       _g1h->clear_region_attr(r);
-      _g1h->register_region_with_region_attr(r);
+      _g1h->update_region_attr(r);
       r->clear_index_in_opt_cset();
     };
 

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

@@ -84,7 +84,6 @@ struct G1HeapRegionAttr {
 
   bool remset_is_tracked() const     { return _remset_is_tracked != 0; }
 
-  void set_new_survivor()              { _type = NewSurvivor; }
   bool is_pinned() const               { return _is_pinned != 0; }
 
   void set_old()                       { _type = Old; }
@@ -132,10 +131,10 @@ class G1HeapRegionAttrBiasedMappedArray : public G1BiasedMappedArray<G1HeapRegio
     set_by_index(index, G1HeapRegionAttr(G1HeapRegionAttr::Optional, remset_is_tracked));
   }
 
-  void set_new_survivor_region(uintptr_t index) {
+  void set_new_survivor_region(uintptr_t index, bool region_is_pinned) {
     assert(get_by_index(index).is_default(),
            "Region attributes at index " INTPTR_FORMAT " should be default but is %s", index, get_by_index(index).get_type_str());
-    get_ref_by_index(index)->set_new_survivor();
+    set_by_index(index, G1HeapRegionAttr(G1HeapRegionAttr::NewSurvivor, true, region_is_pinned));
   }
 
   void set_humongous_candidate(uintptr_t index) {
@@ -170,12 +169,10 @@ class G1HeapRegionAttrBiasedMappedArray : public G1BiasedMappedArray<G1HeapRegio
     set_by_index(index, G1HeapRegionAttr(G1HeapRegionAttr::Young, true, is_pinned));
   }
 
-  void set_in_old(uintptr_t index, bool remset_is_tracked) {
+  void set_in_old(uintptr_t index, bool remset_is_tracked, bool is_pinned) {
     assert(get_by_index(index).is_default(),
            "Region attributes at index " INTPTR_FORMAT " should be default but is %s", index, get_by_index(index).get_type_str());
-    // We do not select regions with pinned objects into the collection set.
-    const bool region_is_pinned = false;
-    set_by_index(index, G1HeapRegionAttr(G1HeapRegionAttr::Old, remset_is_tracked, region_is_pinned));
+    set_by_index(index, G1HeapRegionAttr(G1HeapRegionAttr::Old, remset_is_tracked, is_pinned));
   }
 
   bool is_in_cset_or_humongous_candidate(HeapWord* addr) const { return at(addr).is_in_cset_or_humongous_candidate(); }

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

@@ -426,7 +426,7 @@ class G1PrepareEvacuationTask : public WorkerTask {
 
       // Now check if region is a humongous candidate
       if (!hr->is_starts_humongous()) {
-        _g1h->register_region_with_region_attr(hr);
+        _g1h->update_region_attr(hr);
         return false;
       }
 
@@ -436,7 +436,7 @@ class G1PrepareEvacuationTask : public WorkerTask {
         _worker_humongous_candidates++;
         // We will later handle the remembered sets of these regions.
       } else {
-        _g1h->register_region_with_region_attr(hr);
+        _g1h->update_region_attr(hr);
       }
       log_debug(gc, humongous)("Humongous region %u (object size %zu @ " PTR_FORMAT ") remset %zu code roots %zu "
                                "marked %d pinned count %zu reclaim candidate %d type %s",