# HG changeset patch
# User tschatzl
# Date 1395671436 -3600
# Node ID 96b1c2e06e25a05500e43af9cdae7af56448cc01
# Parent  d7070f3717703de7e442cc33ace220748c0ba003
8027295: Free CSet takes ~50% of young pause time
Summary: Improve fast card cache iteration and avoid taking locks when freeing the collection set.
Reviewed-by: brutisso

diff -r d7070f371770 -r 96b1c2e06e25 src/share/vm/gc_implementation/g1/g1CollectedHeap.cpp
--- a/src/share/vm/gc_implementation/g1/g1CollectedHeap.cpp	Mon Mar 24 15:30:30 2014 +0100
+++ b/src/share/vm/gc_implementation/g1/g1CollectedHeap.cpp	Mon Mar 24 15:30:36 2014 +0100
@@ -5975,7 +5975,8 @@
 
 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");
@@ -5986,7 +5987,7 @@
   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);
 }
 
@@ -6195,7 +6196,7 @@
       }
     }
 
-    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");
@@ -6229,7 +6230,7 @@
       // 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()) {
diff -r d7070f371770 -r 96b1c2e06e25 src/share/vm/gc_implementation/g1/g1CollectedHeap.hpp
--- a/src/share/vm/gc_implementation/g1/g1CollectedHeap.hpp	Mon Mar 24 15:30:30 2014 +0100
+++ b/src/share/vm/gc_implementation/g1/g1CollectedHeap.hpp	Mon Mar 24 15:30:36 2014 +0100
@@ -763,9 +763,12 @@
   // 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
diff -r d7070f371770 -r 96b1c2e06e25 src/share/vm/gc_implementation/g1/g1GCPhaseTimes.cpp
--- a/src/share/vm/gc_implementation/g1/g1GCPhaseTimes.cpp	Mon Mar 24 15:30:30 2014 +0100
+++ b/src/share/vm/gc_implementation/g1/g1GCPhaseTimes.cpp	Mon Mar 24 15:30:36 2014 +0100
@@ -317,6 +317,10 @@
   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);
   }
diff -r d7070f371770 -r 96b1c2e06e25 src/share/vm/gc_implementation/g1/heapRegion.cpp
--- a/src/share/vm/gc_implementation/g1/heapRegion.cpp	Mon Mar 24 15:30:30 2014 +0100
+++ b/src/share/vm/gc_implementation/g1/heapRegion.cpp	Mon Mar 24 15:30:36 2014 +0100
@@ -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 @@
   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 @@
   if (!par) {
     // If this is parallel, this will be done later.
     HeapRegionRemSet* hrrs = rem_set();
-    hrrs->clear();
+    if (locked) {
+      hrrs->clear_locked();
+    } else {
+      hrrs->clear();
+    }
     _claimed = InitialClaimValue;
   }
   zero_marked_bytes();
diff -r d7070f371770 -r 96b1c2e06e25 src/share/vm/gc_implementation/g1/heapRegion.hpp
--- a/src/share/vm/gc_implementation/g1/heapRegion.hpp	Mon Mar 24 15:30:30 2014 +0100
+++ b/src/share/vm/gc_implementation/g1/heapRegion.hpp	Mon Mar 24 15:30:36 2014 +0100
@@ -596,7 +596,7 @@
   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.
diff -r d7070f371770 -r 96b1c2e06e25 src/share/vm/gc_implementation/g1/heapRegionRemSet.cpp
--- a/src/share/vm/gc_implementation/g1/heapRegionRemSet.cpp	Mon Mar 24 15:30:30 2014 +0100
+++ b/src/share/vm/gc_implementation/g1/heapRegionRemSet.cpp	Mon Mar 24 15:30:36 2014 +0100
@@ -731,7 +731,8 @@
 
 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;
   }
 }
diff -r d7070f371770 -r 96b1c2e06e25 test/gc/g1/TestGCLogMessages.java
--- a/test/gc/g1/TestGCLogMessages.java	Mon Mar 24 15:30:30 2014 +0100
+++ b/test/gc/g1/TestGCLogMessages.java	Mon Mar 24 15:30:36 2014 +0100
@@ -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 @@
 
     ProcessBuilder pb = ProcessTools.createJavaProcessBuilder("-XX:+UseG1GC",
                                                               "-Xmx10M",
-                                                              "-XX:+PrintGCDetails",
                                                               GCTest.class.getName());
 
     OutputAnalyzer output = new OutputAnalyzer(pb.start());
 
+    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 {