8151920: Region liveness printing is broken
authortschatzl
Thu, 17 Mar 2016 09:36:04 +0100
changeset 37130 73d99f6918aa
parent 37128 ea9e0371b8e6
child 37131 a530aaacd7c5
8151920: Region liveness printing is broken Summary: Remove obsolete code that causes an assert. Reviewed-by: drwhite, brutisso
hotspot/src/share/vm/gc/g1/g1ConcurrentMark.cpp
hotspot/src/share/vm/gc/g1/g1ConcurrentMark.hpp
hotspot/test/gc/g1/TestRegionLivenessPrint.java
--- a/hotspot/src/share/vm/gc/g1/g1ConcurrentMark.cpp	Wed Mar 16 16:21:36 2016 +0000
+++ b/hotspot/src/share/vm/gc/g1/g1ConcurrentMark.cpp	Thu Mar 17 09:36:04 2016 +0100
@@ -3557,8 +3557,6 @@
 G1PrintRegionLivenessInfoClosure(const char* phase_name)
   : _total_used_bytes(0), _total_capacity_bytes(0),
     _total_prev_live_bytes(0), _total_next_live_bytes(0),
-    _hum_used_bytes(0), _hum_capacity_bytes(0),
-    _hum_prev_live_bytes(0), _hum_next_live_bytes(0),
     _total_remset_bytes(0), _total_strong_code_roots_bytes(0) {
   G1CollectedHeap* g1h = G1CollectedHeap::heap();
   MemRegion g1_reserved = g1h->g1_reserved();
@@ -3598,36 +3596,6 @@
                           "(bytes)", "(bytes)");
 }
 
-// It takes as a parameter a reference to one of the _hum_* fields, it
-// deduces the corresponding value for a region in a humongous region
-// series (either the region size, or what's left if the _hum_* field
-// is < the region size), and updates the _hum_* field accordingly.
-size_t G1PrintRegionLivenessInfoClosure::get_hum_bytes(size_t* hum_bytes) {
-  size_t bytes = 0;
-  // The > 0 check is to deal with the prev and next live bytes which
-  // could be 0.
-  if (*hum_bytes > 0) {
-    bytes = MIN2(HeapRegion::GrainBytes, *hum_bytes);
-    *hum_bytes -= bytes;
-  }
-  return bytes;
-}
-
-// It deduces the values for a region in a humongous region series
-// from the _hum_* fields and updates those accordingly. It assumes
-// that that _hum_* fields have already been set up from the "starts
-// humongous" region and we visit the regions in address order.
-void G1PrintRegionLivenessInfoClosure::get_hum_bytes(size_t* used_bytes,
-                                                     size_t* capacity_bytes,
-                                                     size_t* prev_live_bytes,
-                                                     size_t* next_live_bytes) {
-  assert(_hum_used_bytes > 0 && _hum_capacity_bytes > 0, "pre-condition");
-  *used_bytes      = get_hum_bytes(&_hum_used_bytes);
-  *capacity_bytes  = get_hum_bytes(&_hum_capacity_bytes);
-  *prev_live_bytes = get_hum_bytes(&_hum_prev_live_bytes);
-  *next_live_bytes = get_hum_bytes(&_hum_next_live_bytes);
-}
-
 bool G1PrintRegionLivenessInfoClosure::doHeapRegion(HeapRegion* r) {
   const char* type       = r->get_type_str();
   HeapWord* bottom       = r->bottom();
@@ -3640,24 +3608,6 @@
   size_t remset_bytes    = r->rem_set()->mem_size();
   size_t strong_code_roots_bytes = r->rem_set()->strong_code_roots_mem_size();
 
-  if (r->is_starts_humongous()) {
-    assert(_hum_used_bytes == 0 && _hum_capacity_bytes == 0 &&
-           _hum_prev_live_bytes == 0 && _hum_next_live_bytes == 0,
-           "they should have been zeroed after the last time we used them");
-    // Set up the _hum_* fields.
-    _hum_capacity_bytes  = capacity_bytes;
-    _hum_used_bytes      = used_bytes;
-    _hum_prev_live_bytes = prev_live_bytes;
-    _hum_next_live_bytes = next_live_bytes;
-    get_hum_bytes(&used_bytes, &capacity_bytes,
-                  &prev_live_bytes, &next_live_bytes);
-    end = bottom + HeapRegion::GrainWords;
-  } else if (r->is_continues_humongous()) {
-    get_hum_bytes(&used_bytes, &capacity_bytes,
-                  &prev_live_bytes, &next_live_bytes);
-    assert(end == bottom + HeapRegion::GrainWords, "invariant");
-  }
-
   _total_used_bytes      += used_bytes;
   _total_capacity_bytes  += capacity_bytes;
   _total_prev_live_bytes += prev_live_bytes;
--- a/hotspot/src/share/vm/gc/g1/g1ConcurrentMark.hpp	Wed Mar 16 16:21:36 2016 +0000
+++ b/hotspot/src/share/vm/gc/g1/g1ConcurrentMark.hpp	Thu Mar 17 09:36:04 2016 +0100
@@ -1001,18 +1001,6 @@
   size_t _total_prev_live_bytes;
   size_t _total_next_live_bytes;
 
-  // These are set up when we come across a "stars humongous" region
-  // (as this is where most of this information is stored, not in the
-  // subsequent "continues humongous" regions). After that, for every
-  // region in a given humongous region series we deduce the right
-  // values for it by simply subtracting the appropriate amount from
-  // these fields. All these values should reach 0 after we've visited
-  // the last region in the series.
-  size_t _hum_used_bytes;
-  size_t _hum_capacity_bytes;
-  size_t _hum_prev_live_bytes;
-  size_t _hum_next_live_bytes;
-
   // Accumulator for the remembered set size
   size_t _total_remset_bytes;
 
@@ -1031,11 +1019,6 @@
     return (double) val / (double) M;
   }
 
