8027295: Free CSet takes ~50% of young pause time

Improve fast card cache iteration and avoid taking locks when freeing the collection set.

Reviewed-by: brutisso

hotspot/src/share/vm/gc_implementation/g1/g1CollectedHeap.cpp

@@ -5939,7 +5939,8 @@ void G1CollectedHeap::evacuate_collection_set(EvacuationInfo& evacuation_info) {
 
 void G1CollectedHeap::free_region(HeapRegion* hr,
                                   FreeRegionList* free_list,
-                                  bool par) {
+                                  bool par,
+                                  bool locked) {
   assert(!hr->isHumongous(), "this is only for non-humongous regions");
   assert(!hr->is_empty(), "the region should not be empty");
   assert(free_list != NULL, "pre-condition");
@@ -5950,7 +5951,7 @@ void G1CollectedHeap::free_region(HeapRegion* hr,
   if (!hr->is_young()) {
     _cg1r->hot_card_cache()->reset_card_counts(hr);
   }
-  hr->hr_clear(par, true /* clear_space */);
+  hr->hr_clear(par, true /* clear_space */, locked /* locked */);
   free_list->add_as_head(hr);
 }
 
@@ -6159,7 +6160,7 @@ void G1CollectedHeap::free_collection_set(HeapRegion* cs_head, EvacuationInfo& e
       }
     }
 
-    rs_lengths += cur->rem_set()->occupied();
+    rs_lengths += cur->rem_set()->occupied_locked();
 
     HeapRegion* next = cur->next_in_collection_set();
     assert(cur->in_collection_set(), "bad CS");
@@ -6193,7 +6194,7 @@ void G1CollectedHeap::free_collection_set(HeapRegion* cs_head, EvacuationInfo& e
       // And the region is empty.
       assert(!used_mr.is_empty(), "Should not have empty regions in a CS.");
       pre_used += cur->used();
-      free_region(cur, &local_free_list, false /* par */);
+      free_region(cur, &local_free_list, false /* par */, true /* locked */);
     } else {
       cur->uninstall_surv_rate_group();
       if (cur->is_young()) {

hotspot/src/share/vm/gc_implementation/g1/g1CollectedHeap.hpp

@@ -763,9 +763,12 @@ public:
   // list later). The used bytes of freed regions are accumulated in
   // pre_used. If par is true, the region's RSet will not be freed
   // up. The assumption is that this will be done later.
+  // 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 par);
+                   bool par,
+                   bool locked = false);
 
   // Frees a humongous region by collapsing it into individual regions
   // and calling free_region() for each of them. The freed regions

hotspot/src/share/vm/gc_implementation/g1/g1GCPhaseTimes.cpp

@@ -317,6 +317,10 @@ void G1GCPhaseTimes::print(double pause_time_sec) {
   print_stats(2, "Free CSet",
     (_recorded_young_free_cset_time_ms +
     _recorded_non_young_free_cset_time_ms));
+  if (G1Log::finest()) {
+    print_stats(3, "Young Free CSet", _recorded_young_free_cset_time_ms);
+    print_stats(3, "Non-Young Free CSet", _recorded_non_young_free_cset_time_ms);
+  }
   if (_cur_verify_after_time_ms > 0.0) {
     print_stats(2, "Verify After", _cur_verify_after_time_ms);
   }

hotspot/src/share/vm/gc_implementation/g1/heapRegion.cpp

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2001, 2013, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2001, 2014, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -205,7 +205,7 @@ void HeapRegion::reset_after_compaction() {
   init_top_at_mark_start();
 }
 
-void HeapRegion::hr_clear(bool par, bool clear_space) {
+void HeapRegion::hr_clear(bool par, bool clear_space, bool locked) {
   assert(_humongous_type == NotHumongous,
          "we should have already filtered out humongous regions");
   assert(_humongous_start_region == NULL,
@@ -223,7 +223,11 @@ void HeapRegion::hr_clear(bool par, bool clear_space) {
   if (!par) {
     // If this is parallel, this will be done later.
     HeapRegionRemSet* hrrs = rem_set();
+    if (locked) {
+      hrrs->clear_locked();
+    } else {
       hrrs->clear();
+    }
     _claimed = InitialClaimValue;
   }
   zero_marked_bytes();

hotspot/src/share/vm/gc_implementation/g1/heapRegion.hpp

@@ -596,7 +596,7 @@ class HeapRegion: public G1OffsetTableContigSpace {
   void save_marks();
 
   // Reset HR stuff to default values.
-  void hr_clear(bool par, bool clear_space);
+  void hr_clear(bool par, bool clear_space, bool locked = false);
   void par_clear();
 
   // Get the start of the unmarked area in this region.

hotspot/src/share/vm/gc_implementation/g1/heapRegionRemSet.cpp

@@ -731,7 +731,8 @@ size_t OtherRegionsTable::fl_mem_size() {
 
 void OtherRegionsTable::clear_fcc() {
   uint hrs_idx = hr()->hrs_index();
-  for (uint i = 0; i < HeapRegionRemSet::num_par_rem_sets(); i++) {
+  uint num_par_remsets = HeapRegionRemSet::num_par_rem_sets();
+  for (uint i = 0; i < num_par_remsets; i++) {
     _from_card_cache[i][hrs_idx] = -1;
   }
 }

hotspot/test/gc/g1/TestGCLogMessages.java

@@ -23,7 +23,7 @@
 
 /*
  * @test TestPrintGCDetails
- * @bug 8035406
+ * @bug 8035406 8027295
  * @summary Ensure that the PrintGCDetails output for a minor GC with G1
  * includes the expected necessary messages.
  * @key gc
@@ -38,13 +38,41 @@ public class TestGCLogMessages {
 
     ProcessBuilder pb = ProcessTools.createJavaProcessBuilder("-XX:+UseG1GC",
                                                               "-Xmx10M",
-                                                              "-XX:+PrintGCDetails",
                                                               GCTest.class.getName());
 
     OutputAnalyzer output = new OutputAnalyzer(pb.start());
 
-    output.shouldContain("[Code Root Purge");
+    output.shouldNotContain("[Code Root Purge");
+    output.shouldNotContain("[Young Free CSet");
+    output.shouldNotContain("[Non-Young Free CSet");
     output.shouldHaveExitValue(0);
+
+    pb = ProcessTools.createJavaProcessBuilder("-XX:+UseG1GC",
+                                               "-Xmx10M",
+                                               "-XX:+PrintGCDetails",
+                                               GCTest.class.getName());
+
+    output = new OutputAnalyzer(pb.start());
+
+    output.shouldContain("[Code Root Purge");
+    output.shouldNotContain("[Young Free CSet");
+    output.shouldNotContain("[Non-Young Free CSet");
+    output.shouldHaveExitValue(0);
+
+    pb = ProcessTools.createJavaProcessBuilder("-XX:+UseG1GC",
+                                               "-Xmx10M",
+                                               "-XX:+PrintGCDetails",
+                                               "-XX:+UnlockExperimentalVMOptions",
+                                               "-XX:G1LogLevel=finest",
+                                               GCTest.class.getName());
+
+    output = new OutputAnalyzer(pb.start());
+
+    output.shouldContain("[Code Root Purge");
+    output.shouldContain("[Young Free CSet");
+    output.shouldContain("[Non-Young Free CSet");
+    output.shouldHaveExitValue(0);
+
   }
 
   static class GCTest {