RFR: fix the bug with failure when multispace is a separator in --jvmargs argument and similar annotations.

Aleksey Shipilev aleksey.shipilev at oracle.com
Fri Jun 21 07:12:12 PDT 2013


I like that suggestion better. Fixed Sergey's patch to use [ ]+, and
still committed the regression tests. Pushed.

Thanks everyone!
-Aleksey.

P.S. Ivan, please consider filling out OCA, as per:
 http://openjdk.java.net/contribute/

On 06/21/2013 05:28 PM, Ivan Gerasimov wrote:
> As the argument to .split() is a regexp, wouldn't it be better to use "[
> ]+" as a delimiter?
> 
> Sincerely,
> Ivan
> 
> On 21.06.2013 17:04, Sergey Kuksenko wrote:
>> fix the bug with failure when multiply spaces is a separator in
>> --jvmargs argument and similar annotations.
>> ----- patch starts here---------------------------
>> diff -r 9d5036308216
>> jmh-core-it/src/test/java/org/openjdk/jmh/it/fork/ForkedJvmArgsTest3.java
>> --- /dev/null    Thu Jan 01 00:00:00 1970 +0000
>> +++
>> b/jmh-core-it/src/test/java/org/openjdk/jmh/it/fork/ForkedJvmArgsTest3.java
>> Fri Jun 21 17:01:58 2013 +0400
>> @@ -0,0 +1,74 @@
>> +/**
>> + * Copyright (c) 2005, 2013, 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.it.fork;
>> +
>> +import org.junit.Assert;
>> +import org.junit.Test;
>> +import org.openjdk.jmh.Main;
>> +import org.openjdk.jmh.annotations.BenchmarkMode;
>> +import org.openjdk.jmh.annotations.Fork;
>> +import org.openjdk.jmh.annotations.GenerateMicroBenchmark;
>> +import org.openjdk.jmh.annotations.Measurement;
>> +import org.openjdk.jmh.annotations.Mode;
>> +import org.openjdk.jmh.annotations.Warmup;
>> +import org.openjdk.jmh.it.Fixtures;
>> +
>> +import java.util.concurrent.TimeUnit;
>> +
>> +/**
>> + * tests multiply spaces in jvmArgs annotations.
>> + * @author Sergey Kuksenko (sergey.kuksenko at oracle.com)
>> + */
>> + at BenchmarkMode(Mode.All)
>> +public class ForkedJvmArgsTest3 {
>> +
>> +    @GenerateMicroBenchmark
>> +    @Warmup(iterations = 0)
>> +    @Measurement(iterations = 1, time = 100, timeUnit =
>> TimeUnit.MILLISECONDS)
>> +    @Fork(jvmArgs = "-Dtest1  -Dtest3")
>> +    public void test1() {
>> +        Fixtures.work();
>> +        Assert.assertNotNull(System.getProperty("test1"));
>> +        Assert.assertNull(System.getProperty("test2"));
>> +        Assert.assertNotNull(System.getProperty("test3"));
>> +    }
>> +
>> +    @GenerateMicroBenchmark
>> +    @Warmup(iterations = 0)
>> +    @Measurement(iterations = 1, time = 100, timeUnit =
>> TimeUnit.MILLISECONDS)
>> +    @Fork(jvmArgs = "-Dtest2  -Dtest3")
>> +    public void test2() {
>> +        Fixtures.work();
>> +        Assert.assertNull(System.getProperty("test1"));
>> +        Assert.assertNotNull(System.getProperty("test2"));
>> +        Assert.assertNotNull(System.getProperty("test3"));
>> +    }
>> +
>> +    @Test
>> +    public void invoke() {
>> +        Main.testMain(Fixtures.getTestMask(this.getClass()) + " -foe");
>> +    }
>> +
>> +}
>> diff -r 9d5036308216
>> jmh-core/src/main/java/org/openjdk/jmh/runner/options/BaseOptions.java
>> ---
>> a/jmh-core/src/main/java/org/openjdk/jmh/runner/options/BaseOptions.java
>> Fri Jun 14 22:43:17 2013 +0400
>> +++
>> b/jmh-core/src/main/java/org/openjdk/jmh/runner/options/BaseOptions.java
>> Fri Jun 21 17:01:58 2013 +0400
>> @@ -138,7 +138,9 @@
>>              String opImage = fieldToCommandLineImage(f);
>>              if (opImage != null && !opImage.isEmpty()) {
>>                  for (String s : opImage.split(" ")) {
>> -                    sb.add(s);
>> +                    if(s!=null && !s.isEmpty()) {
>> +                        sb.add(s);
>> +                    }
>>                  }
>>              }
>>          }
>> diff -r 9d5036308216
>> jmh-core/src/main/java/org/openjdk/jmh/runner/options/HarnessOptions.java
>> ---
>> a/jmh-core/src/main/java/org/openjdk/jmh/runner/options/HarnessOptions.java
>> Fri Jun 14 22:43:17 2013 +0400
>> +++
>> b/jmh-core/src/main/java/org/openjdk/jmh/runner/options/HarnessOptions.java
>> Fri Jun 21 17:01:58 2013 +0400
>> @@ -33,6 +33,7 @@
>>  import org.openjdk.jmh.runner.BenchmarkRecord;
>>  import org.openjdk.jmh.runner.CompilerHints;
>>  import org.openjdk.jmh.runner.options.handlers.ForkOptionHandler;
>> +import org.openjdk.jmh.util.internal.CollectionUtils;
>>
>>  import java.io.File;
>>  import java.lang.management.ManagementFactory;
>> @@ -192,9 +193,9 @@
>>          }
>>
>>          if (getJvmArgs() != null) { // use supplied jvm args if given
>> in cmd line
>> -            command.addAll(Arrays.asList(getJvmArgs().split(" ")));
>> +            CollectionUtils.addAllNotEmpty(command,
>> Arrays.asList(getJvmArgs().split(" ")));
>>          } else if (annJvmArgs != null) { // use jvm args supplied in
>> annotation which shuns implicit args
>> -            command.addAll(Arrays.asList(annJvmArgs.split(" ")));
>> +            CollectionUtils.addAllNotEmpty(command,
>> Arrays.asList(annJvmArgs.split(" ")));
>>          } else {
>>              // else use same jvm args given to this runner
>>              RuntimeMXBean RuntimemxBean =
>> ManagementFactory.getRuntimeMXBean();
>> diff -r 9d5036308216
>> jmh-core/src/main/java/org/openjdk/jmh/util/internal/CollectionUtils.java
>> ---
>> a/jmh-core/src/main/java/org/openjdk/jmh/util/internal/CollectionUtils.java
>> Fri Jun 14 22:43:17 2013 +0400
>> +++
>> b/jmh-core/src/main/java/org/openjdk/jmh/util/internal/CollectionUtils.java
>> Fri Jun 21 17:01:58 2013 +0400
>> @@ -26,6 +26,7 @@
>>
>>  import org.openjdk.jmh.util.AnnotationUtils;
>>
>> +import java.util.Collection;
>>  import java.util.HashMap;
>>  import java.util.List;
>>  import java.util.Map;
>> @@ -103,4 +104,13 @@
>>          }
>>          return list;
>>      }
>> +
>> +    public static List<String> addAllNotEmpty(List<String> list,
>> Collection<String> toAdd) {
>> +        for(String s : toAdd) {
>> +            if (s != null && !s.isEmpty()) {
>> +                list.add(s);
>> +            }
>> +        }
>> +        return list;
>> +    }
>>  }
>>
>>
> 



More information about the jmh-dev mailing list