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
--- a/hotspot/src/share/vm/gc_implementation/g1/g1CollectedHeap.cpp Mon Mar 17 10:12:47 2014 +0100
+++ b/hotspot/src/share/vm/gc_implementation/g1/g1CollectedHeap.cpp Mon Mar 17 10:13:18 2014 +0100
@@ -5939,7 +5939,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");
@@ -5950,7 +5951,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);
}
@@ -6159,7 +6160,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");
@@ -6193,7 +6194,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()) {
--- a/hotspot/src/share/vm/gc_implementation/g1/g1CollectedHeap.hpp Mon Mar 17 10:12:47 2014 +0100
+++ b/hotspot/src/share/vm/gc_implementation/g1/g1CollectedHeap.hpp Mon Mar 17 10:13:18 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
--- a/hotspot/src/share/vm/gc_implementation/g1/g1GCPhaseTimes.cpp Mon Mar 17 10:12:47 2014 +0100
+++ b/hotspot/src/share/vm/gc_implementation/g1/g1GCPhaseTimes.cpp Mon Mar 17 10:13:18 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);
}
--- a/hotspot/src/share/vm/gc_implementation/g1/heapRegion.cpp Mon Mar 17 10:12:47 2014 +0100
+++ b/hotspot/src/share/vm/gc_implementation/g1/heapRegion.cpp Mon Mar 17 10:13:18 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();
--- a/hotspot/src/share/vm/gc_implementation/g1/heapRegion.hpp Mon Mar 17 10:12:47 2014 +0100
+++ b/hotspot/src/share/vm/gc_implementation/g1/heapRegion.hpp Mon Mar 17 10:13:18 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.
--- a/hotspot/src/share/vm/gc_implementation/g1/heapRegionRemSet.cpp Mon Mar 17 10:12:47 2014 +0100
+++ b/hotspot/src/share/vm/gc_implementation/g1/heapRegionRemSet.cpp Mon Mar 17 10:13:18 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;
}
}
--- a/hotspot/test/gc/g1/TestGCLogMessages.java Mon Mar 17 10:12:47 2014 +0100
+++ b/hotspot/test/gc/g1/TestGCLogMessages.java Mon Mar 17 10:13:18 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 {