8068942: Improve validation of -XX:G1ConfidencePercent value

Validate during argument processing and simplify usage by assuming validated.

Reviewed-by: jmasa, tschatzl

hotspot/src/share/vm/gc_implementation/g1/g1CollectorPolicy.cpp

@@ -139,6 +139,8 @@ G1CollectorPolicy::G1CollectorPolicy() :
   _survivor_cset_region_length(0),
   _old_cset_region_length(0),
 
+  _sigma(G1ConfidencePercent / 100.0),
+
   _collection_set(NULL),
   _collection_set_bytes_used_before(0),
 
@@ -161,17 +163,8 @@ G1CollectorPolicy::G1CollectorPolicy() :
 
   _gc_overhead_perc(0.0) {
 
-  uintx confidence_perc = G1ConfidencePercent;
+  // SurvRateGroups below must be initialized after '_sigma' because they
-  // Put an artificial ceiling on this so that it's not set to a silly value.
+  // indirectly access '_sigma' through this object passed to their constructor.
-  if (confidence_perc > 100) {
-    confidence_perc = 100;
-    warning("G1ConfidencePercent is set to a value that is too large, "
-            "it's been updated to %u", confidence_perc);
-  }
-  // '_sigma' must be initialized before the SurvRateGroups below because they
-  // indirecty access '_sigma' trough the 'this' pointer in their constructor.
-  _sigma = (double) confidence_perc / 100.0;
-
   _short_lived_surv_rate_group =
     new SurvRateGroup(this, "Short Lived", G1YoungSurvRateNumRegionsSummary);
   _survivor_surv_rate_group =

hotspot/src/share/vm/runtime/arguments.cpp

@@ -2316,6 +2316,7 @@ bool Arguments::check_vm_args_consistency() {
     status = status && verify_percentage(G1MaxNewSizePercent, "G1MaxNewSizePercent");
     status = status && verify_interval(G1NewSizePercent, 0, G1MaxNewSizePercent, "G1NewSizePercent");
 
+    status = status && verify_percentage(G1ConfidencePercent, "G1ConfidencePercent");
     status = status && verify_percentage(InitiatingHeapOccupancyPercent,
                                          "InitiatingHeapOccupancyPercent");
     status = status && verify_min_value(G1RefProcDrainInterval, 1,

hotspot/test/TEST.groups

@@ -224,6 +224,7 @@ needs_g1gc = \
   gc/arguments/TestAlignmentToUseLargePages.java \
   gc/arguments/TestG1HeapRegionSize.java \
   gc/arguments/TestG1HeapSizeFlags.java \
+  gc/arguments/TestG1PercentageOptions.java \
   gc/arguments/TestMaxHeapSizeTools.java \
   gc/arguments/TestMaxNewSize.java \
   gc/arguments/TestParallelGCThreads.java \

hotspot/test/gc/arguments/TestG1PercentageOptions.java

@@ -0,0 +1,89 @@
+/*
+* Copyright (c) 2015, 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 TestG1PercentageOptions
+ * @key gc
+ * @bug 8068942
+ * @summary Test argument processing of various percentage options
+ * @library /testlibrary
+ * @run driver TestG1PercentageOptions
+ */
+
+import com.oracle.java.testlibrary.*;
+
+public class TestG1PercentageOptions {
+
+    private static final class OptionDescription {
+        public final String name;
+        public final String[] valid;
+        public final String[] invalid;
+
+        OptionDescription(String name_, String[] valid_, String[] invalid_) {
+            name = name_;
+            valid = valid_;
+            invalid = invalid_;
+        }
+    }
+
+    private static final String[] defaultValid = new String[] {
+        "0", "1", "50", "95", "100" };
+    private static final String[] defaultInvalid = new String[] {
+        "-10", "110", "bad" };
+
+    // All of the G1 product arguments that are percentages.
+    private static final OptionDescription[] percentOptions = new OptionDescription[] {
+        new OptionDescription("G1ConfidencePercent", defaultValid, defaultInvalid)
+        // Other percentage options are not yet validated by argument processing.
+    };
+
+    private static void check(String flag, boolean is_valid) throws Exception {
+        String[] flags = new String[] { "-XX:+UseG1GC", flag, "-version" };
+        ProcessBuilder pb = ProcessTools.createJavaProcessBuilder(flags);
+        OutputAnalyzer output = new OutputAnalyzer(pb.start());
+        if (is_valid) {
+            output.shouldHaveExitValue(0);
+        } else {
+            output.shouldHaveExitValue(1);
+        }
+    }
+
+    private static
+    void check(String name, String value, boolean is_valid) throws Exception {
+        check("-XX:" + name + "=" + value, is_valid);
+    }
+
+    public static void main(String args[]) throws Exception {
+        for (OptionDescription option : percentOptions) {
+            for (String value : option.valid) {
+                check(option.name, value, true);
+            }
+            for (String value : option.invalid) {
+                check(option.name, value, false);
+            }
+            check("-XX:" + option.name, false);
+            check("-XX:+" + option.name, false);
+            check("-XX:-" + option.name, false);
+        }
+    }
+}