8199850: Move parsing of VerifyGCType to G1
authorpliden
Mon, 26 Mar 2018 12:44:39 +0200
changeset 49602 db050c11c3b5
parent 49601 bfc70d5d291a
child 49603 6ce4db4460ca
8199850: Move parsing of VerifyGCType to G1 Reviewed-by: sjohanss, tschatzl
src/hotspot/share/gc/g1/g1Arguments.cpp
src/hotspot/share/gc/g1/g1Arguments.hpp
src/hotspot/share/gc/g1/g1HeapVerifier.cpp
src/hotspot/share/gc/g1/g1HeapVerifier.hpp
src/hotspot/share/gc/shared/gcArguments.cpp
src/hotspot/share/gc/shared/gcArguments.hpp
src/hotspot/share/memory/universe.cpp
test/hotspot/gtest/gc/g1/test_g1HeapVerifier.cpp
test/hotspot/jtreg/gc/g1/TestVerifyGCType.java
--- a/src/hotspot/share/gc/g1/g1Arguments.cpp	Mon Mar 26 09:35:20 2018 -0400
+++ b/src/hotspot/share/gc/g1/g1Arguments.cpp	Mon Mar 26 12:44:39 2018 +0200
@@ -1,4 +1,5 @@
 /*
+ * Copyright (c) 2018, Oracle and/or its affiliates. All rights reserved.
  * Copyright (c) 2017, Red Hat, Inc. and/or its affiliates.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
@@ -37,6 +38,40 @@
   return HeapRegion::max_region_size();
 }
 
+void G1Arguments::initialize_verification_types() {
+  if (strlen(VerifyGCType) > 0) {
+    const char delimiter[] = " ,\n";
+    size_t length = strlen(VerifyGCType);
+    char* type_list = NEW_C_HEAP_ARRAY(char, length + 1, mtInternal);
+    strncpy(type_list, VerifyGCType, length + 1);
+    char* token = strtok(type_list, delimiter);
+    while (token != NULL) {
+      parse_verification_type(token);
+      token = strtok(NULL, delimiter);
+    }
+    FREE_C_HEAP_ARRAY(char, type_list);
+  }
+}
+
+void G1Arguments::parse_verification_type(const char* type) {
+  if (strcmp(type, "young-only") == 0) {
+    G1HeapVerifier::enable_verification_type(G1HeapVerifier::G1VerifyYoungOnly);
+  } else if (strcmp(type, "initial-mark") == 0) {
+    G1HeapVerifier::enable_verification_type(G1HeapVerifier::G1VerifyInitialMark);
+  } else if (strcmp(type, "mixed") == 0) {
+    G1HeapVerifier::enable_verification_type(G1HeapVerifier::G1VerifyMixed);
+  } else if (strcmp(type, "remark") == 0) {
+    G1HeapVerifier::enable_verification_type(G1HeapVerifier::G1VerifyRemark);
+  } else if (strcmp(type, "cleanup") == 0) {
+    G1HeapVerifier::enable_verification_type(G1HeapVerifier::G1VerifyCleanup);
+  } else if (strcmp(type, "full") == 0) {
+    G1HeapVerifier::enable_verification_type(G1HeapVerifier::G1VerifyFull);
+  } else {
+    log_warning(gc, verify)("VerifyGCType: '%s' is unknown. Available types are: "
+                            "young-only, initial-mark, mixed, remark, cleanup and full", type);
+  }
+}
+
 void G1Arguments::initialize_flags() {
   GCArguments::initialize_flags();
   assert(UseG1GC, "Error");
@@ -100,12 +135,8 @@
     }
   }
 #endif
-}
 
-bool G1Arguments::parse_verification_type(const char* type) {
-  G1CollectedHeap::heap()->verifier()->parse_verification_type(type);
-  // Always return true because we want to parse all values.
-  return true;
+  initialize_verification_types();
 }
 
 CollectedHeap* G1Arguments::create_heap() {
--- a/src/hotspot/share/gc/g1/g1Arguments.hpp	Mon Mar 26 09:35:20 2018 -0400
+++ b/src/hotspot/share/gc/g1/g1Arguments.hpp	Mon Mar 26 12:44:39 2018 +0200
@@ -1,4 +1,5 @@
 /*
+ * Copyright (c) 2018, Oracle and/or its affiliates. All rights reserved.
  * Copyright (c) 2017, Red Hat, Inc. and/or its affiliates.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
@@ -30,9 +31,14 @@
 class CollectedHeap;
 
 class G1Arguments : public GCArguments {
+  friend class G1HeapVerifierTest_parse_Test;
+
+private:
+  static void initialize_verification_types();
+  static void parse_verification_type(const char* type);
+
 public:
   virtual void initialize_flags();
-  virtual bool parse_verification_type(const char* type);
   virtual size_t conservative_max_heap_alignment();
   virtual CollectedHeap* create_heap();
 };
--- a/src/hotspot/share/gc/g1/g1HeapVerifier.cpp	Mon Mar 26 09:35:20 2018 -0400
+++ b/src/hotspot/share/gc/g1/g1HeapVerifier.cpp	Mon Mar 26 12:44:39 2018 +0200
@@ -43,6 +43,8 @@
 #include "oops/oop.inline.hpp"
 #include "runtime/handles.inline.hpp"
 
+int G1HeapVerifier::_enabled_verification_types = G1HeapVerifier::G1VerifyAll;
+
 class VerifyRootsClosure: public OopClosure {
 private:
   G1CollectedHeap* _g1h;
@@ -379,25 +381,6 @@
   }
 };
 
-void G1HeapVerifier::parse_verification_type(const char* type) {
-  if (strcmp(type, "young-only") == 0) {
-    enable_verification_type(G1VerifyYoungOnly);
-  } else if (strcmp(type, "initial-mark") == 0) {
-    enable_verification_type(G1VerifyInitialMark);
-  } else if (strcmp(type, "mixed") == 0) {
-    enable_verification_type(G1VerifyMixed);
-  } else if (strcmp(type, "remark") == 0) {
-    enable_verification_type(G1VerifyRemark);
-  } else if (strcmp(type, "cleanup") == 0) {
-    enable_verification_type(G1VerifyCleanup);
-  } else if (strcmp(type, "full") == 0) {
-    enable_verification_type(G1VerifyFull);
-  } else {
-    log_warning(gc, verify)("VerifyGCType: '%s' is unknown. Available types are: "
-                            "young-only, initial-mark, mixed, remark, cleanup and full", type);
-  }
-}
-
 void G1HeapVerifier::enable_verification_type(G1VerifyType type) {
   // First enable will clear _enabled_verification_types.
   if (_enabled_verification_types == G1VerifyAll) {
--- a/src/hotspot/share/gc/g1/g1HeapVerifier.hpp	Mon Mar 26 09:35:20 2018 -0400
+++ b/src/hotspot/share/gc/g1/g1HeapVerifier.hpp	Mon Mar 26 12:44:39 2018 +0200
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2016, 2017, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2016, 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
@@ -33,8 +33,9 @@
 
 class G1HeapVerifier : public CHeapObj<mtGC> {
 private:
+  static int _enabled_verification_types;
+
   G1CollectedHeap* _g1h;
-  int _enabled_verification_types;
 
   // verify_region_sets() performs verification over the region
   // lists. It will be compiled in the product code to be used when
@@ -52,11 +53,10 @@
     G1VerifyAll         = -1
   };
 
-  G1HeapVerifier(G1CollectedHeap* heap) : _g1h(heap), _enabled_verification_types(G1VerifyAll) { }
+  G1HeapVerifier(G1CollectedHeap* heap) : _g1h(heap) {}
 
-  void parse_verification_type(const char* type);
-  void enable_verification_type(G1VerifyType type);
-  bool should_verify(G1VerifyType type);
+  static void enable_verification_type(G1VerifyType type);
+  static bool should_verify(G1VerifyType type);
 
   // Perform verification.
 
--- a/src/hotspot/share/gc/shared/gcArguments.cpp	Mon Mar 26 09:35:20 2018 -0400
+++ b/src/hotspot/share/gc/shared/gcArguments.cpp	Mon Mar 26 12:44:39 2018 +0200
@@ -1,4 +1,5 @@
 /*
+ * Copyright (c) 2018, Oracle and/or its affiliates. All rights reserved.
  * Copyright (c) 2017, Red Hat, Inc. and/or its affiliates.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
@@ -85,12 +86,6 @@
 #endif // INCLUDE_ALL_GCS
 }
 
-bool GCArguments::parse_verification_type(const char* type) {
-  log_warning(gc, verify)("VerifyGCType is not supported by this collector.");
-  // Return false to avoid multiple warnings.
-  return false;
-}
-
 void GCArguments::initialize_flags() {
 #if INCLUDE_ALL_GCS
   if (MinHeapFreeRatio == 100) {
@@ -106,24 +101,6 @@
 #endif // INCLUDE_ALL_GCS
 }
 
-void GCArguments::post_heap_initialize() {
-  if (strlen(VerifyGCType) > 0) {
-    const char delimiter[] = " ,\n";
-    size_t length = strlen(VerifyGCType);
-    char* type_list = NEW_C_HEAP_ARRAY(char, length + 1, mtInternal);
-    strncpy(type_list, VerifyGCType, length + 1);
-    char* token = strtok(type_list, delimiter);
-    while (token != NULL) {
-      bool success = parse_verification_type(token);
-      if (!success) {
-        break;
-      }
-      token = strtok(NULL, delimiter);
-    }
-    FREE_C_HEAP_ARRAY(char, type_list);
-  }
-}
-
 jint GCArguments::initialize() {
   assert(!is_initialized(), "GC arguments already initialized");
 
--- a/src/hotspot/share/gc/shared/gcArguments.hpp	Mon Mar 26 09:35:20 2018 -0400
+++ b/src/hotspot/share/gc/shared/gcArguments.hpp	Mon Mar 26 12:44:39 2018 +0200
@@ -1,4 +1,5 @@
 /*
+ * Copyright (c) 2018, Oracle and/or its affiliates. All rights reserved.
  * Copyright (c) 2017, Red Hat, Inc. and/or its affiliates.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
@@ -46,16 +47,8 @@
   static bool is_initialized();
   static GCArguments* arguments();
 
-  void post_heap_initialize();
-
   virtual void initialize_flags();
 
-  // Collector specific function to allow finer grained verification
-  // through VerifyGCType. If not overridden the default version will
-  // warn that the flag is not supported for the given collector.
-  // Returns true if parsing should continue, false otherwise.
-  virtual bool parse_verification_type(const char* type);
-
   virtual size_t conservative_max_heap_alignment() = 0;
 
   virtual CollectedHeap* create_heap() = 0;
--- a/src/hotspot/share/memory/universe.cpp	Mon Mar 26 09:35:20 2018 -0400
+++ b/src/hotspot/share/memory/universe.cpp	Mon Mar 26 12:44:39 2018 +0200
@@ -765,7 +765,6 @@
   }
   log_info(gc)("Using %s", _collectedHeap->name());
 
-  GCArguments::arguments()->post_heap_initialize();
   ThreadLocalAllocBuffer::set_max_size(Universe::heap()->max_tlab_size());
 
 #ifdef _LP64
--- a/test/hotspot/gtest/gc/g1/test_g1HeapVerifier.cpp	Mon Mar 26 09:35:20 2018 -0400
+++ b/test/hotspot/gtest/gc/g1/test_g1HeapVerifier.cpp	Mon Mar 26 12:44:39 2018 +0200
@@ -23,6 +23,7 @@
  */
 
 #include "precompiled.hpp"
