changeset 1412:37ef3550476c

7901910: Non-ASCII @Param values get garbled on non-UTF-8 platform encoding
author shade
date Fri, 10 Mar 2017 22:47:33 +0100
parents ecd9e76155fe
children 6eb89dc11810
files jmh-core-ct/src/test/java/org/openjdk/jmh/ct/InMemoryGeneratorDestination.java jmh-core-it/src/test/java/org/openjdk/jmh/it/params/UTF8ParamsTest.java jmh-core/src/main/java/org/openjdk/jmh/generators/core/BenchmarkGenerator.java jmh-core/src/main/java/org/openjdk/jmh/generators/core/CompilerControlPlugin.java jmh-core/src/main/java/org/openjdk/jmh/generators/core/FileSystemDestination.java jmh-core/src/main/java/org/openjdk/jmh/generators/core/GeneratorDestination.java jmh-core/src/main/java/org/openjdk/jmh/runner/AbstractResourceReader.java jmh-core/src/main/java/org/openjdk/jmh/runner/BenchmarkList.java jmh-core/src/test/java/org/openjdk/jmh/runner/TestBenchmarkListEncoding.java jmh-generator-annprocess/src/main/java/org/openjdk/jmh/generators/annotations/APGeneratorDestinaton.java
diffstat 10 files changed, 264 insertions(+), 79 deletions(-) [+]
line wrap: on
line diff
--- a/jmh-core-ct/src/test/java/org/openjdk/jmh/ct/InMemoryGeneratorDestination.java	Fri Feb 24 21:43:59 2017 +0100
+++ b/jmh-core-ct/src/test/java/org/openjdk/jmh/ct/InMemoryGeneratorDestination.java	Fri Mar 10 22:47:33 2017 +0100
@@ -27,12 +27,7 @@
 import org.openjdk.jmh.generators.core.GeneratorDestination;
 import org.openjdk.jmh.generators.core.MetadataInfo;
 
-import java.io.IOException;
-import java.io.PrintWriter;
-import java.io.Reader;
-import java.io.StringReader;
-import java.io.StringWriter;
-import java.io.Writer;
+import java.io.*;
 import java.util.ArrayList;
 import java.util.HashMap;
 import java.util.List;
@@ -45,22 +40,22 @@
     private final List<String> infos = new ArrayList<>();
 
     private final Map<String, StringWriter> classBodies = new HashMap<>();
-    private final Map<String, StringWriter> resourceBodies = new HashMap<>();
+    private final Map<String, ByteArrayOutputStream> resourceBodies = new HashMap<>();
 
     @Override
