changeset 1230:10560d7513f4

7901431: Overhaul option range checking Contributed-by: Vladimir Sitnikov <sitnikov.vladimir@gmail.com>
author shade
date Fri, 22 May 2015 13:41:28 +0300
parents 32d02cd39320
children 3387560164f9
files jmh-core-it/src/test/java/org/openjdk/jmh/it/batchsize/SingleShotBatchApi05Test.java jmh-core/src/main/java/org/openjdk/jmh/runner/options/CommandLineOptions.java jmh-core/src/main/java/org/openjdk/jmh/runner/options/IntegerValueConverter.java jmh-core/src/main/java/org/openjdk/jmh/runner/options/OptionsBuilder.java jmh-core/src/main/java/org/openjdk/jmh/runner/options/ThreadsValueConverter.java jmh-core/src/main/java/org/openjdk/jmh/runner/options/TimeValue.java jmh-core/src/test/java/org/openjdk/jmh/runner/options/TestOptions.java
diffstat 7 files changed, 474 insertions(+), 165 deletions(-) [+]
line wrap: on
line diff
--- a/jmh-core-it/src/test/java/org/openjdk/jmh/it/batchsize/SingleShotBatchApi05Test.java	Fri May 22 11:57:06 2015 +0300
+++ b/jmh-core-it/src/test/java/org/openjdk/jmh/it/batchsize/SingleShotBatchApi05Test.java	Fri May 22 13:41:28 2015 +0300
@@ -26,14 +26,7 @@
 
 import org.junit.Assert;
 import org.junit.Test;
-import org.openjdk.jmh.annotations.Fork;
-import org.openjdk.jmh.annotations.Benchmark;
-import org.openjdk.jmh.annotations.Level;
-import org.openjdk.jmh.annotations.Mode;
-import org.openjdk.jmh.annotations.Scope;
-import org.openjdk.jmh.annotations.Setup;
-import org.openjdk.jmh.annotations.State;
-import org.openjdk.jmh.annotations.TearDown;
+import org.openjdk.jmh.annotations.*;
 import org.openjdk.jmh.it.Fixtures;
 import org.openjdk.jmh.runner.Runner;
 import org.openjdk.jmh.runner.RunnerException;
@@ -50,7 +43,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();
--- a/jmh-core/src/main/java/org/openjdk/jmh/runner/options/CommandLineOptions.java	Fri May 22 11:57:06 2015 +0300
+++ b/jmh-core/src/main/java/org/openjdk/jmh/runner/options/CommandLineOptions.java	Fri May 22 13:41:28 2015 +0300
@@ -24,12 +24,8 @@
  */
 package org.openjdk.jmh.runner.options;
 
-import joptsimple.OptionException;
-import joptsimple.OptionParser;
-import joptsimple.OptionSet;
-import joptsimple.OptionSpec;
+import joptsimple.*;
 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 +92,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. 'max' means 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");
+                .withRequiredArg().ofType(Boolean.class).describedAs("bool");
 
         OptionSpec<Boolean> optGC = parser.accepts("gc", "Should JMH force GC between iterations?")
-                .withOptionalArg().ofType(Boolean.class).describedAs("bool");
+                .withRequiredArg().ofType(Boolean.class).describedAs("bool");
 
         OptionSpec<Boolean> optFOE = parser.accepts("foe", "Should JMH fail immediately if any benchmark had" +
                 " experienced the unrecoverable error?")
-                .withOptionalArg().ofType(Boolean.class).describedAs("bool");
+                .withRequiredArg().ofType(Boolean.class).describedAs("bool");
 
         OptionSpec<String> optVerboseMode = parser.accepts("v", "Verbosity mode. Available modes are: " + Arrays.toString(VerboseMode.values()))
                 .withRequiredArg().ofType(String.class).describedAs("mode");
@@ -144,11 +140,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");
+                .withRequiredArg().withValuesConvertedBy(IntegerValueConverter.NON_NEGATIVE).describedAs("int");
 
         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 +157,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 +176,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 +236,7 @@
                 timeUnit = Optional.none();
             }
 