+#include "gc/g1/g1Arguments.hpp"
 #include "gc/g1/g1HeapVerifier.hpp"
 #include "logging/logConfiguration.hpp"
 #include "logging/logTestFixture.hpp"
@@ -32,50 +33,48 @@
 };
 
 TEST_F(G1HeapVerifierTest, parse) {
-  G1HeapVerifier verifier(NULL);
-
   LogConfiguration::configure_stdout(LogLevel::Off, true, LOG_TAGS(gc, verify));
 
   // Default is to verify everything.
-  ASSERT_TRUE(verifier.should_verify(G1HeapVerifier::G1VerifyAll));
-  ASSERT_TRUE(verifier.should_verify(G1HeapVerifier::G1VerifyYoungOnly));
-  ASSERT_TRUE(verifier.should_verify(G1HeapVerifier::G1VerifyInitialMark));
-  ASSERT_TRUE(verifier.should_verify(G1HeapVerifier::G1VerifyMixed));
-  ASSERT_TRUE(verifier.should_verify(G1HeapVerifier::G1VerifyRemark));
-  ASSERT_TRUE(verifier.should_verify(G1HeapVerifier::G1VerifyCleanup));
-  ASSERT_TRUE(verifier.should_verify(G1HeapVerifier::G1VerifyFull));
+  ASSERT_TRUE(G1HeapVerifier::should_verify(G1HeapVerifier::G1VerifyAll));
+  ASSERT_TRUE(G1HeapVerifier::should_verify(G1HeapVerifier::G1VerifyYoungOnly));
+  ASSERT_TRUE(G1HeapVerifier::should_verify(G1HeapVerifier::G1VerifyInitialMark));
+  ASSERT_TRUE(G1HeapVerifier::should_verify(G1HeapVerifier::G1VerifyMixed));
+  ASSERT_TRUE(G1HeapVerifier::should_verify(G1HeapVerifier::G1VerifyRemark));
+  ASSERT_TRUE(G1HeapVerifier::should_verify(G1HeapVerifier::G1VerifyCleanup));
+  ASSERT_TRUE(G1HeapVerifier::should_verify(G1HeapVerifier::G1VerifyFull));
 
   // Setting one will disable all other.
-  verifier.parse_verification_type("full");
-  ASSERT_FALSE(verifier.should_verify(G1HeapVerifier::G1VerifyAll));
-  ASSERT_FALSE(verifier.should_verify(G1HeapVerifier::G1VerifyYoungOnly));
-  ASSERT_FALSE(verifier.should_verify(G1HeapVerifier::G1VerifyInitialMark));
-  ASSERT_FALSE(verifier.should_verify(G1HeapVerifier::G1VerifyMixed));
-  ASSERT_FALSE(verifier.should_verify(G1HeapVerifier::G1VerifyRemark));
-  ASSERT_FALSE(verifier.should_verify(G1HeapVerifier::G1VerifyCleanup));
-  ASSERT_TRUE(verifier.should_verify(G1HeapVerifier::G1VerifyFull));
+  G1Arguments::parse_verification_type("full");
+  ASSERT_FALSE(G1HeapVerifier::should_verify(G1HeapVerifier::G1VerifyAll));
+  ASSERT_FALSE(G1HeapVerifier::should_verify(G1HeapVerifier::G1VerifyYoungOnly));
+  ASSERT_FALSE(G1HeapVerifier::should_verify(G1HeapVerifier::G1VerifyInitialMark));
+  ASSERT_FALSE(G1HeapVerifier::should_verify(G1HeapVerifier::G1VerifyMixed));
+  ASSERT_FALSE(G1HeapVerifier::should_verify(G1HeapVerifier::G1VerifyRemark));
+  ASSERT_FALSE(G1HeapVerifier::should_verify(G1HeapVerifier::G1VerifyCleanup));
+  ASSERT_TRUE(G1HeapVerifier::should_verify(G1HeapVerifier::G1VerifyFull));
 
   // Verify case sensitivity.
-  verifier.parse_verification_type("YOUNG-ONLY");
-  ASSERT_FALSE(verifier.should_verify(G1HeapVerifier::G1VerifyYoungOnly));
-  verifier.parse_verification_type("young-only");
-  ASSERT_TRUE(verifier.should_verify(G1HeapVerifier::G1VerifyYoungOnly));
+  G1Arguments::parse_verification_type("YOUNG-ONLY");
+  ASSERT_FALSE(G1HeapVerifier::should_verify(G1HeapVerifier::G1VerifyYoungOnly));
+  G1Arguments::parse_verification_type("young-only");
+  ASSERT_TRUE(G1HeapVerifier::should_verify(G1HeapVerifier::G1VerifyYoungOnly));
 
   // Verify perfect match
-  verifier.parse_verification_type("mixedgc");
-  ASSERT_FALSE(verifier.should_verify(G1HeapVerifier::G1VerifyMixed));
-  verifier.parse_verification_type("mixe");
-  ASSERT_FALSE(verifier.should_verify(G1HeapVerifier::G1VerifyMixed));
-  verifier.parse_verification_type("mixed");
-  ASSERT_TRUE(verifier.should_verify(G1HeapVerifier::G1VerifyMixed));
+  G1Arguments::parse_verification_type("mixedgc");
+  ASSERT_FALSE(G1HeapVerifier::should_verify(G1HeapVerifier::G1VerifyMixed));
+  G1Arguments::parse_verification_type("mixe");
+  ASSERT_FALSE(G1HeapVerifier::should_verify(G1HeapVerifier::G1VerifyMixed));
+  G1Arguments::parse_verification_type("mixed");
+  ASSERT_TRUE(G1HeapVerifier::should_verify(G1HeapVerifier::G1VerifyMixed));
 
   // Verify the last three
-  verifier.parse_verification_type("initial-mark");
-  verifier.parse_verification_type("remark");
-  verifier.parse_verification_type("cleanup");
-  ASSERT_TRUE(verifier.should_verify(G1HeapVerifier::G1VerifyRemark));
-  ASSERT_TRUE(verifier.should_verify(G1HeapVerifier::G1VerifyCleanup));
+  G1Arguments::parse_verification_type("initial-mark");
+  G1Arguments::parse_verification_type("remark");
+  G1Arguments::parse_verification_type("cleanup");
+  ASSERT_TRUE(G1HeapVerifier::should_verify(G1HeapVerifier::G1VerifyRemark));
+  ASSERT_TRUE(G1HeapVerifier::should_verify(G1HeapVerifier::G1VerifyCleanup));
 
   // Enabling all is not the same as G1VerifyAll
-  ASSERT_FALSE(verifier.should_verify(G1HeapVerifier::G1VerifyAll));
+  ASSERT_FALSE(G1HeapVerifier::should_verify(G1HeapVerifier::G1VerifyAll));
 }
