Command Line Runner accepts negative int arguments

Vladimir Sitnikov sitnikov.vladimir at gmail.com
Thu May 21 15:47:12 UTC 2015


Alexey,

Do you think "batch size" could be equal to 0?
What sense does it make?

org.openjdk.jmh.it.batchsize.SingleShotBatchApi05Test tries to use
warmupBatchSize(0), so I'm somewhat confused if that is a legal use.

Please check the attached patch.

Vladimir
-------------- next part --------------
# HG changeset patch
# User Vladimir Sitnikov <sitnikov.vladimir at gmail.com>
# Date 1432223131 -10800
#      Thu May 21 18:45:31 2015 +0300
# Node ID 13ffd1b8c9a1e5c90174d64189009e555b6fc7c2
# Parent  16f9d77c22333bd6359589af257d5d09a0005ee5
Improve validation of command line options (like threads being positive) and options from OptionsBuilder

diff -r 16f9d77c2233 -r 13ffd1b8c9a1 jmh-core-it/src/test/java/org/openjdk/jmh/it/batchsize/SingleShotBatchApi05Test.java
--- a/jmh-core-it/src/test/java/org/openjdk/jmh/it/batchsize/SingleShotBatchApi05Test.java	Fri May 15 13:25:25 2015 +0300
+++ b/jmh-core-it/src/test/java/org/openjdk/jmh/it/batchsize/SingleShotBatchApi05Test.java	Thu May 21 18:45:31 2015 +0300
@@ -50,7 +50,7 @@
 
     private static final int WARMUP_ITERATIONS = 2;
     private static final int MEASUREMENT_ITERATIONS = 1;
-    private static final int WARMUP_BATCH = 0;
+    private static final int WARMUP_BATCH = 1;
     private static final int MEASUREMENT_BATCH = 5;
 
     private final AtomicInteger iterationCount = new AtomicInteger();
diff -r 16f9d77c2233 -r 13ffd1b8c9a1 jmh-core/src/main/java/org/openjdk/jmh/runner/options/CommandLineOptions.java
--- a/jmh-core/src/main/java/org/openjdk/jmh/runner/options/CommandLineOptions.java	Fri May 15 13:25:25 2015 +0300
+++ b/jmh-core/src/main/java/org/openjdk/jmh/runner/options/CommandLineOptions.java	Thu May 21 18:45:31 2015 +0300
@@ -28,8 +28,8 @@
 import joptsimple.OptionParser;
 import joptsimple.OptionSet;
 import joptsimple.OptionSpec;
+import joptsimple.ValueConversionException;
 import org.openjdk.jmh.annotations.Mode;
-import org.openjdk.jmh.annotations.Threads;
 import org.openjdk.jmh.profile.Profiler;
 import org.openjdk.jmh.profile.ProfilerFactory;
 import org.openjdk.jmh.results.format.ResultFormatType;
@@ -96,43 +96,43 @@
         parser.formatHelpWith(new OptionFormatter());
 
         OptionSpec<Integer> optMeasureCount = parser.accepts("i", "Number of measurement iterations to do.")
-                .withRequiredArg().ofType(Integer.class).describedAs("int");
+                .withRequiredArg().withValuesConvertedBy(IntegerValueConverter.POSITIVE).describedAs("int");
 
         OptionSpec<Integer> optMeasureBatchSize = parser.accepts("bs", "Batch size: number of benchmark method calls per operation. " +
                 "(some benchmark modes can ignore this setting)")
-                .withRequiredArg().ofType(Integer.class).describedAs("int");
+                .withRequiredArg().withValuesConvertedBy(IntegerValueConverter.POSITIVE).describedAs("int");
 
-        OptionSpec<String> optMeasureTime = parser.accepts("r", "Time to spend at each measurement iteration.")
-                .withRequiredArg().ofType(String.class).describedAs("time");
+        OptionSpec<TimeValue> optMeasureTime = parser.accepts("r", "Time to spend at each measurement iteration.")
+                .withRequiredArg().ofType(TimeValue.class).describedAs("time");
 
         OptionSpec<Integer> optWarmupCount = parser.accepts("wi", "Number of warmup iterations to do.")
-                .withRequiredArg().ofType(Integer.class).describedAs("int");
+                .withRequiredArg().withValuesConvertedBy(IntegerValueConverter.NON_NEGATIVE).describedAs("int");
 
         OptionSpec<Integer> optWarmupBatchSize = parser.accepts("wbs", "Warmup batch size: number of benchmark method calls per operation. " +
                 "(some benchmark modes can ignore this setting)")