-    public Writer newResource(String resourcePath) throws IOException {
-        StringWriter sw = new StringWriter();
+    public OutputStream newResource(String resourcePath) throws IOException {
+        ByteArrayOutputStream sw = new ByteArrayOutputStream();
         resourceBodies.put(resourcePath, sw);
-        return new PrintWriter(sw, true);
+        return sw;
     }
 
     @Override
-    public Reader getResource(String resourcePath) throws IOException {
-        StringWriter sw = resourceBodies.get(resourcePath);
+    public InputStream getResource(String resourcePath) throws IOException {
+        ByteArrayOutputStream sw = resourceBodies.get(resourcePath);
         if (sw == null) {
             throw new IOException("Does not exist: " + resourcePath);
         }
-        return new StringReader(sw.toString());
+        return new ByteArrayInputStream(sw.toByteArray());
     }
 
     @Override
@@ -126,8 +121,12 @@
 
     public Map<String, String> getResources() {
         Map<String, String> result = new HashMap<>();
-        for (Map.Entry<String, StringWriter> e : resourceBodies.entrySet()) {
-            result.put(e.getKey(), e.getValue().toString());
+        for (Map.Entry<String, ByteArrayOutputStream> e : resourceBodies.entrySet()) {
+            try {
+                result.put(e.getKey(), e.getValue().toString("UTF-8"));
+            } catch (UnsupportedEncodingException e1) {
+                e1.printStackTrace();
+            }
         }
         return result;
     }
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/jmh-core-it/src/test/java/org/openjdk/jmh/it/params/UTF8ParamsTest.java	Fri Mar 10 22:47:33 2017 +0100
@@ -0,0 +1,71 @@
+/*
+ * Copyright (c) 2005, 2014, 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.params;
+
+import junit.framework.Assert;
+import org.junit.Test;
+import org.openjdk.jmh.annotations.*;
+import org.openjdk.jmh.it.Fixtures;
+import org.openjdk.jmh.runner.Runner;
+import org.openjdk.jmh.runner.RunnerException;
+import org.openjdk.jmh.runner.options.Options;
+import org.openjdk.jmh.runner.options.OptionsBuilder;
+
+import java.util.Locale;
+import java.util.concurrent.TimeUnit;
+
+@State(Scope.Benchmark)
+public class UTF8ParamsTest {
+
+    @Param("\uff11000")
+    String value;
+
+    @Benchmark
+    @Warmup(iterations = 0)
+    @Measurement(iterations = 1, time = 100, timeUnit = TimeUnit.MILLISECONDS)
+    @Fork(1)
+    public void test() {
+        Fixtures.work();
+        Assert.assertEquals("\uff11000", value);
+    }
+
+    @Test
+    public void vals() throws RunnerException {
+        Locale prev = Locale.getDefault();
+        Locale.setDefault(Locale.ROOT);
+
+        try {
+            Options opts = new OptionsBuilder()
+                    .include(Fixtures.getTestMask(this.getClass()))
+                    .shouldFailOnError(true)
+                    .build();
+
+            new Runner(opts).run();
+        } finally {
+            Locale.setDefault(prev);
+        }
+    }
+
+}
--- a/jmh-core/src/main/java/org/openjdk/jmh/generators/core/BenchmarkGenerator.java	Fri Feb 24 21:43:59 2017 +0100
+++ b/jmh-core/src/main/java/org/openjdk/jmh/generators/core/BenchmarkGenerator.java	Fri Mar 10 22:47:33 2017 +0100
@@ -29,14 +29,11 @@
 import org.openjdk.jmh.results.*;
 import org.openjdk.jmh.runner.*;
 import org.openjdk.jmh.runner.Defaults;
-import org.openjdk.jmh.util.FileUtils;
 import org.openjdk.jmh.util.HashMultimap;
 import org.openjdk.jmh.util.Multimap;
 import org.openjdk.jmh.util.SampleBuffer;
 
-import java.io.IOException;
-import java.io.PrintWriter;
-import java.io.Reader;
+import java.io.*;
 import java.lang.reflect.Field;
 import java.util.*;
 import java.util.concurrent.TimeUnit;
@@ -131,12 +128,13 @@
         // to detect removed @Benchmark entries. To do so, we are overwriting all benchmark
         // records that belong to a current compilation unit.
         Multimap<String, BenchmarkListEntry> entriesByQName = new HashMultimap<>();
-        try {
-            for (String line : readBenchmarkList(destination)) {
-                BenchmarkListEntry br = new BenchmarkListEntry(line);
-                entries.add(br);
-                entriesByQName.put(br.getUserClassQName(), br);
+        try (InputStream stream = destination.getResource(BenchmarkList.BENCHMARK_LIST.substring(1))) {
+            for (BenchmarkListEntry ble : BenchmarkList.readBenchmarkList(stream)) {
+                entries.add(ble);
+                entriesByQName.put(ble.getUserClassQName(), ble);
             }
+        } catch (IOException e) {
+            // okay, move on
         } catch (UnsupportedOperationException e) {
             destination.printError("Unable to read the existing benchmark list.", e);
         }
@@ -185,26 +183,8 @@
             }
         }
 
-        writeBenchmarkList(destination, entries);
-    }
-
-    private Collection<String> readBenchmarkList(GeneratorDestination destination) {
-        String list = BenchmarkList.BENCHMARK_LIST.substring(1);
-        try (Reader reader = destination.getResource(list)) {
-            return FileUtils.readAllLines(reader);
-        } catch (IOException e) {
-            // okay, move on
-        }
-        return Collections.emptyList();
-    }
-
-    private void writeBenchmarkList(GeneratorDestination destination, Collection<BenchmarkListEntry> entries) {
-        String list = BenchmarkList.BENCHMARK_LIST.substring(1);
-        try (PrintWriter writer = new PrintWriter(destination.newResource(list))) {
-            // Write out the complete benchmark list
-            for (BenchmarkListEntry entry : entries) {
-                writer.println(entry.toLine());
-            }
+        try (OutputStream stream = destination.newResource(BenchmarkList.BENCHMARK_LIST.substring(1))) {
+            BenchmarkList.writeBenchmarkList(stream, entries);
         } catch (IOException ex) {
             destination.printError("Error writing benchmark list", ex);
         }
--- a/jmh-core/src/main/java/org/openjdk/jmh/generators/core/CompilerControlPlugin.java	Fri Feb 24 21:43:59 2017 +0100
+++ b/jmh-core/src/main/java/org/openjdk/jmh/generators/core/CompilerControlPlugin.java	Fri Mar 10 22:47:33 2017 +0100
@@ -28,7 +28,10 @@
 import org.openjdk.jmh.runner.CompilerHints;
 
 import java.io.IOException;
+import java.io.OutputStreamWriter;
 import java.io.PrintWriter;
+import java.io.Writer;
+import java.nio.charset.StandardCharsets;
 import java.util.Comparator;
 import java.util.Set;
 import java.util.SortedSet;
@@ -97,8 +100,8 @@
     }
 
     public void finish(GeneratorSource source, GeneratorDestination destination) {
-        try {
-            PrintWriter writer = new PrintWriter(destination.newResource(CompilerHints.LIST.substring(1)));
+        try (Writer w = new OutputStreamWriter(destination.newResource(CompilerHints.LIST.substring(1)), StandardCharsets.UTF_8)){
+            PrintWriter writer = new PrintWriter(w);
             for (String line : lines) {
                 writer.println(line);
             }
--- a/jmh-core/src/main/java/org/openjdk/jmh/generators/core/FileSystemDestination.java	Fri Feb 24 21:43:59 2017 +0100
+++ b/jmh-core/src/main/java/org/openjdk/jmh/generators/core/FileSystemDestination.java	Fri Mar 10 22:47:33 2017 +0100
@@ -24,12 +24,7 @@
  */
 package org.openjdk.jmh.generators.core;
 
-import java.io.File;
-import java.io.FileReader;
-import java.io.FileWriter;
-import java.io.IOException;
-import java.io.Reader;
-import java.io.Writer;
+import java.io.*;
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.List;
@@ -49,23 +44,23 @@
     }
 
     @Override
-    public Writer newResource(String resourcePath) throws IOException {
+    public OutputStream newResource(String resourcePath) throws IOException {
         String pathName = resourceDir.getAbsolutePath() + "/" + resourcePath;
         File p = new File(pathName.substring(0, pathName.lastIndexOf("/")));
         if (!p.mkdirs() && !p.isDirectory()) {
             throw new IOException("Unable to create " + p.getAbsolutePath());
         }
-        return new FileWriter(new File(pathName));
+        return new FileOutputStream(new File(pathName));
     }
 
     @Override
-    public Reader getResource(String resourcePath) throws IOException {
+    public InputStream getResource(String resourcePath) throws IOException {
         String pathName = resourceDir.getAbsolutePath() + "/" + resourcePath;
         File p = new File(pathName.substring(0, pathName.lastIndexOf("/")));
         if (!p.isFile() && !p.exists()) {
             throw new IOException("Unable to open " + pathName);
         }
-        return new FileReader(new File(pathName));
+        return new FileInputStream(new File(pathName));
     }
 
     @Override
--- a/jmh-core/src/main/java/org/openjdk/jmh/generators/core/GeneratorDestination.java	Fri Feb 24 21:43:59 2017 +0100
+++ b/jmh-core/src/main/java/org/openjdk/jmh/generators/core/GeneratorDestination.java	Fri Mar 10 22:47:33 2017 +0100
@@ -24,9 +24,7 @@
  */
 package org.openjdk.jmh.generators.core;
 
-import java.io.IOException;
-import java.io.Reader;
-import java.io.Writer;
+import java.io.*;
 
 /**
  * Generator destination.
@@ -36,24 +34,24 @@
 public interface GeneratorDestination {
 
     /**
-     * Returns the Writer for the given resource.
-     * Callers are responsible for closing Writers.
+     * Returns the stream for the given resource.
+     * Callers are responsible for closing streams.
      *
      * @param resourcePath resource path
-     * @return writer usable to write the resource
+     * @return output stream to write the resource to.
      * @throws java.io.IOException if something wacked happens
      */
-    Writer newResource(String resourcePath) throws IOException;
+    OutputStream newResource(String resourcePath) throws IOException;
 
     /**
-     * Returns the Reader for the given resource.
-     * Callers are responsible for closing Readers.
+     * Returns the stream for the given resource.
+     * Callers are responsible for closing streams.
      *
      * @param resourcePath resource path
-     * @return reader usable to read the resource
+     * @return stream usable to read the resource
      * @throws java.io.IOException if something wacked happens
      */
-    Reader getResource(String resourcePath) throws IOException;
+    InputStream getResource(String resourcePath) throws IOException;
 
     /**
      * Returns the Writer for the given class.
--- a/jmh-core/src/main/java/org/openjdk/jmh/runner/AbstractResourceReader.java	Fri Feb 24 21:43:59 2017 +0100
+++ b/jmh-core/src/main/java/org/openjdk/jmh/runner/AbstractResourceReader.java	Fri Mar 10 22:47:33 2017 +0100
@@ -26,6 +26,7 @@
 
 import java.io.*;
 import java.net.URL;
+import java.nio.charset.StandardCharsets;
 import java.util.ArrayList;
 import java.util.Collections;
 import java.util.Enumeration;
@@ -55,7 +56,7 @@
 
         if (file != null) {
             try {
-                return Collections.<Reader>singletonList(new FileReader(file));
+                return Collections.<Reader>singletonList(new InputStreamReader(new FileInputStream(file), StandardCharsets.UTF_8));
             } catch (FileNotFoundException e) {
                 throw new RuntimeException("ERROR: Could not find resource", e);
             }
@@ -80,7 +81,7 @@
                     while (urls.hasMoreElements()) {
                         url = urls.nextElement();
                         InputStream stream = url.openStream();
-                        readers.add(new InputStreamReader(stream));
+                        readers.add(new InputStreamReader(stream, StandardCharsets.UTF_8));
                     }
                 } catch (IOException e) {
                     for (Reader r : readers) {
--- a/jmh-core/src/main/java/org/openjdk/jmh/runner/BenchmarkList.java	Fri Feb 24 21:43:59 2017 +0100
+++ b/jmh-core/src/main/java/org/openjdk/jmh/runner/BenchmarkList.java	Fri Mar 10 22:47:33 2017 +0100
@@ -25,10 +25,10 @@
 package org.openjdk.jmh.runner;
 
 import org.openjdk.jmh.runner.format.OutputFormat;
+import org.openjdk.jmh.util.FileUtils;
 
-import java.io.BufferedReader;
-import java.io.IOException;
-import java.io.Reader;
+import java.io.*;
+import java.nio.charset.StandardCharsets;
 import java.util.*;
 import java.util.regex.Pattern;
 
@@ -56,6 +56,25 @@
         return new BenchmarkList(null, null, strings);
     }
 
+    public static Collection<BenchmarkListEntry> readBenchmarkList(InputStream stream) throws IOException {
+        try (Reader reader = new InputStreamReader(stream, StandardCharsets.UTF_8)) {
+            Collection<BenchmarkListEntry> entries = new ArrayList<>();
+            for (String line : FileUtils.readAllLines(reader)) {
+                BenchmarkListEntry ble = new BenchmarkListEntry(line);
+                entries.add(ble);
+            }
+            return entries;
+        }
+    }
+
+    public static void writeBenchmarkList(OutputStream stream, Collection<BenchmarkListEntry> entries) throws IOException {
+        try (PrintWriter writer = new PrintWriter(new OutputStreamWriter(stream, StandardCharsets.UTF_8))) {
+            for (BenchmarkListEntry entry : entries) {
+                writer.println(entry.toLine());
+            }
+        }
+    }
+
     private BenchmarkList(String file, String resource, String strings) {
         super(file, resource, strings);
     }
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/jmh-core/src/test/java/org/openjdk/jmh/runner/TestBenchmarkListEncoding.java	Fri Mar 10 22:47:33 2017 +0100
@@ -0,0 +1,121 @@
+/*
+ * Copyright (c) 2017, Red Hat Inc. 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;
+
+import org.junit.Test;
+import org.openjdk.jmh.annotations.Mode;
+import org.openjdk.jmh.runner.options.TimeValue;
+import org.openjdk.jmh.util.Optional;
+
+import java.io.ByteArrayInputStream;
+import java.io.ByteArrayOutputStream;
+import java.io.IOException;
+import java.lang.reflect.Field;
+import java.nio.charset.Charset;
+import java.util.*;
+import java.util.concurrent.TimeUnit;
+
+import static org.junit.Assert.assertEquals;
+
+public class TestBenchmarkListEncoding {
+
+    private static BenchmarkListEntry stub(String userClassQName, String generatedClassQName, String method, Mode mode) {
+        BenchmarkListEntry br = new BenchmarkListEntry(
+                userClassQName,
+                generatedClassQName,
+                method,
+                mode,
+                Optional.<Integer>none(),
+                new int[]{1},
+                Optional.<Collection<String>>none(),
+                Optional.<Integer>none(),
+                Optional.<TimeValue>none(),
+                Optional.<Integer>none(),
+                Optional.<Integer>none(),
+                Optional.<TimeValue>none(),
+                Optional.<Integer>none(),
+                Optional.<Integer>none(),
+                Optional.<Integer>none(),
+                Optional.<String>none(),
+                Optional.<Collection<String>>none(),
+                Optional.<Collection<String>>none(),
+                Optional.<Collection<String>>none(),
+                Optional.<Map<String, String[]>>none(),
+                Optional.<TimeUnit>none(),
+                Optional.<Integer>none(),
+                Optional.<TimeValue>none()
+        );
+        return br;
+    }
+
+    @Test
+    public void test_ASCII_UTF8() throws Exception {
+        testWith("ASCII", "UTF-8");
+    }
+
+    @Test
+    public void test_UTF8_ASCII() throws Exception {
+        testWith("UTF-8", "ASCII");
+    }
+
+    @Test
+    public void test_UTF8_UTF8() throws Exception {
+        testWith("UTF-8", "UTF-8");
+    }
+
+    @Test
+    public void test_ASCII_ASCII() throws Exception {
+        testWith("ASCII", "ASCII");
+    }
+
+    public void testWith(String src, String dst) throws IOException {
+        BenchmarkListEntry br = stub("something.Test",
+                "something.generated.Test",
+                "testКонкаррентХэшмап",
+                Mode.AverageTime);
+
+        resetCharset();
+        System.setProperty("file.encoding", src);
+        ByteArrayOutputStream bos = new ByteArrayOutputStream();
+        BenchmarkList.writeBenchmarkList(bos, Collections.singleton(br));
+
+        resetCharset();
+        System.setProperty("file.encoding", dst);
+        ByteArrayInputStream bis = new ByteArrayInputStream(bos.toByteArray());
+        Collection<BenchmarkListEntry> read = BenchmarkList.readBenchmarkList(bis);
+        BenchmarkListEntry first = read.iterator().next();
+        assertEquals("something.Test.testКонкаррентХэшмап", first.getUsername());
+    }
+
+    private void resetCharset() {
+        try {
+            Field f = Charset.class.getDeclaredField("defaultCharset");
+            f.setAccessible(true);
+            f.set(null, null);
+        } catch (Exception e) {
+            // okay then.
+        }
+    }
+}
--- a/jmh-generator-annprocess/src/main/java/org/openjdk/jmh/generators/annotations/APGeneratorDestinaton.java	Fri Feb 24 21:43:59 2017 +0100
+++ b/jmh-generator-annprocess/src/main/java/org/openjdk/jmh/generators/annotations/APGeneratorDestinaton.java	Fri Mar 10 22:47:33 2017 +0100
@@ -32,9 +32,7 @@
 import javax.annotation.processing.RoundEnvironment;
 import javax.tools.Diagnostic;
 import javax.tools.StandardLocation;
-import java.io.IOException;
-import java.io.Reader;
-import java.io.Writer;
+import java.io.*;
 
 public class APGeneratorDestinaton implements GeneratorDestination {
 
@@ -45,13 +43,13 @@
     }
 
     @Override
-    public Writer newResource(String resourcePath) throws IOException {
-        return processingEnv.getFiler().createResource(StandardLocation.CLASS_OUTPUT, "", resourcePath).openWriter();
+    public OutputStream newResource(String resourcePath) throws IOException {
+        return processingEnv.getFiler().createResource(StandardLocation.CLASS_OUTPUT, "", resourcePath).openOutputStream();
     }
 
     @Override
-    public Reader getResource(String resourcePath) throws IOException {
-        return processingEnv.getFiler().getResource(StandardLocation.CLASS_OUTPUT, "", resourcePath).openReader(true);
+    public InputStream getResource(String resourcePath) throws IOException {
+        return processingEnv.getFiler().getResource(StandardLocation.CLASS_OUTPUT, "", resourcePath).openInputStream();
     }
 
     @Override