-  // See the .cpp file.
-  size_t get_hum_bytes(size_t* hum_bytes);
-  void get_hum_bytes(size_t* used_bytes, size_t* capacity_bytes,
-                     size_t* prev_live_bytes, size_t* next_live_bytes);
-
 public:
   // The header and footer are printed in the constructor and
   // destructor respectively.
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/hotspot/test/gc/g1/TestRegionLivenessPrint.java	Thu Mar 17 09:36:04 2016 +0100
@@ -0,0 +1,53 @@
+/*
+ * Copyright (c) 2016, 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 TestRegionLivenessPrint.java
+ * @bug 8151920
+ * @requires vm.gc=="G1" | vm.gc=="null"
+ * @summary Make sure that G1 does not assert when printing region liveness data on a humongous continues region.
+ * @key gc
+ * @library /testlibrary /test/lib
+ * @modules java.base/sun.misc
+ * @build TestRegionLivenessPrint
+ * @run main ClassFileInstaller sun.hotspot.WhiteBox
+ *                              sun.hotspot.WhiteBox$WhiteBoxPermission
+ * @run main/othervm -Xbootclasspath/a:. -XX:+WhiteBoxAPI -XX:+UseG1GC -Xmx128M -XX:G1HeapRegionSize=1m -Xlog:gc+liveness=trace TestRegionLivenessPrint
+ */
+
+import sun.hotspot.WhiteBox;
+
+public class TestRegionLivenessPrint {
+
+  static byte[] bigobj = new byte[1024* 1024 * 2];
+
+  public static void main(String[] args) throws InterruptedException {
+      WhiteBox wb = WhiteBox.getWhiteBox();
+      // Run a concurrent mark cycle to trigger the liveness accounting log messages.
+      wb.g1StartConcMarkCycle();
+      while (wb.g1InConcurrentMark()) {
+          Thread.sleep(100);
+      }
+  }
+
+}