8169703: G1 crashes with guarantee(pretouch_gang != NULL) failed: No pretouch gang specified
authortschatzl
Thu, 24 Nov 2016 10:05:47 +0100
changeset 42595 b1ae41a4eae9
parent 42593 d7be1cb119a2
child 42596 900edeec9776
8169703: G1 crashes with guarantee(pretouch_gang != NULL) failed: No pretouch gang specified Summary: Allow use of AlwaysPreTouch without passing a WorkGang. Reviewed-by: kbarrett, dfazunen, ddmitriev
hotspot/src/share/vm/gc/g1/g1CollectedHeap.cpp
hotspot/src/share/vm/gc/g1/g1PageBasedVirtualSpace.cpp
hotspot/src/share/vm/gc/g1/heapRegionManager.cpp
hotspot/src/share/vm/gc/g1/heapRegionManager.hpp
hotspot/test/gc/g1/TestParallelAlwaysPreTouch.java
hotspot/test/gc/g1/TestSharedArchiveWithPreTouch.java
--- a/hotspot/src/share/vm/gc/g1/g1CollectedHeap.cpp	Wed Nov 23 14:33:45 2016 +0100
+++ b/hotspot/src/share/vm/gc/g1/g1CollectedHeap.cpp	Thu Nov 24 10:05:47 2016 +0100
@@ -428,8 +428,7 @@
       log_debug(gc, ergo, heap)("Attempt heap expansion (humongous allocation request failed). Allocation request: " SIZE_FORMAT "B",
                                     word_size * HeapWordSize);
 
-
-      _hrm.expand_at(first, obj_regions);
+      _hrm.expand_at(first, obj_regions, workers());
       g1_policy()->record_new_heap_size(num_regions());
 
 #ifdef ASSERT
@@ -741,7 +740,7 @@
 
     // Perform the actual region allocation, exiting if it fails.
     // Then note how much new space we have allocated.
-    if (!_hrm.allocate_containing_regions(curr_range, &commits)) {
+    if (!_hrm.allocate_containing_regions(curr_range, &commits, workers())) {
       return false;
     }
     increase_used(word_size * HeapWordSize);
--- a/hotspot/src/share/vm/gc/g1/g1PageBasedVirtualSpace.cpp	Wed Nov 23 14:33:45 2016 +0100
+++ b/hotspot/src/share/vm/gc/g1/g1PageBasedVirtualSpace.cpp	Thu Nov 24 10:05:47 2016 +0100
@@ -235,11 +235,12 @@
 public:
   G1PretouchTask(char* start_address, char* end_address, size_t page_size) :
     AbstractGangTask("G1 PreTouch",
-                     Universe::is_fully_initialized() ? GCId::current_raw() :
-                                                        // During VM initialization there is
-                                                        // no GC cycle that this task can be
-                                                        // associated with.
-                                                        GCId::undefined()),
+                     Universe::is_fully_initialized() &&
+                     Thread::current()->is_Named_thread() ? GCId::current_raw() :
+                                                            // During VM initialization there is
+                                                            // no GC cycle that this task can be
+                                                            // associated with.
+                                                            GCId::undefined()),
     _cur_addr(start_address),
     _start_addr(start_address),
     _end_addr(end_address),