--- a/test/hotspot/jtreg/gc/g1/TestVerifyGCType.java	Mon Mar 26 09:35:20 2018 -0400
+++ b/test/hotspot/jtreg/gc/g1/TestVerifyGCType.java	Mon Mar 26 12:44:39 2018 +0200
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2017, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2017, 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
@@ -53,7 +53,6 @@
         testFullAndRemark();
         testConcurrentMark();
         testBadVerificationType();
-        testUnsupportedCollector();
     }
 
     private static void testAllVerificationEnabled() throws Exception {
@@ -127,14 +126,6 @@
         verifyCollection("Pause Full", true, true, true, output.getStdout());
     }
 
-    private static void testUnsupportedCollector() throws Exception {
-        OutputAnalyzer output;
-        // Test bad gc
-        output = testWithBadGC();
-        output.shouldHaveExitValue(0);
-        output.shouldMatch("VerifyGCType is not supported by this collector.");
-    }
-
     private static OutputAnalyzer testWithVerificationType(String[] types) throws Exception {
         ArrayList<String> basicOpts = new ArrayList<>();
         Collections.addAll(basicOpts, new String[] {
@@ -161,17 +152,6 @@
         return analyzer;
     }
 
-    private static OutputAnalyzer testWithBadGC() throws Exception {
-        ProcessBuilder procBuilder =  ProcessTools.createJavaProcessBuilder(new String[] {
-                "-XX:+UseParallelGC",
-                "-XX:+UnlockDiagnosticVMOptions",
-                "-XX:VerifyGCType=full",
-                "-version"});
-
-        OutputAnalyzer analyzer = new OutputAnalyzer(procBuilder.start());
-        return analyzer;
-    }
-
     private static void verifyCollection(String name, boolean expectBefore, boolean expectDuring, boolean expectAfter, String data) {
         CollectionInfo ci = CollectionInfo.parseFirst(name, data);
         Asserts.assertTrue(ci != null, "Expected GC not found: " + name + "\n" + data);