8205426: Humongous continues remembered set does not match humongous start region one after Remark
Summary: Remembered set states for humongous objects crossing an internal per-thread processing threshold could synchronized if the humongous continues regions were processed first.
Reviewed-by: ehelin, kbarrett
--- a/src/hotspot/share/gc/g1/g1ConcurrentMark.cpp Mon Jul 09 12:20:56 2018 +0800
+++ b/src/hotspot/share/gc/g1/g1ConcurrentMark.cpp Mon Jul 09 10:19:51 2018 +0200
@@ -1024,11 +1024,17 @@
uint _num_regions_selected_for_rebuild; // The number of regions actually selected for rebuild.
- void update_remset_before_rebuild(HeapRegion * hr) {
+ void update_remset_before_rebuild(HeapRegion* hr) {
G1RemSetTrackingPolicy* tracking_policy = _g1h->g1_policy()->remset_tracker();
- size_t const live_bytes = _cm->liveness(hr->hrm_index()) * HeapWordSize;
- bool selected_for_rebuild = tracking_policy->update_before_rebuild(hr, live_bytes);
+ bool selected_for_rebuild;
+ if (hr->is_humongous()) {
+ bool const is_live = _cm->liveness(hr->humongous_start_region()->hrm_index()) > 0;
+ selected_for_rebuild = tracking_policy->update_humongous_before_rebuild(hr, is_live);
+ } else {
+ size_t const live_bytes = _cm->liveness(hr->hrm_index());
+ selected_for_rebuild = tracking_policy->update_before_rebuild(hr, live_bytes);
+ }
if (selected_for_rebuild) {
_num_regions_selected_for_rebuild++;
}
--- a/src/hotspot/share/gc/g1/g1RemSetTrackingPolicy.cpp Mon Jul 09 12:20:56 2018 +0800
+++ b/src/hotspot/share/gc/g1/g1RemSetTrackingPolicy.cpp Mon Jul 09 10:19:51 2018 +0200
@@ -29,10 +29,6 @@
#include "gc/g1/heapRegionRemSet.hpp"
#include "runtime/safepoint.hpp"
-bool G1RemSetTrackingPolicy::is_interesting_humongous_region(HeapRegion* r) const {
- return r->is_humongous() && oop(r->humongous_start_region()->bottom())->is_typeArray();
-}
-
bool G1RemSetTrackingPolicy::needs_scan_for_rebuild(HeapRegion* r) const {
// All non-free, non-young, non-closed archive regions need to be scanned for references;
// At every gc we gather references to other regions in young, and closed archive
@@ -64,51 +60,81 @@
/* nothing to do */
}
+static void print_before_rebuild(HeapRegion* r, bool selected_for_rebuild, size_t total_live_bytes, size_t live_bytes) {
+ log_trace(gc, remset, tracking)("Before rebuild region %u "
+ "(ntams: " PTR_FORMAT ") "
+ "total_live_bytes " SIZE_FORMAT " "
+ "selected %s "
+ "(live_bytes " SIZE_FORMAT " "
+ "next_marked " SIZE_FORMAT " "
+ "marked " SIZE_FORMAT " "
+ "type %s)",
+ r->hrm_index(),
+ p2i(r->next_top_at_mark_start()),
+ total_live_bytes,
+ BOOL_TO_STR(selected_for_rebuild),
+ live_bytes,
+ r->next_marked_bytes(),
+ r->marked_bytes(),
+ r->get_type_str());
+}
+
+bool G1RemSetTrackingPolicy::update_humongous_before_rebuild(HeapRegion* r, bool is_live) {
+ assert(SafepointSynchronize::is_at_safepoint(), "should be at safepoint");
+ assert(r->is_humongous(), "Region %u should be humongous", r->hrm_index());
+
+ if (r->is_archive()) {
+ return false;
+ }
+
+ assert(!r->rem_set()->is_updating(), "Remembered set of region %u is updating before rebuild", r->hrm_index());
+
+ bool selected_for_rebuild = false;
+ // For humongous regions, to be of interest for rebuilding the remembered set the following must apply:
+ // - We always try to update the remembered sets of humongous regions containing
+ // type arrays as they might have been reset after full gc.
+ if (is_live && oop(r->humongous_start_region()->bottom())->is_typeArray() && !r->rem_set()->is_tracked()) {
+ r->rem_set()->set_state_updating();
+ selected_for_rebuild = true;
+ }
+
+ size_t const live_bytes = is_live ? HeapRegion::GrainBytes : 0;
+ print_before_rebuild(r, selected_for_rebuild, live_bytes, live_bytes);
+
+ return selected_for_rebuild;
+}
+
bool G1RemSetTrackingPolicy::update_before_rebuild(HeapRegion* r, size_t live_bytes) {
assert(SafepointSynchronize::is_at_safepoint(), "should be at safepoint");
-
- bool selected_for_rebuild = false;
+ assert(!r->is_humongous(), "Region %u is humongous", r->hrm_index());
// Only consider updating the remembered set for old gen regions - excluding archive regions
// which never move (but are "Old" regions).
- if (r->is_old_or_humongous() && !r->is_archive()) {
- size_t between_ntams_and_top = (r->top() - r->next_top_at_mark_start()) * HeapWordSize;
- size_t total_live_bytes = live_bytes + between_ntams_and_top;
- // Completely free regions after rebuild are of no interest wrt rebuilding the
- // remembered set.
- assert(!r->rem_set()->is_updating(), "Remembered set of region %u is updating before rebuild", r->hrm_index());
- // To be of interest for rebuilding the remembered set the following must apply:
- // - They must contain some live data in them.
- // - We always try to update the remembered sets of humongous regions containing
- // type arrays if they are empty as they might have been reset after full gc.
- // - Only need to rebuild non-complete remembered sets.
- // - Otherwise only add those old gen regions which occupancy is low enough that there
- // is a chance that we will ever evacuate them in the mixed gcs.
- if ((total_live_bytes > 0) &&
- (is_interesting_humongous_region(r) || CollectionSetChooser::region_occupancy_low_enough_for_evac(total_live_bytes)) &&
- !r->rem_set()->is_tracked()) {
+ if (!r->is_old() || r->is_archive()) {
+ return false;
+ }
+
+ assert(!r->rem_set()->is_updating(), "Remembered set of region %u is updating before rebuild", r->hrm_index());
+
+ size_t between_ntams_and_top = (r->top() - r->next_top_at_mark_start()) * HeapWordSize;
+ size_t total_live_bytes = live_bytes + between_ntams_and_top;
- r->rem_set()->set_state_updating();
- selected_for_rebuild = true;
- }
- log_trace(gc, remset, tracking)("Before rebuild region %u "
- "(ntams: " PTR_FORMAT ") "
- "total_live_bytes " SIZE_FORMAT " "
- "selected %s "
- "(live_bytes " SIZE_FORMAT " "
- "next_marked " SIZE_FORMAT " "
- "marked " SIZE_FORMAT " "
- "type %s)",
- r->hrm_index(),
- p2i(r->next_top_at_mark_start()),
- total_live_bytes,
- BOOL_TO_STR(selected_for_rebuild),
- live_bytes,
- r->next_marked_bytes(),
- r->marked_bytes(),
- r->get_type_str());
+ bool selected_for_rebuild = false;
+ // For old regions, to be of interest for rebuilding the remembered set the following must apply:
+ // - They must contain some live data in them.
+ // - Only need to rebuild non-complete remembered sets.
+ // - Otherwise only add those old gen regions which occupancy is low enough that there
+ // is a chance that we will ever evacuate them in the mixed gcs.
+ if ((total_live_bytes > 0) &&
+ CollectionSetChooser::region_occupancy_low_enough_for_evac(total_live_bytes) &&
+ !r->rem_set()->is_tracked()) {
+
+ r->rem_set()->set_state_updating();
+ selected_for_rebuild = true;
}
+ print_before_rebuild(r, selected_for_rebuild, total_live_bytes, live_bytes);
+
return selected_for_rebuild;
}
@@ -149,4 +175,3 @@
r->rem_set()->mem_size());
}
}
-
--- a/src/hotspot/share/gc/g1/g1RemSetTrackingPolicy.hpp Mon Jul 09 12:20:56 2018 +0800
+++ b/src/hotspot/share/gc/g1/g1RemSetTrackingPolicy.hpp Mon Jul 09 10:19:51 2018 +0200
@@ -33,10 +33,6 @@
// the remembered set, ie. when it should be tracked, and if/when the remembered
// set is complete.
class G1RemSetTrackingPolicy : public CHeapObj<mtGC> {
-private:
- // Is the given region an interesting humongous region to start remembered set tracking
- // for?
- bool is_interesting_humongous_region(HeapRegion* r) const;
public:
// Do we need to scan the given region to get all outgoing references for remembered
// set rebuild?
@@ -45,6 +41,9 @@
// called at any time. The caller makes sure that the changes to the remembered
// set state are visible to other threads.
void update_at_allocate(HeapRegion* r);
+ // Update remembered set tracking state for humongous regions before we are going to
+ // rebuild remembered sets. Called at safepoint in the remark pause.
+ bool update_humongous_before_rebuild(HeapRegion* r, bool is_live);
// Update remembered set tracking state before we are going to rebuild remembered
// sets. Called at safepoint in the remark pause.
bool update_before_rebuild(HeapRegion* r, size_t live_bytes);
--- /dev/null Thu Jan 01 00:00:00 1970 +0000
+++ b/test/hotspot/jtreg/gc/g1/TestHumongousRemsetsMatch.java Mon Jul 09 10:19:51 2018 +0200
@@ -0,0 +1,79 @@
+/*
+ * Copyright (c) 2018, 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
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+
+/*
+ * @test TestHumongousRemSetsMatch
+ * @bug 8205426
+ * @summary Test to make sure that humongous object remset states are in sync
+ * @key gc
+ * @requires vm.gc.G1
+ * @library /test/lib
+ * @build sun.hotspot.WhiteBox
+ * @run driver ClassFileInstaller sun.hotspot.WhiteBox
+ * @run main/othervm -Xbootclasspath/a:. -XX:+UnlockDiagnosticVMOptions -Xmx512M -Xms512M -Xmn10M -XX:ParallelGCThreads=2 -XX:-UseDynamicNumberOfGCThreads -XX:+UseG1GC -XX:+WhiteBoxAPI -XX:G1HeapRegionSize=1M -XX:+VerifyAfterGC -Xlog:gc,gc+remset+tracking=trace TestHumongousRemsetsMatch
+ */
+
+import sun.hotspot.WhiteBox;
+
+public class TestHumongousRemsetsMatch {
+
+ // G1 at the moment uses one thread every this amount of regions.
+ private static final int WorkerThreadBoundary = 384;
+
+ private static final int ObjSizeInRegions = 17;
+ private static final int M = 1024 * 1024;
+ private static final int TypeArrayObjSize = ObjSizeInRegions * M / 4 /* sizeof(int) */ - 1024 /* > header size */;
+
+ public static void main(String[] args) throws Exception {
+ WhiteBox wb = WhiteBox.getWhiteBox();
+
+ for (int j = 0; j < 3; j++) {
+ wb.fullGC(); // Start with a clean slate
+
+ // It may happen that our 7-region sized humongous objects may just be "misaligned"
+ // so that they do not cross the region 384 boundary. Try to counter this by offsetting
+ // the humongous objects just a little.
+ Object alignmentFudge = new int[(j + 1) * M / 4 /* sizeof(int) */ - 1024];
+
+ // Fill the heap so that more than WorkerThreadBoundary regions are occupied with humongous objects
+ // and hopefully one of these objects crosses the region WorkerThreadBoundary boundary.
+ Object[] lotsOfHumongousObjects = new Object[(WorkerThreadBoundary / ObjSizeInRegions) + 3];
+
+ for (int i = 0; i < lotsOfHumongousObjects.length; i++) {
+ lotsOfHumongousObjects[i] = new int[TypeArrayObjSize];
+ }
+
+ wb.fullGC();
+
+ // Trigger a concurrent cycle and wait until the Remark pause
+ wb.g1StartConcMarkCycle();
+ while (wb.g1InConcurrentMark()) {
+ Thread.sleep(200);
+ }
+ wb.youngGC(); // Trigger verification error.
+
+ System.out.println(lotsOfHumongousObjects + " " + alignmentFudge);
+ }
+ }
+}
+