@@ -262,15 +263,20 @@
 };
 
 void G1PageBasedVirtualSpace::pretouch(size_t start_page, size_t size_in_pages, WorkGang* pretouch_gang) {
-  guarantee(pretouch_gang != NULL, "No pretouch gang specified.");
+  G1PretouchTask cl(page_start(start_page), bounded_end_addr(start_page + size_in_pages), _page_size);
 
-  size_t num_chunks = MAX2((size_t)1, size_in_pages * _page_size / MAX2(G1PretouchTask::chunk_size(), _page_size));
+  if (pretouch_gang != NULL) {
+    size_t num_chunks = MAX2((size_t)1, size_in_pages * _page_size / MAX2(G1PretouchTask::chunk_size(), _page_size));
 
-  uint num_workers = MIN2((uint)num_chunks, pretouch_gang->active_workers());
-  G1PretouchTask cl(page_start(start_page), bounded_end_addr(start_page + size_in_pages), _page_size);
-  log_debug(gc, heap)("Running %s with %u workers for " SIZE_FORMAT " work units pre-touching " SIZE_FORMAT "B.",
-                      cl.name(), num_workers, num_chunks, size_in_pages * _page_size);
-  pretouch_gang->run_task(&cl, num_workers);
+    uint num_workers = MIN2((uint)num_chunks, pretouch_gang->active_workers());
+    log_debug(gc, heap)("Running %s with %u workers for " SIZE_FORMAT " work units pre-touching " SIZE_FORMAT "B.",
+                        cl.name(), num_workers, num_chunks, size_in_pages * _page_size);
+    pretouch_gang->run_task(&cl, num_workers);
+  } else {
+    log_debug(gc, heap)("Running %s pre-touching " SIZE_FORMAT "B.",
+                        cl.name(), size_in_pages * _page_size);
+    cl.work(0);
+  }
 }
 
 bool G1PageBasedVirtualSpace::contains(const void* p) const {
--- a/hotspot/src/share/vm/gc/g1/heapRegionManager.cpp	Wed Nov 23 14:33:45 2016 +0100
+++ b/hotspot/src/share/vm/gc/g1/heapRegionManager.cpp	Thu Nov 24 10:05:47 2016 +0100
@@ -286,7 +286,7 @@
   while (true) {
     HeapRegion *hr = _regions.get_by_index(curr);
     if (hr == NULL) {
-      uint res = expand_at(curr, 1);
+      uint res = expand_at(curr, 1, NULL);
       if (res == 1) {
         *expanded = true;
         return curr;
@@ -304,7 +304,7 @@
   }
 }
 
-bool HeapRegionManager::allocate_containing_regions(MemRegion range, size_t* commit_count) {
+bool HeapRegionManager::allocate_containing_regions(MemRegion range, size_t* commit_count, WorkGang* pretouch_workers) {
   size_t commits = 0;
   uint start_index = (uint)_regions.get_index_by_address(range.start());
   uint last_index = (uint)_regions.get_index_by_address(range.last());
@@ -314,7 +314,7 @@
   for (uint curr_index = start_index; curr_index <= last_index; curr_index++) {
     if (!is_available(curr_index)) {
       commits++;
-      expand_at(curr_index, 1);
+      expand_at(curr_index, 1, pretouch_workers);
     }
     HeapRegion* curr_region  = _regions.get_by_index(curr_index);
     if (!curr_region->is_free()) {
--- a/hotspot/src/share/vm/gc/g1/heapRegionManager.hpp	Wed Nov 23 14:33:45 2016 +0100
+++ b/hotspot/src/share/vm/gc/g1/heapRegionManager.hpp	Thu Nov 24 10:05:47 2016 +0100
@@ -210,12 +210,12 @@
   // HeapRegions, or re-use existing ones. Returns the number of regions the
   // sequence was expanded by. If a HeapRegion allocation fails, the resulting
   // number of regions might be smaller than what's desired.
-  uint expand_by(uint num_regions, WorkGang* pretouch_workers = NULL);
+  uint expand_by(uint num_regions, WorkGang* pretouch_workers);
 
   // Makes sure that the regions from start to start+num_regions-1 are available
   // for allocation. Returns the number of regions that were committed to achieve
   // this.
-  uint expand_at(uint start, uint num_regions, WorkGang* pretouch_workers = NULL);
+  uint expand_at(uint start, uint num_regions, WorkGang* pretouch_workers);
 
   // Find a contiguous set of empty regions of length num. Returns the start index of
   // that set, or G1_NO_HRM_INDEX.
@@ -234,7 +234,7 @@
   // Allocate the regions that contain the address range specified, committing the
   // regions if necessary. Return false if any of the regions is already committed
   // and not free, and return the number of regions newly committed in commit_count.
-  bool allocate_containing_regions(MemRegion range, size_t* commit_count);
+  bool allocate_containing_regions(MemRegion range, size_t* commit_count, WorkGang* pretouch_workers);
 
   // Apply blk->doHeapRegion() on all committed regions in address order,
   // terminating the iteration early if doHeapRegion() returns true.
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/hotspot/test/gc/g1/TestParallelAlwaysPreTouch.java	Thu Nov 24 10:05:47 2016 +0100
@@ -0,0 +1,46 @@
+/*
+ * 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
+ * @bug 8169703
+ * @summary Regression test to ensure AlwaysPreTouch with multiple threads works at mutator time.
+ * Allocates a few humongous objects that will be allocated by expanding the heap, causing concurrent parallel
+ * pre-touch.
+ * @requires vm.gc.G1
+ * @key gc
+ * @key regression
+ * @run main/othervm -XX:+UseG1GC -Xms10M -Xmx100m -XX:G1HeapRegionSize=1M -XX:+AlwaysPreTouch -XX:PreTouchParallelChunkSize=512k -Xlog:gc+ergo+heap=debug,gc+heap=debug,gc=debug TestParallelAlwaysPreTouch
+ */
+
+public class TestParallelAlwaysPreTouch {
+    public static void main(String[] args) throws Exception {
+        final int M = 1024 * 1024; // Something guaranteed to be larger than a region to be counted as humongous.
+
+        for (int i = 0; i < 10; i++) {
+            Object[] obj = new Object[M];
+            System.out.println(obj);
+        }
+    }
+}
+
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/hotspot/test/gc/g1/TestSharedArchiveWithPreTouch.java	Thu Nov 24 10:05:47 2016 +0100
@@ -0,0 +1,84 @@
+/*
+ * 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
+ * @bug 8169703
+ * @summary Verifies that dumping and loading a CDS archive succeeds with AlwaysPreTouch
+ * @requires vm.gc.G1
+ * @key gc
+ * @key regression
+ * @library /test/lib
+ * @modules java.base/jdk.internal.misc
+ *          java.management
+ * @run main TestSharedArchiveWithPreTouch
+ */
+
+import java.util.List;
+import java.util.ArrayList;
+import java.util.Arrays;
+
+import jdk.test.lib.Platform;
+import jdk.test.lib.process.ProcessTools;
+import jdk.test.lib.process.OutputAnalyzer;
+
+public class TestSharedArchiveWithPreTouch {
+    public static void main(String[] args) throws Exception {
+        final String ArchiveFileName = "./SharedArchiveWithPreTouch.jsa";
+
+        final List<String> BaseOptions = Arrays.asList(new String[] {"-XX:+UseG1GC", "-XX:+AlwaysPreTouch",
+            "-XX:+UnlockDiagnosticVMOptions", "-XX:SharedArchiveFile=" + ArchiveFileName });
+
+        ProcessBuilder pb;
+
+        List<String> dump_args = new ArrayList<String>(BaseOptions);
+
+        if (Platform.is64bit()) {
+          dump_args.addAll(0, Arrays.asList(new String[] { "-XX:+UseCompressedClassPointers", "-XX:+UseCompressedOops" }));
+        }
+        dump_args.addAll(Arrays.asList(new String[] { "-Xshare:dump" }));
+
+        pb = ProcessTools.createJavaProcessBuilder(dump_args.toArray(new String[0]));
+        OutputAnalyzer output = new OutputAnalyzer(pb.start());
+        try {
+            output.shouldContain("Loading classes to share");
+            output.shouldHaveExitValue(0);
+
+            List<String> load_args = new ArrayList<String>(BaseOptions);
+
+            if (Platform.is64bit()) {
+                load_args.addAll(0, Arrays.asList(new String[] { "-XX:+UseCompressedClassPointers", "-XX:+UseCompressedOops" }));
+            }
+            load_args.addAll(Arrays.asList(new String[] { "-Xshare:on", "-version" }));
+
+            pb = ProcessTools.createJavaProcessBuilder(load_args.toArray(new String[0]));
+            output = new OutputAnalyzer(pb.start());
+            output.shouldContain("sharing");
+            output.shouldHaveExitValue(0);
+        } catch (RuntimeException e) {
+            // Report 'passed' if CDS was turned off.
+            output.shouldContain("Unable to use shared archive");
+            output.shouldHaveExitValue(1);
+        }
+    }
+}