-                .withRequiredArg().ofType(Integer.class).describedAs("int");
+                .withRequiredArg().withValuesConvertedBy(IntegerValueConverter.POSITIVE).describedAs("int");
 
-        OptionSpec<String> optWarmupTime = parser.accepts("w", "Time to spend at each warmup iteration.")
-                .withRequiredArg().ofType(String.class).describedAs("time");
+        OptionSpec<TimeValue> optWarmupTime = parser.accepts("w", "Time to spend at each warmup iteration.")
+                .withRequiredArg().ofType(TimeValue.class).describedAs("time");
 
-        OptionSpec<String> optTimeoutTime = parser.accepts("to", "Timeout for benchmark iteration.")
-                .withRequiredArg().ofType(String.class).describedAs("time");
+        OptionSpec<TimeValue> optTimeoutTime = parser.accepts("to", "Timeout for benchmark iteration.")
+                .withRequiredArg().ofType(TimeValue.class).describedAs("time");
 
-        OptionSpec<String> optThreads = parser.accepts("t", "Number of worker threads to run with.")
-                .withRequiredArg().ofType(String.class).describedAs("int");
+        OptionSpec<Integer> optThreads = parser.accepts("t", "Number of worker threads to run with. Positive integer or 'max' meaning Runtime.getRuntime().availableProcessors()")
+                .withRequiredArg().withValuesConvertedBy(ThreadsValueConverter.INSTANCE).describedAs("int");
 
         OptionSpec<String> optBenchmarkMode = parser.accepts("bm", "Benchmark mode. Available modes are: " + Mode.getKnown())
                 .withRequiredArg().ofType(String.class).withValuesSeparatedBy(',').describedAs("mode");
 
         OptionSpec<Boolean> optSyncIters = parser.accepts("si", "Synchronize iterations?")
-                .withOptionalArg().ofType(Boolean.class).describedAs("bool");
+                .withOptionalArg().ofType(Boolean.class).describedAs("bool").defaultsTo(true);
 
         OptionSpec<Boolean> optGC = parser.accepts("gc", "Should JMH force GC between iterations?")
-                .withOptionalArg().ofType(Boolean.class).describedAs("bool");
+                .withOptionalArg().ofType(Boolean.class).describedAs("bool").defaultsTo(true);
 
         OptionSpec<Boolean> optFOE = parser.accepts("foe", "Should JMH fail immediately if any benchmark had" +
                 " experienced the unrecoverable error?")
-                .withOptionalArg().ofType(Boolean.class).describedAs("bool");
+                .withOptionalArg().ofType(Boolean.class).describedAs("bool").defaultsTo(true);
 
         OptionSpec<String> optVerboseMode = parser.accepts("v", "Verbosity mode. Available modes are: " + Arrays.toString(VerboseMode.values()))
                 .withRequiredArg().ofType(String.class).describedAs("mode");
@@ -144,11 +144,11 @@
                 " Use 0 to disable forking altogether (WARNING: disabling forking may have detrimental" +
                 " impact on benchmark and infrastructure reliability, you might want to use different" +
                 " warmup mode instead).")
-                .withOptionalArg().ofType(Integer.class).describedAs("int");
+                .withOptionalArg().withValuesConvertedBy(IntegerValueConverter.NON_NEGATIVE).describedAs("int").defaultsTo(1);
 
         OptionSpec<Integer> optWarmupForks = parser.accepts("wf", "How many warmup forks to make " +
                 "for a single benchmark. 0 to disable warmup forks.")
-                .withRequiredArg().ofType(Integer.class).describedAs("int");
+                .withRequiredArg().withValuesConvertedBy(IntegerValueConverter.NON_NEGATIVE).describedAs("int");
 
         OptionSpec<String> optOutput = parser.accepts("o", "Redirect human-readable output to file.")
                 .withRequiredArg().ofType(String.class).describedAs("filename");
@@ -161,7 +161,8 @@
                 .withRequiredArg().withValuesSeparatedBy(',').ofType(String.class).describedAs("profiler+");
 
         OptionSpec<Integer> optThreadGroups = parser.accepts("tg", "Override thread group distribution for asymmetric benchmarks.")
-                .withRequiredArg().withValuesSeparatedBy(',').ofType(Integer.class).describedAs("int+");
+                .withRequiredArg().withValuesSeparatedBy(',').ofType(Integer.class)
+                .withValuesConvertedBy(IntegerValueConverter.NON_NEGATIVE).describedAs("int+");
 
         OptionSpec<String> optJvm = parser.accepts("jvm", "Custom JVM to use when forking (path to JVM executable).")
                 .withRequiredArg().ofType(String.class).describedAs("string");
