8235427: Remove unnecessary parameters from G1CollectedHeap::free_region and HeapRegion::hr_clear

Reviewed-by: tschatzl, kbarrett

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

@@ -4080,11 +4080,7 @@ void G1CollectedHeap::record_obj_copy_mem_stats() {
                                                create_g1_evac_summary(&_old_evac_stats));
 }
 
-void G1CollectedHeap::free_region(HeapRegion* hr,
+void G1CollectedHeap::free_region(HeapRegion* hr, FreeRegionList* free_list) {
-                                  FreeRegionList* free_list,
-                                  bool skip_remset,
-                                  bool skip_hot_card_cache,
-                                  bool locked) {
   assert(!hr->is_free(), "the region should not be free");
   assert(!hr->is_empty(), "the region should not be empty");
   assert(_hrm->is_available(hr->hrm_index()), "region should be committed");
@@ -4097,11 +4093,14 @@ void G1CollectedHeap::free_region(HeapRegion* hr,
   // Clear the card counts for this region.
   // Note: we only need to do this if the region is not young
   // (since we don't refine cards in young regions).
-  if (!skip_hot_card_cache && !hr->is_young()) {
+  if (!hr->is_young()) {
     _hot_card_cache->reset_card_counts(hr);
   }
-  hr->hr_clear(skip_remset, true /* clear_space */, locked /* locked */);
+
+  // Reset region metadata to allow reuse.
+  hr->hr_clear(true /* clear_space */);
   _policy->remset_tracker()->update_at_free(hr);
+
   if (free_list != NULL) {
     free_list->add_ordered(hr);
   }
@@ -4112,7 +4111,7 @@ void G1CollectedHeap::free_humongous_region(HeapRegion* hr,
   assert(hr->is_humongous(), "this is only for humongous regions");
   assert(free_list != NULL, "pre-condition");
   hr->clear_humongous();
-  free_region(hr, free_list, false /* skip_remset */, false /* skip_hcc */, true /* locked */);
+  free_region(hr, free_list);
 }
 
 void G1CollectedHeap::remove_from_old_sets(const uint old_regions_removed,
@@ -4259,7 +4258,7 @@ class G1FreeCollectionSetTask : public AbstractGangTask {
       stats()->account_evacuated_region(r);
 
       // Free the region and and its remembered set.
-      _g1h->free_region(r, NULL, false /* skip_remset */, true /* skip_hot_card_cache */, true  /* locked */);
+      _g1h->free_region(r, NULL);
     }
 
     void handle_failed_region(HeapRegion* r) {
@@ -4306,8 +4305,6 @@ class G1FreeCollectionSetTask : public AbstractGangTask {
       if (r->is_young()) {
         assert_in_cset(r);
         r->record_surv_words_in_group(_surviving_young_words[r->young_index_in_cset()]);
-      } else {
-        _g1h->hot_card_cache()->reset_card_counts(r);
       }
 
       if (r->evacuation_failed()) {

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

@@ -658,20 +658,13 @@ public:
   // regions as necessary.
   HeapRegion* alloc_highest_free_region();
 
-  // Frees a non-humongous region by initializing its contents and
+  // Frees a region by resetting its metadata and adding it to the free list
-  // adding it to the free list that's passed as a parameter (this is
+  // passed as a parameter (this is usually a local list which will be appended
-  // usually a local list which will be appended to the master free
+  // to the master free list later or NULL if free list management is handled
-  // list later). The used bytes of freed regions are accumulated in
+  // in another way).
-  // pre_used. If skip_remset is true, the region's RSet will not be freed
+  // Callers must ensure they are the only one calling free on the given region
-  // up. If skip_hot_card_cache is true, the region's hot card cache will not
+  // at the same time.
-  // be freed up. The assumption is that this will be done later.
+  void free_region(HeapRegion* hr, FreeRegionList* free_list);
-  // The locked parameter indicates if the caller has already taken
-  // care of proper synchronization. This may allow some optimizations.
-  void free_region(HeapRegion* hr,
-                   FreeRegionList* free_list,
-                   bool skip_remset,
-                   bool skip_hot_card_cache = false,
-                   bool locked = false);
 
   // It dirties the cards that cover the block so that the post
   // write barrier never queues anything when updating objects on this

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

@@ -1274,7 +1274,7 @@ class G1ReclaimEmptyRegionsTask : public AbstractGangTask {
           _g1h->free_humongous_region(hr, _local_cleanup_list);
         } else {
           _old_regions_removed++;
-          _g1h->free_region(hr, _local_cleanup_list, false /* skip_remset */, false /* skip_hcc */, true /* locked */);
+          _g1h->free_region(hr, _local_cleanup_list);
         }
         hr->clear_cardtable();
         _g1h->concurrent_mark()->clear_statistics_in_region(hr->hrm_index());

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

@@ -123,7 +123,7 @@ void HeapRegion::unlink_from_list() {
   set_containing_set(NULL);
 }
 
-void HeapRegion::hr_clear(bool keep_remset, bool clear_space, bool locked) {
+void HeapRegion::hr_clear(bool clear_space) {
   assert(_humongous_start_region == NULL,
          "we should have already filtered out humongous regions");
   assert(!in_collection_set(),
@@ -135,13 +135,7 @@ void HeapRegion::hr_clear(bool keep_remset, bool clear_space, bool locked) {
   set_free();
   reset_pre_dummy_top();
 
-  if (!keep_remset) {
+  rem_set()->clear_locked();
-    if (locked) {
-      rem_set()->clear_locked();
-    } else {
-      rem_set()->clear();
-    }
-  }
 
   zero_marked_bytes();
 
@@ -286,7 +280,7 @@ void HeapRegion::initialize(bool clear_space, bool mangle_space) {
   set_compaction_top(bottom());
   reset_bot();
 
-  hr_clear(false /*par*/, false /*clear_space*/);
+  hr_clear(false /*clear_space*/);
 }
 
 void HeapRegion::report_region_type_change(G1HeapRegionTraceType::Type to) {

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

@@ -490,11 +490,10 @@ public:
 #endif // ASSERT
 
 
-  // Reset the HeapRegion to default values.
+  // Reset the HeapRegion to default values and clear its remembered set.
-  // If skip_remset is true, do not clear the remembered set.
   // If clear_space is true, clear the HeapRegion's memory.
-  // If locked is true, assume we are the only thread doing this operation.
+  // Callers must ensure this is not called by multiple threads at the same time.
-  void hr_clear(bool skip_remset, bool clear_space, bool locked = false);
+  void hr_clear(bool clear_space);
   // Clear the card table corresponding to this region.
   void clear_cardtable();