-            opsPerInvocation = Optional.eitherOf(optOPI.value(set));
+            opsPerInvocation = toOptional(optOPI, set);
 
             if (set.has(optWarmupMode)) {
                 try {
@@ -266,61 +263,21 @@
             listResultFormats = set.has("lrf");
             listProfilers = set.has("lprof");
 
-            iterations = Optional.eitherOf(optMeasureCount.value(set));
-
-            batchSize = Optional.eitherOf(optMeasureBatchSize.value(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();
-            }
-
-            warmupIterations = Optional.eitherOf(optWarmupCount.value(set));
-
-            warmupBatchSize = Optional.eitherOf(optWarmupBatchSize.value(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();
-            }
-
-            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();
-            }
-
-            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();
-            }
+            iterations = toOptional(optMeasureCount, set);
+            batchSize = toOptional(optMeasureBatchSize, set);
+            runTime = toOptional(optMeasureTime, set);
+            warmupIterations = toOptional(optWarmupCount, set);
+            warmupBatchSize = toOptional(optWarmupBatchSize, set);
+            warmupTime = toOptional(optWarmupTime, set);
+            timeout = toOptional(optTimeoutTime, set);
+            threads = toOptional(optThreads, set);
+            synchIterations = toOptional(optSyncIters, set);
+            gcEachIteration = toOptional(optGC, set);
+            failOnError = toOptional(optFOE, set);
+            fork = toOptional(optForks, set);
+            warmupFork = toOptional(optWarmupForks, set);
+            output = toOptional(optOutput, set);
+            result = toOptional(optOutputResults, set);
 
             if (set.has(optBenchmarkMode)) {
                 try {
@@ -334,36 +291,6 @@
                 }
             }
 
-            if (set.has(optSyncIters)) {
-                if (set.hasArgument(optSyncIters)) {
-                    synchIterations = Optional.of(optSyncIters.value(set));
-                } else {
-                    synchIterations = Optional.of(true);
-                }
-            } else {
-                synchIterations = Optional.none();
-            }
-
-            if (set.has(optGC)) {
-                if (set.hasArgument(optGC)) {
-                    gcEachIteration = Optional.of(optGC.value(set));
-                } else {
-                    gcEachIteration = Optional.of(true);
-                }
-            } else {
-                gcEachIteration = Optional.none();
-            }
-
-            if (set.has(optFOE)) {
-                if (set.hasArgument(optFOE)) {
-                    failOnError = Optional.of(optFOE.value(set));
-                } else {
-                    failOnError = Optional.of(true);
-                }
-            } else {
-                failOnError = Optional.none();
-            }
-
             if (set.has(optVerboseMode)) {
                 try {
                     if (set.hasArgument(optVerboseMode)) {
@@ -380,20 +307,6 @@
 
             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();
-            }
-
-            warmupFork = Optional.eitherOf(optWarmupForks.value(set));
-            output = Optional.eitherOf(optOutput.value(set));
-            result = Optional.eitherOf(optOutputResults.value(set));
-
             if (set.has(optProfilers)) {
                 try {
                     for (String m : optProfilers.values(set)) {
@@ -410,9 +323,16 @@
 
             if (set.has(optThreadGroups)) {
                 threadGroups.addAll(set.valuesOf(optThreadGroups));
+                int total = 0;
+                for (int group : threadGroups) {
+                    total += group;
+                }
+                if (total <= 0) {
+                    throw new CommandLineOptionException("Group thread count should be positive, but it is " + total);
+                }
             }
 
-            jvm = Optional.eitherOf(optJvm.value(set));
+            jvm = toOptional(optJvm, set);
 
             jvmArgs = treatQuoted(set, optJvmArgs);
             jvmArgsAppend = treatQuoted(set, optJvmArgsAppend);
@@ -429,10 +349,22 @@
             }
 
         } catch (OptionException e) {
-            throw new CommandLineOptionException(e.getMessage(), e);
+            String message = e.getMessage();
+            Throwable cause = e.getCause();
+            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 {
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/jmh-core/src/main/java/org/openjdk/jmh/runner/options/IntegerValueConverter.java	Fri May 22 13:41:28 2015 +0300
@@ -0,0 +1,77 @@
+/*
+ * 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 == 0) {
+                message += "non-negative";
+            } else 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";
+    }
+}
--- a/jmh-core/src/main/java/org/openjdk/jmh/runner/options/OptionsBuilder.java	Fri May 22 11:57:06 2015 +0300
+++ b/jmh-core/src/main/java/org/openjdk/jmh/runner/options/OptionsBuilder.java	Fri May 22 13:41:28 2015 +0300
@@ -25,18 +25,16 @@
 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;
-import java.util.Arrays;
-import java.util.Collection;
-import java.util.EnumSet;
-import java.util.List;
+import java.util.*;
 import java.util.concurrent.TimeUnit;
 
 public class OptionsBuilder implements Options, ChainedOptionsBuilder {
@@ -57,6 +55,21 @@
         return this;
     }
 
+    private static void checkGreaterOrEqual(int value, int minValue, String s) {
+        if (value >= minValue) {
+            return;
+        }
+        String message = s + " (" + value + ") should be ";
+        if (minValue == 0) {
+            message += "non-negative";
+        } else 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 +252,9 @@
 
     @Override
     public ChainedOptionsBuilder threads(int count) {
+        if (count != Threads.MAX) {
+            checkGreaterOrEqual(count, 1, "Threads");
+        }
         this.threads = Optional.of(count);
         return this;
     }
@@ -258,7 +274,13 @@
 
     @Override
     public ChainedOptionsBuilder threadGroups(int... groups) {
-        this.threadGroups = Optional.of(groups);
+        if (groups != null) {
+            for (int i = 0; i < groups.length; i++) {
+                checkGreaterOrEqual(groups[i], 0, "Group #" + i + " thread count");
+            }
+            checkGreaterOrEqual(Utils.sum(groups), 1, "Group thread count");
+        }
+        this.threadGroups = Optional.of((groups == null || groups.length != 0) ? groups : null);
         return this;
     }
 
@@ -296,6 +318,7 @@
 
     @Override
     public ChainedOptionsBuilder warmupIterations(int value) {
+        checkGreaterOrEqual(value, 0, "Warmup iterations");
         this.warmupIterations = Optional.of(value);
         return this;
     }
@@ -315,6 +338,7 @@
 
     @Override
     public ChainedOptionsBuilder warmupBatchSize(int value) {
+        checkGreaterOrEqual(value, 1, "Warmup batch size");
         this.warmupBatchSize = Optional.of(value);
         return this;
     }
@@ -392,6 +416,7 @@
 
     @Override
     public ChainedOptionsBuilder measurementIterations(int count) {
+        checkGreaterOrEqual(count, 1, "Measurement iterations");
         this.iterations = Optional.of(count);
         return this;
     }
@@ -430,6 +455,7 @@
 
     @Override
     public ChainedOptionsBuilder measurementBatchSize(int value) {
+        checkGreaterOrEqual(value, 1, "Measurement batch size");
         this.measurementBatchSize = Optional.of(value);
         return this;
     }
@@ -488,6 +514,7 @@
 
     @Override
     public ChainedOptionsBuilder operationsPerInvocation(int opsPerInv) {
+        checkGreaterOrEqual(opsPerInv, 1, "Operations per invocation");
         this.opsPerInvocation = Optional.of(opsPerInv);
         return this;
     }
@@ -507,6 +534,7 @@
 
     @Override
     public ChainedOptionsBuilder forks(int value) {
+        checkGreaterOrEqual(value, 0, "Forks");
         this.forks = Optional.of(value);
         return this;
     }
@@ -526,6 +554,7 @@
 
     @Override
     public ChainedOptionsBuilder warmupForks(int value) {
+        checkGreaterOrEqual(value, 0, "Warmup forks");
         this.warmupForks = Optional.of(value);
         return this;
     }
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/jmh-core/src/main/java/org/openjdk/jmh/runner/options/ThreadsValueConverter.java	Fri May 22 13:41:28 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();
+    }
+}
--- a/jmh-core/src/main/java/org/openjdk/jmh/runner/options/TimeValue.java	Fri May 22 11:57:06 2015 +0300
+++ b/jmh-core/src/main/java/org/openjdk/jmh/runner/options/TimeValue.java	Fri May 22 13:41:28 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");
--- a/jmh-core/src/test/java/org/openjdk/jmh/runner/options/TestOptions.java	Fri May 22 11:57:06 2015 +0300
+++ b/jmh-core/src/test/java/org/openjdk/jmh/runner/options/TestOptions.java	Fri May 22 13:41:28 2015 +0300
@@ -145,13 +145,6 @@
     }
 
     @Test
-    public void testGC_Set() throws Exception {
-        CommandLineOptions cmdLine = new CommandLineOptions("-gc");
-        Options builder = new OptionsBuilder().shouldDoGC(true).build();
-        Assert.assertEquals(builder.getOutput(), cmdLine.getOutput());
-    }
-
-    @Test
     public void testGC_True() throws Exception {
         CommandLineOptions cmdLine = new CommandLineOptions("-gc", "true");
         Options builder = new OptionsBuilder().shouldDoGC(true).build();
@@ -216,13 +209,6 @@
     }
 
     @Test
-    public void testSFOE_Set() throws Exception {
-        CommandLineOptions cmdLine = new CommandLineOptions("-foe");
-        Options builder = new OptionsBuilder().shouldFailOnError(true).build();
-        Assert.assertEquals(builder.shouldFailOnError(), cmdLine.shouldFailOnError());
-    }
-
-    @Test
     public void testSFOE_True() throws Exception {
         CommandLineOptions cmdLine = new CommandLineOptions("-foe", "true");
         Options builder = new OptionsBuilder().shouldFailOnError(true).build();
@@ -256,6 +242,55 @@
     }
 
     @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
+    public void testThreads_Minus42() throws Exception {
+        try {
+            new CommandLineOptions("-t", "-42");
+            Assert.fail();
+        } catch (CommandLineOptionException e) {
+            Assert.assertEquals("Cannot parse argument '-42' of option ['t']. The given value -42 should be positive", e.getMessage());
+        }
+    }
+
+    @Test
+    public void testThreads_Minus42_OptionsBuilder() throws Exception {
+        try {
+            new OptionsBuilder().threads(-42);
+        } catch (IllegalArgumentException e) {
+            Assert.assertEquals("Threads (-42) should be positive", e.getMessage());
+        }
+    }
+
+    @Test
     public void testThreads_Default() throws Exception {
         Assert.assertEquals(EMPTY_BUILDER.getThreads(), EMPTY_CMDLINE.getThreads());
     }
@@ -273,10 +308,50 @@
     }
 
     @Test
-    public void testSynchIterations_Set() throws Exception {
-        CommandLineOptions cmdLine = new CommandLineOptions("-si");
-        Options builder = new OptionsBuilder().syncIterations(true).build();
-        Assert.assertEquals(builder.shouldSyncIterations(), cmdLine.shouldSyncIterations());
+    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("Group thread count should be positive, but it 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("Group thread count (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 non-negative", 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 thread count (-1) should be non-negative", e.getMessage());
+        }
     }
 
     @Test
@@ -311,6 +386,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 non-negative", 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 non-negative", 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 +437,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,15 +513,28 @@
     }
 
     @Test
-    public void testOPI_Default() throws Exception {
-        Assert.assertEquals(EMPTY_BUILDER.getOperationsPerInvocation(), EMPTY_CMDLINE.getOperationsPerInvocation());
+    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 testFork() throws Exception {
-        CommandLineOptions cmdLine = new CommandLineOptions("-f");
-        Options builder = new OptionsBuilder().forks(1).build();
-        Assert.assertEquals(builder.getForkCount(), cmdLine.getForkCount());
+    public void testOPI_Zero_OptionsBuilder() throws Exception {
+        try {
+            new OptionsBuilder().operationsPerInvocation(0);
+            Assert.fail();
+        } catch (IllegalArgumentException e) {
+            Assert.assertEquals("Operations per invocation (0) should be positive", e.getMessage());
+        }
+    }
+
+    @Test
+    public void testOPI_Default() throws Exception {
+        Assert.assertEquals(EMPTY_BUILDER.getOperationsPerInvocation(), EMPTY_CMDLINE.getOperationsPerInvocation());
     }
 
     @Test
@@ -422,6 +557,26 @@
     }
 
     @Test
+    public void testFork_MinusOne() throws Exception {
+        try {
+            new CommandLineOptions("-f", "-1");
+            Assert.fail();
+        } catch (CommandLineOptionException e) {
+            Assert.assertEquals("Cannot parse argument '-1' of option ['f']. The given value -1 should be non-negative", 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 non-negative", e.getMessage());
+        }
+    }
+
+    @Test
     public void testWarmupFork_0() throws Exception {
         CommandLineOptions cmdLine = new CommandLineOptions("-wf", "0");
         Options builder = new OptionsBuilder().warmupForks(0).build();
@@ -441,6 +596,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 non-negative", 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 non-negative", 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 +676,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 +708,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"));
     }