@@ -179,7 +180,7 @@
                 .withRequiredArg().ofType(String.class).describedAs("TU");
 
         OptionSpec<Integer> optOPI = parser.accepts("opi", "Operations per invocation.")
-                .withRequiredArg().ofType(Integer.class).describedAs("int");
+                .withRequiredArg().withValuesConvertedBy(IntegerValueConverter.POSITIVE).describedAs("int");
 
         OptionSpec<String> optResultFormat = parser.accepts("rf", "Result format type. See the list of available result formats first.")
                 .withRequiredArg().ofType(String.class).describedAs("type");
@@ -239,7 +240,7 @@
                 timeUnit = Optional.none();
             }
 
-            opsPerInvocation = Optional.eitherOf(optOPI.value(set));
+            opsPerInvocation = toOptional(optOPI, set);
 
             if (set.has(optWarmupMode)) {
                 try {
@@ -266,61 +267,21 @@
             listResultFormats = set.has("lrf");
             listProfilers = set.has("lprof");
 
-            iterations = Optional.eitherOf(optMeasureCount.value(set));
+            iterations = toOptional(optMeasureCount, set);
 
-            batchSize = Optional.eitherOf(optMeasureBatchSize.value(set));
+            batchSize = toOptional(optMeasureBatchSize, set);
 
-            if (set.has(optMeasureTime)) {
-                String value = optMeasureTime.value(set);
-                try {
-                    runTime = Optional.of(TimeValue.fromString(value));
-                } catch (IllegalArgumentException iae) {
-                    throw new CommandLineOptionException(iae.getMessage(), iae);
-                }
-            } else {
-                runTime = Optional.none();
-            }
+            runTime = toOptional(optMeasureTime, set);
 
-            warmupIterations = Optional.eitherOf(optWarmupCount.value(set));
+            warmupIterations = toOptional(optWarmupCount, set);
 
-            warmupBatchSize = Optional.eitherOf(optWarmupBatchSize.value(set));
+            warmupBatchSize = toOptional(optWarmupBatchSize, set);
 
-            if (set.has(optWarmupTime)) {
-                String value = optWarmupTime.value(set);
-                try {
-                    warmupTime = Optional.of(TimeValue.fromString(value));
-                } catch (IllegalArgumentException iae) {
-                    throw new CommandLineOptionException(iae.getMessage(), iae);
-                }
-            } else {
-                warmupTime = Optional.none();
-            }
+            warmupTime = toOptional(optWarmupTime, set);
 
-            if (set.has(optTimeoutTime)) {
-                String value = optTimeoutTime.value(set);
-                try {
-                    timeout = Optional.of(TimeValue.fromString(value));
-                } catch (IllegalArgumentException iae) {
-                    throw new CommandLineOptionException(iae.getMessage(), iae);
-                }
-            } else {
-                timeout = Optional.none();
-            }
+            timeout = toOptional(optTimeoutTime, set);
 
-            if (set.has(optThreads)) {
-                String v = optThreads.value(set);
-                if (v.equalsIgnoreCase("max")) {
-                    threads = Optional.of(Threads.MAX);
-                } else {
-                    try {
-                        threads = Optional.of(Integer.valueOf(v));
-                    } catch (IllegalArgumentException iae) {
-                        throw new CommandLineOptionException(iae.getMessage(), iae);
-                    }
-                }
-            } else {
-                threads = Optional.none();
-            }
+            threads = toOptional(optThreads, set);
 
             if (set.has(optBenchmarkMode)) {
                 try {
@@ -334,35 +295,11 @@
                 }
             }
 
-            if (set.has(optSyncIters)) {
-                if (set.hasArgument(optSyncIters)) {
-                    synchIterations = Optional.of(optSyncIters.value(set));
-                } else {
-                    synchIterations = Optional.of(true);
-                }
-            } else {
-                synchIterations = Optional.none();
-            }
+            synchIterations = toOptional(optSyncIters, set);
 
-            if (set.has(optGC)) {
-                if (set.hasArgument(optGC)) {
-                    gcEachIteration = Optional.of(optGC.value(set));
-                } else {
-                    gcEachIteration = Optional.of(true);
-                }
-            } else {
-                gcEachIteration = Optional.none();
-            }
+            gcEachIteration = toOptional(optGC, set);
 
-            if (set.has(optFOE)) {
-                if (set.hasArgument(optFOE)) {
-                    failOnError = Optional.of(optFOE.value(set));
-                } else {
-                    failOnError = Optional.of(true);
-                }
-            } else {
-                failOnError = Optional.none();
-            }
+            failOnError = toOptional(optFOE, set);
 
             if (set.has(optVerboseMode)) {
                 try {
@@ -380,19 +317,11 @@
 
             regexps.addAll(set.valuesOf(optArgs));
 
-            if (set.has(optForks)) {
-                if (set.hasArgument(optForks)) {
-                    fork = Optional.of(optForks.value(set));
-                } else {
-                    fork = Optional.of(1);
-                }
-            } else {
-                fork = Optional.none();
-            }
+            fork = toOptional(optForks, set);
 
-            warmupFork = Optional.eitherOf(optWarmupForks.value(set));
-            output = Optional.eitherOf(optOutput.value(set));
-            result = Optional.eitherOf(optOutputResults.value(set));
+            warmupFork = toOptional(optWarmupForks, set);
+            output = toOptional(optOutput, set);
+            result = toOptional(optOutputResults, set);
 
             if (set.has(optProfilers)) {
                 try {
@@ -410,9 +339,16 @@
 
             if (set.has(optThreadGroups)) {
                 threadGroups.addAll(set.valuesOf(optThreadGroups));
+                int total = 0;
+                for (Integer group : threadGroups) {
+                    total += group;
+                }
+                if (total <= 0) {
+                    throw new CommandLineOptionException("Total share of all thread groups should be positive. Actual sum is " + total);
+                }
             }
 
-            jvm = Optional.eitherOf(optJvm.value(set));
+            jvm = toOptional(optJvm, set);
 
             jvmArgs = treatQuoted(set, optJvmArgs);
             jvmArgsAppend = treatQuoted(set, optJvmArgsAppend);
@@ -429,10 +365,23 @@
             }
 
         } catch (OptionException e) {
-            throw new CommandLineOptionException(e.getMessage(), e);
+            String message = e.getMessage();
+            Throwable cause = e.getCause();
+            // Add something like "The given value 0 should be positive"
+            if (cause instanceof ValueConversionException) {
+                message += ". " + cause.getMessage();
+            }
+            throw new CommandLineOptionException(message, e);
         }
     }
 
+    private static <T> Optional<T> toOptional(OptionSpec<T> option, OptionSet set) {
+        if (set.has(option)) {
+            return Optional.eitherOf(option.value(set));
+        }
+        return Optional.none();
+    }
+
     public Optional<Collection<String>> treatQuoted(OptionSet set, OptionSpec<String> spec) {
         if (set.hasArgument(spec)) {
             try {
diff -r 16f9d77c2233 -r 13ffd1b8c9a1 jmh-core/src/main/java/org/openjdk/jmh/runner/options/IntegerValueConverter.java
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/jmh-core/src/main/java/org/openjdk/jmh/runner/options/IntegerValueConverter.java	Thu May 21 18:45:31 2015 +0300
@@ -0,0 +1,75 @@
+/*
+ * Copyright (c) 2014, 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.  Oracle designates this
+ * particular file as subject to the "Classpath" exception as provided
+ * by Oracle in the LICENSE file that accompanied this code.
+ *
+ * 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.
+ */
+package org.openjdk.jmh.runner.options;
+
+import joptsimple.ValueConversionException;
+import joptsimple.ValueConverter;
+import joptsimple.internal.Reflection;
+
+/**
+ * Converts option value from {@link String} to {@link Integer} and makes sure the value exceeds given minimal threshold.
+ */
+public class IntegerValueConverter implements ValueConverter<Integer> {
+    private final static ValueConverter<Integer> TO_INT_CONVERTER = Reflection.findConverter(int.class);
+
+    public final static IntegerValueConverter POSITIVE = new IntegerValueConverter(1);
+    public final static IntegerValueConverter NON_NEGATIVE = new IntegerValueConverter(0);
+
+    private final int minValue;
+
+    public IntegerValueConverter(int minValue) {
+        this.minValue = minValue;
+    }
+
+    @Override
+    public Integer convert(String value) {
+        Integer newValue = TO_INT_CONVERTER.convert(value);
+        if (newValue == null) {
+            // should not get here
+            throw new ValueConversionException("value should not be null");
+        }
+
+        if (newValue < minValue) {
+            String message = "The given value " + value + " should be ";
+            if (minValue == 1) {
+                message += "positive";
+            } else {
+                message += "greater or equal than " + minValue;
+            }
+            throw new ValueConversionException(message);
+        }
+        return newValue;
+    }
+
+    @Override
+    public Class<Integer> valueType() {
+        return TO_INT_CONVERTER.valueType();
+    }
+
+    @Override
+    public String valuePattern() {
+        return "int";
+    }
+}
diff -r 16f9d77c2233 -r 13ffd1b8c9a1 jmh-core/src/main/java/org/openjdk/jmh/runner/options/OptionsBuilder.java
--- a/jmh-core/src/main/java/org/openjdk/jmh/runner/options/OptionsBuilder.java	Fri May 15 13:25:25 2015 +0300
+++ b/jmh-core/src/main/java/org/openjdk/jmh/runner/options/OptionsBuilder.java	Thu May 21 18:45:31 2015 +0300
@@ -25,11 +25,13 @@
 package org.openjdk.jmh.runner.options;
 
 import org.openjdk.jmh.annotations.Mode;
+import org.openjdk.jmh.annotations.Threads;
 import org.openjdk.jmh.profile.Profiler;
 import org.openjdk.jmh.results.format.ResultFormatType;
 import org.openjdk.jmh.util.HashMultimap;
 import org.openjdk.jmh.util.Multimap;
 import org.openjdk.jmh.util.Optional;
+import org.openjdk.jmh.util.Utils;
 
 import java.lang.management.ManagementFactory;
 import java.util.ArrayList;
@@ -57,6 +59,19 @@
         return this;
     }
 
+    private static void assertGreaterOrEqual(int value, int minValue, String s) {
+        if (value >= minValue) {
+            return;
+        }
+        String message = s + " (" + value + ") should be ";
+        if (minValue == 1) {
+            message += "positive";
+        } else {
+            message += "greater or equal than " + minValue;
+        }
+        throw new IllegalArgumentException(message);
+    }
+
     // ---------------------------------------------------------------------------
 
     private final List<String> regexps = new ArrayList<String>();
@@ -239,6 +254,9 @@
 
     @Override
     public ChainedOptionsBuilder threads(int count) {
+        if (count != Threads.MAX) {
+            assertGreaterOrEqual(count, 1, "Threads");
+        }
         this.threads = Optional.of(count);
         return this;
     }
@@ -258,7 +276,13 @@
 
     @Override
     public ChainedOptionsBuilder threadGroups(int... groups) {
-        this.threadGroups = Optional.of(groups);
+        if (groups != null) {
+            for (int i = 0; i < groups.length; i++) {
+                assertGreaterOrEqual(groups[i], 0, "Group #" + i + " share");
+            }
+            assertGreaterOrEqual(Utils.sum(groups), 1, "Total share of all the groups");
+        }
+        this.threadGroups = Optional.of(groups == null || groups.length != 0 ? groups : null);
         return this;
     }
 
@@ -296,6 +320,7 @@
 
     @Override
     public ChainedOptionsBuilder warmupIterations(int value) {
+        assertGreaterOrEqual(value, 0, "Warmup iterations");
         this.warmupIterations = Optional.of(value);
         return this;
     }
@@ -315,6 +340,7 @@
 
     @Override
     public ChainedOptionsBuilder warmupBatchSize(int value) {
+        assertGreaterOrEqual(value, 1, "Warmup batch size");
         this.warmupBatchSize = Optional.of(value);
         return this;
     }
@@ -392,6 +418,7 @@
 
     @Override
     public ChainedOptionsBuilder measurementIterations(int count) {
+        assertGreaterOrEqual(count, 1, "Measurement iterations");
         this.iterations = Optional.of(count);
         return this;
     }
@@ -430,6 +457,7 @@
 
     @Override
     public ChainedOptionsBuilder measurementBatchSize(int value) {
+        assertGreaterOrEqual(value, 1, "Measurement batch size");
         this.measurementBatchSize = Optional.of(value);
         return this;
     }
@@ -488,6 +516,7 @@
 
     @Override
     public ChainedOptionsBuilder operationsPerInvocation(int opsPerInv) {
+        assertGreaterOrEqual(opsPerInv, 1, "Operations per invocation");
         this.opsPerInvocation = Optional.of(opsPerInv);
         return this;
     }
@@ -507,6 +536,7 @@
 
     @Override
     public ChainedOptionsBuilder forks(int value) {
+        assertGreaterOrEqual(value, 0, "Forks");
         this.forks = Optional.of(value);
         return this;
     }
@@ -526,6 +556,7 @@
 
     @Override
     public ChainedOptionsBuilder warmupForks(int value) {
+        assertGreaterOrEqual(value, 0, "Warmup forks");
         this.warmupForks = Optional.of(value);
         return this;
     }
diff -r 16f9d77c2233 -r 13ffd1b8c9a1 jmh-core/src/main/java/org/openjdk/jmh/runner/options/ThreadsValueConverter.java
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/jmh-core/src/main/java/org/openjdk/jmh/runner/options/ThreadsValueConverter.java	Thu May 21 18:45:31 2015 +0300
@@ -0,0 +1,53 @@
+/*
+ * Copyright (c) 2014, 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.  Oracle designates this
+ * particular file as subject to the "Classpath" exception as provided
+ * by Oracle in the LICENSE file that accompanied this code.
+ *
+ * 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.
+ */
+package org.openjdk.jmh.runner.options;
+
+import joptsimple.ValueConverter;
+import org.openjdk.jmh.annotations.Threads;
+
+/**
+ * Converts {@link String} value to {@link Integer} and uses {@link Threads#MAX} if {@code max} string was given.
+ */
+public class ThreadsValueConverter implements ValueConverter<Integer> {
+    public static final ValueConverter<Integer> INSTANCE = new ThreadsValueConverter();
+
+    @Override
+    public Integer convert(String value) {
+        if (value.equalsIgnoreCase("max")) {
+            return Threads.MAX;
+        }
+        return IntegerValueConverter.POSITIVE.convert(value);
+    }
+
+    @Override
+    public Class<Integer> valueType() {
+        return IntegerValueConverter.POSITIVE.valueType();
+    }
+
+    @Override
+    public String valuePattern() {
+        return IntegerValueConverter.POSITIVE.valuePattern();
+    }
+}
diff -r 16f9d77c2233 -r 13ffd1b8c9a1 jmh-core/src/main/java/org/openjdk/jmh/runner/options/TimeValue.java
--- a/jmh-core/src/main/java/org/openjdk/jmh/runner/options/TimeValue.java	Fri May 15 13:25:25 2015 +0300
+++ b/jmh-core/src/main/java/org/openjdk/jmh/runner/options/TimeValue.java	Thu May 21 18:45:31 2015 +0300
@@ -151,6 +151,16 @@
         }
     }
 
+    /**
+     * Parses time value from a string representation.
+     * This method is called by joptsimple to resolve string values.
+     * @param timeString string representation of a time value
+     * @return TimeValue value
+     */
+    public static TimeValue valueOf(String timeString) {
+        return fromString(timeString);
+    }
+
     public static TimeValue fromString(String timeString) {
         if (timeString == null) {
             throw new IllegalArgumentException("String is null");
diff -r 16f9d77c2233 -r 13ffd1b8c9a1 jmh-core/src/test/java/org/openjdk/jmh/runner/options/TestOptions.java
--- a/jmh-core/src/test/java/org/openjdk/jmh/runner/options/TestOptions.java	Fri May 15 13:25:25 2015 +0300
+++ b/jmh-core/src/test/java/org/openjdk/jmh/runner/options/TestOptions.java	Thu May 21 18:45:31 2015 +0300
@@ -256,6 +256,46 @@
     }
 
     @Test
+    public void testThreads_Zero() throws Exception {
+        try {
+            new CommandLineOptions("-t", "0");
+            Assert.fail();
+        } catch (CommandLineOptionException e) {
+            Assert.assertEquals("Cannot parse argument '0' of option ['t']. The given value 0 should be positive", e.getMessage());
+        }
+    }
+
+    @Test
+    public void testThreads_Zero_OptionsBuilder() throws Exception {
+        try {
+            new OptionsBuilder().threads(0);
+            Assert.fail();
+        } catch (IllegalArgumentException e) {
+            Assert.assertEquals("Threads (0) should be positive", e.getMessage());
+        }
+    }
+
+    @Test
+    public void testThreads_MinusOne() throws Exception {
+        try {
+            new CommandLineOptions("-t", "-1");
+            Assert.fail();
+        } catch (CommandLineOptionException e) {
+            Assert.assertEquals("Cannot parse argument '-1' of option ['t']. The given value -1 should be positive", e.getMessage());
+        }
+    }
+
+    @Test(expected = CommandLineOptionException.class)
+    public void testThreads_Minus42() throws Exception {
+        new CommandLineOptions("-t", "-42");
+    }
+
+    @Test(expected = IllegalArgumentException.class)
+    public void testThreads_Minus42_OptionsBuilder() throws Exception {
+        new OptionsBuilder().threads(-42);
+    }
+
+    @Test
     public void testThreads_Default() throws Exception {
         Assert.assertEquals(EMPTY_BUILDER.getThreads(), EMPTY_CMDLINE.getThreads());
     }
@@ -273,6 +313,53 @@
     }
 
     @Test
+    public void testThreadGroups_WithZero() throws Exception {
+        CommandLineOptions cmdLine = new CommandLineOptions("-tg", "3,4,0");
+        Options builder = new OptionsBuilder().threadGroups(3, 4, 0).build();
+        Assert.assertEquals(builder.getThreads(), cmdLine.getThreads());
+    }
+
+    @Test
+    public void testThreadGroups_AllZero() throws Exception {
+        try {
+            new CommandLineOptions("-tg", "0,0,0");
+            Assert.fail();
+        } catch (CommandLineOptionException e) {
+            Assert.assertEquals("Total share of all thread groups should be positive. Actual sum is 0", e.getMessage());
+        }
+    }
+
+    @Test
+    public void testThreadGroups_AllZero_OptionsBuilder() throws Exception {
+        try {
+            new OptionsBuilder().threadGroups(0, 0, 0);
+            Assert.fail();
+        } catch (IllegalArgumentException e) {
+            Assert.assertEquals("Total share of all the groups (0) should be positive", e.getMessage());
+        }
+    }
+
+    @Test
+    public void testThreadGroups_WithNegative() throws Exception {
+        try {
+            new CommandLineOptions("-tg", "-1,-2");
+            Assert.fail();
+        } catch (CommandLineOptionException e) {
+            Assert.assertEquals("Cannot parse argument '-1' of option ['tg']. The given value -1 should be greater or equal than 0", e.getMessage());
+        }
+    }
+
+    @Test
+    public void testThreadGroups_WithNegative_OptionsBuilder() throws Exception {
+        try {
+            new OptionsBuilder().threadGroups(-1,-2);
+            Assert.fail();
+        } catch (IllegalArgumentException e) {
+            Assert.assertEquals("Group #0 share (-1) should be greater or equal than 0", e.getMessage());
+        }
+    }
+
+    @Test
     public void testSynchIterations_Set() throws Exception {
         CommandLineOptions cmdLine = new CommandLineOptions("-si");
         Options builder = new OptionsBuilder().syncIterations(true).build();
@@ -311,6 +398,33 @@
     }
 
     @Test
+    public void testWarmupIterations_Zero() throws Exception {
+        CommandLineOptions cmdLine = new CommandLineOptions("-wi", "0");
+        Options builder = new OptionsBuilder().warmupIterations(0).build();
+        Assert.assertEquals(builder.getWarmupIterations(), cmdLine.getWarmupIterations());
+    }
+
+    @Test
+    public void testWarmupIterations_MinusOne() throws Exception {
+        try {
+            new CommandLineOptions("-wi", "-1");
+            Assert.fail();
+        } catch (CommandLineOptionException e) {
+            Assert.assertEquals("Cannot parse argument '-1' of option ['wi']. The given value -1 should be greater or equal than 0", e.getMessage());
+        }
+    }
+
+    @Test
+    public void testWarmupIterations_MinusOne_OptionsBuilder() throws Exception {
+        try {
+            new OptionsBuilder().warmupIterations(-1);
+            Assert.fail();
+        } catch (IllegalArgumentException e) {
+            Assert.assertEquals("Warmup iterations (-1) should be greater or equal than 0", e.getMessage());
+        }
+    }
+
+    @Test
     public void testWarmupTime() throws Exception {
         CommandLineOptions cmdLine = new CommandLineOptions("-w", "34ms");
         Options builder = new OptionsBuilder().warmupTime(TimeValue.milliseconds(34)).build();
@@ -335,6 +449,26 @@
     }
 
     @Test
+    public void testRuntimeIterations_Zero() throws Exception {
+        try {
+            new CommandLineOptions("-i", "0");
+            Assert.fail();
+        } catch (CommandLineOptionException e) {
+            Assert.assertEquals("Cannot parse argument '0' of option ['i']. The given value 0 should be positive", e.getMessage());
+        }
+    }
+
+    @Test
+    public void testRuntimeIterations_Zero_OptionsBuilder() throws Exception {
+        try {
+            new OptionsBuilder().measurementIterations(0);
+            Assert.fail();
+        } catch (IllegalArgumentException e) {
+            Assert.assertEquals("Measurement iterations (0) should be positive", e.getMessage());
+        }
+    }
+
+    @Test
     public void testRuntime() throws Exception {
         CommandLineOptions cmdLine = new CommandLineOptions("-r", "34ms");
         Options builder = new OptionsBuilder().measurementTime(TimeValue.milliseconds(34)).build();
@@ -391,6 +525,26 @@
     }
 
     @Test
+    public void testOPI_Zero() throws Exception {
+        try {
+            new CommandLineOptions("-opi", "0");
+            Assert.fail();
+        } catch (CommandLineOptionException e) {
+            Assert.assertEquals("Cannot parse argument '0' of option ['opi']. The given value 0 should be positive", e.getMessage());
+        }
+    }
+
+    @Test
+    public void testOPI_Zero_OptionsBuilder() throws Exception {
+        try {
+            new OptionsBuilder().measurementIterations(0);
+            Assert.fail();
+        } catch (IllegalArgumentException e) {
+            Assert.assertEquals("Measurement iterations (0) should be positive", e.getMessage());
+        }
+    }
+
+    @Test
     public void testOPI_Default() throws Exception {
         Assert.assertEquals(EMPTY_BUILDER.getOperationsPerInvocation(), EMPTY_CMDLINE.getOperationsPerInvocation());
     }
@@ -422,6 +576,26 @@
     }
 
     @Test
+    public void testFork_MinusOne() throws Exception {
+        try {
+            new CommandLineOptions("-f", "-1");
+            Assert.fail();
+        } catch (CommandLineOptionException e) {
+            Assert.assertEquals("'1' is not a recognized option", e.getMessage());
+        }
+    }
+
+    @Test
+    public void testFork__MinusOne_OptionsBuilder() throws Exception {
+        try {
+            new OptionsBuilder().forks(-1);
+            Assert.fail();
+        } catch (IllegalArgumentException e) {
+            Assert.assertEquals("Forks (-1) should be greater or equal than 0", e.getMessage());
+        }
+    }
+
+    @Test
     public void testWarmupFork_0() throws Exception {
         CommandLineOptions cmdLine = new CommandLineOptions("-wf", "0");
         Options builder = new OptionsBuilder().warmupForks(0).build();
@@ -441,6 +615,26 @@
     }
 
     @Test
+    public void testWarmupFork_MinusOne() throws Exception {
+        try {
+            new CommandLineOptions("-wf", "-1");
+            Assert.fail();
+        } catch (CommandLineOptionException e) {
+            Assert.assertEquals("Cannot parse argument '-1' of option ['wf']. The given value -1 should be greater or equal than 0", e.getMessage());
+        }
+    }
+
+    @Test
+    public void testWarmupFork_MinusOne_OptionsBuilder() throws Exception {
+        try {
+            new OptionsBuilder().warmupForks(-1);
+            Assert.fail();
+        } catch (IllegalArgumentException e) {
+            Assert.assertEquals("Warmup forks (-1) should be greater or equal than 0", e.getMessage());
+        }
+    }
+
+    @Test
     public void testJvm() throws Exception {
         CommandLineOptions cmdLine = new CommandLineOptions("--jvm", "sample.jar");
         Options builder = new OptionsBuilder().jvm("sample.jar").build();
@@ -501,6 +695,26 @@
     }
 
     @Test
+    public void testBatchSize_Zero() throws Exception {
+        try {
+            new CommandLineOptions("-bs", "0");
+            Assert.fail();
+        } catch (CommandLineOptionException e) {
+            Assert.assertEquals("Cannot parse argument '0' of option ['bs']. The given value 0 should be positive", e.getMessage());
+        }
+    }
+
+    @Test
+    public void testBatchSize_Zero_OptionsBuilder() throws Exception {
+        try {
+            new OptionsBuilder().measurementBatchSize(0);
+            Assert.fail();
+        } catch (IllegalArgumentException e) {
+            Assert.assertEquals("Measurement batch size (0) should be positive", e.getMessage());
+        }
+    }
+
+    @Test
     public void testWarmupBatchSize() throws Exception {
         CommandLineOptions cmdLine = new CommandLineOptions("-wbs", "43");
         Options builder = new OptionsBuilder().warmupBatchSize(43).build();
@@ -513,6 +727,26 @@
     }
 
     @Test
+    public void testWarmupBatchSize_Zero() throws Exception {
+        try {
+            new CommandLineOptions("-wbs", "0");
+            Assert.fail();
+        } catch (CommandLineOptionException e) {
+            Assert.assertEquals("Cannot parse argument '0' of option ['wbs']. The given value 0 should be positive", e.getMessage());
+        }
+    }
+
+    @Test
+    public void testWarmupBatchSize_Zero_OptionsBuilder() throws Exception {
+        try {
+            new OptionsBuilder().warmupBatchSize(0);
+            Assert.fail();
+        } catch (IllegalArgumentException e) {
+            Assert.assertEquals("Warmup batch size (0) should be positive", e.getMessage());
+        }
+    }
+
+    @Test
     public void testParam_Default() {
         Assert.assertEquals(EMPTY_BUILDER.getParameter("sample"), EMPTY_CMDLINE.getParameter("sample"));
     }


More information about the jmh-dev mailing list