changeset 1401:483b68ad4813

7901871: JMH runners/profilers leave huge open files until the host VM exits
author shade
date Fri, 23 Dec 2016 16:41:00 +0100
parents 27c92e77d4fd
children c1d41dff4583
files jmh-core/src/main/java/org/openjdk/jmh/profile/AbstractPerfAsmProfiler.java jmh-core/src/main/java/org/openjdk/jmh/profile/LinuxPerfAsmProfiler.java jmh-core/src/main/java/org/openjdk/jmh/profile/WinPerfAsmProfiler.java jmh-core/src/main/java/org/openjdk/jmh/runner/Runner.java jmh-core/src/main/java/org/openjdk/jmh/util/FileUtils.java jmh-core/src/main/java/org/openjdk/jmh/util/TempFile.java jmh-core/src/main/java/org/openjdk/jmh/util/TempFileManager.java
diffstat 7 files changed, 184 insertions(+), 25 deletions(-) [+]
line wrap: on
line diff
--- a/jmh-core/src/main/java/org/openjdk/jmh/profile/AbstractPerfAsmProfiler.java	Thu Dec 15 22:19:51 2016 +0100
+++ b/jmh-core/src/main/java/org/openjdk/jmh/profile/AbstractPerfAsmProfiler.java	Fri Dec 23 16:41:00 2016 +0100
@@ -68,18 +68,18 @@
     private final boolean printCompilationInfo;
     private final boolean intelSyntax;
 
-    protected final String hsLog;
-    protected final String perfBinData;
-    protected final String perfParsedData;
+    protected final TempFile hsLog;
+    protected final TempFile perfBinData;
+    protected final TempFile perfParsedData;
     protected final OptionSet set;
     private final boolean drawIntraJumps;
     private final boolean drawInterJumps;
 
     protected AbstractPerfAsmProfiler(String initLine, String... events) throws ProfilerException {
         try {
-            hsLog = FileUtils.tempFile("hslog").getAbsolutePath();
-            perfBinData = FileUtils.tempFile("perfbin").getAbsolutePath();
-            perfParsedData = FileUtils.tempFile("perfparsed").getAbsolutePath();
+            hsLog = FileUtils.weakTempFile("hslog");
+            perfBinData = FileUtils.weakTempFile("perfbin");
+            perfParsedData = FileUtils.weakTempFile("perfparsed");
         } catch (IOException e) {
             throw new ProfilerException(e);
         }
@@ -236,7 +236,7 @@
             opts.addAll(Arrays.asList(
                 "-XX:+UnlockDiagnosticVMOptions",
                 "-XX:+LogCompilation",
-                "-XX:LogFile=" + hsLog,
+                "-XX:LogFile=" + hsLog.getAbsolutePath(),
                 "-XX:+PrintAssembly"));
 
             if (!skipInterpreter) {
@@ -272,6 +272,11 @@
     public Collection<? extends Result> afterTrial(BenchmarkResult br, long pid, File stdOut, File stdErr) {
         PerfResult result = processAssembly(br, stdOut, stdErr);
 
+        // we know these are not needed anymore, proactively delete
+        hsLog.delete();
+        perfBinData.delete();
+        perfParsedData.delete();
+
         return Collections.singleton(result);
     }
 
@@ -320,7 +325,7 @@
          * 2. Read out PrintAssembly output
          */
 
-        Assembly assembly = readAssembly(new File(hsLog));
+        Assembly assembly = readAssembly(hsLog.file());
         if (assembly.size() > 0) {
             pw.printf("PrintAssembly processed: %d total address lines.%n", assembly.size());
         } else if (skipAssembly) {
@@ -584,7 +589,7 @@
                 savePerfOutputTo + "/" + br.getParams().id() + ".perf" :
                 savePerfOutputToFile;
             try {
-                FileUtils.copy(perfParsedData, target);
+                FileUtils.copy(perfParsedData.getAbsolutePath(), target);
                 pw.println("Perf output saved to " + target);
             } catch (IOException e) {
                 pw.println("Unable to save perf output to " + target);
@@ -599,7 +604,7 @@
                 savePerfBinTo + "/" + br.getParams().id() + perfBinaryExtension() :
                 savePerfBinFile;
             try {
-                FileUtils.copy(perfBinData, target);
+                FileUtils.copy(perfBinData.getAbsolutePath(), target);
                 pw.println("Perf binary output saved to " + target);
             } catch (IOException e) {
                 pw.println("Unable to save perf binary output to " + target);
--- a/jmh-core/src/main/java/org/openjdk/jmh/profile/LinuxPerfAsmProfiler.java	Thu Dec 15 22:19:51 2016 +0100
+++ b/jmh-core/src/main/java/org/openjdk/jmh/profile/LinuxPerfAsmProfiler.java	Fri Dec 23 16:41:00 2016 +0100
@@ -67,7 +67,7 @@
 
     @Override
     public Collection<String> addJVMInvokeOptions(BenchmarkParams params) {
-        return Arrays.asList(PerfSupport.PERF_EXEC, "record", "--freq", String.valueOf(sampleFrequency), "--event", Utils.join(events, ","), "--output", perfBinData);
+        return Arrays.asList(PerfSupport.PERF_EXEC, "record", "--freq", String.valueOf(sampleFrequency), "--event", Utils.join(events, ","), "--output", perfBinData.getAbsolutePath());
     }
 
     @Override
@@ -77,8 +77,8 @@
 
     @Override
     protected void parseEvents() {
-        try (FileOutputStream fos = new FileOutputStream(perfParsedData)) {
-            ProcessBuilder pb = new ProcessBuilder(PerfSupport.PERF_EXEC, "script", "--fields", "time,event,ip,sym,dso", "--input", perfBinData);
+        try (FileOutputStream fos = new FileOutputStream(perfParsedData.file())) {
+            ProcessBuilder pb = new ProcessBuilder(PerfSupport.PERF_EXEC, "script", "--fields", "time,event,ip,sym,dso", "--input", perfBinData.getAbsolutePath());
             Process p = pb.start();
 
             // drain streams, else we might lock up
@@ -102,7 +102,7 @@
         double readFrom = skipMs / 1000D;
         double readTo = (skipMs + lenMs) / 1000D;
 
-        try (FileReader fr = new FileReader(perfParsedData);
+        try (FileReader fr = new FileReader(perfParsedData.file());
              BufferedReader reader = new BufferedReader(fr)) {
             Deduplicator<MethodDesc> dedup = new Deduplicator<>();
 
--- a/jmh-core/src/main/java/org/openjdk/jmh/profile/WinPerfAsmProfiler.java	Thu Dec 15 22:19:51 2016 +0100
+++ b/jmh-core/src/main/java/org/openjdk/jmh/profile/WinPerfAsmProfiler.java	Fri Dec 23 16:41:00 2016 +0100
@@ -146,21 +146,21 @@
     @Override
     protected void parseEvents() {
         // 1. Stop profiling by calling xperf dumper.
-        Collection<String> errs = Utils.tryWith(path, "-d", perfBinData);
+        Collection<String> errs = Utils.tryWith(path, "-d", perfBinData.getAbsolutePath());
 
         if (!errs.isEmpty())
             throw new IllegalStateException("Failed to stop xperf: " + errs);
 
         // 2. Convert binary data to text form.
         try {
-            ProcessBuilder pb = new ProcessBuilder(path, "-i", perfBinData, "-symbols", "-a", "dumper");
+            ProcessBuilder pb = new ProcessBuilder(path, "-i", perfBinData.getAbsolutePath(), "-symbols", "-a", "dumper");
             if (symbolDir != null) {
                 pb.environment().put("_NT_SYMBOL_PATH", symbolDir);
             }
 
             Process p = pb.start();
 
-            FileOutputStream fos = new FileOutputStream(perfParsedData);
+            FileOutputStream fos = new FileOutputStream(perfParsedData.file());
 
             InputStreamDrainer errDrainer = new InputStreamDrainer(p.getErrorStream(), fos);
             InputStreamDrainer outDrainer = new InputStreamDrainer(p.getInputStream(), fos);
@@ -182,7 +182,7 @@
         double readFrom = skipMs / 1000D;
         double readTo = (skipMs + lenMs) / 1000D;
 
-        try (FileReader fr = new FileReader(perfParsedData);
+        try (FileReader fr = new FileReader(perfParsedData.file());
              BufferedReader reader = new BufferedReader(fr)) {
             Deduplicator<MethodDesc> dedup = new Deduplicator<>();
 
--- a/jmh-core/src/main/java/org/openjdk/jmh/runner/Runner.java	Thu Dec 15 22:19:51 2016 +0100
+++ b/jmh-core/src/main/java/org/openjdk/jmh/runner/Runner.java	Fri Dec 23 16:41:00 2016 +0100
@@ -623,13 +623,17 @@
                     etaBeforeBenchmark();
                     out.println("# Warmup Fork: " + (i + 1) + " of " + warmupForkCount);
 
-                    File stdErr = FileUtils.tempFile("stderr");
-                    File stdOut = FileUtils.tempFile("stdout");
+                    TempFile stdErr = FileUtils.weakTempFile("stderr");
+                    TempFile stdOut = FileUtils.weakTempFile("stdout");
 
-                    doFork(server, forkedString, stdOut, stdErr, printOut, printErr);
+                    doFork(server, forkedString, stdOut.file(), stdErr.file(), printOut, printErr);
 
                     etaAfterBenchmark(params);
                     out.println("");
+
+                    // we know these are not needed anymore, proactively delete
+                    stdErr.delete();
+                    stdOut.delete();
                 }
             }
 
@@ -640,8 +644,8 @@
                 etaBeforeBenchmark();
                 out.println("# Fork: " + (i + 1) + " of " + forkCount);
 
-                File stdErr = FileUtils.tempFile("stderr");
-                File stdOut = FileUtils.tempFile("stdout");
+                TempFile stdErr = FileUtils.weakTempFile("stderr");
+                TempFile stdOut = FileUtils.weakTempFile("stdout");
 
                 if (!profilers.isEmpty()) {
                     out.print("# Preparing profilers: ");
@@ -659,7 +663,7 @@
                     }
                 }
 
-                List<IterationResult> result = doFork(server, forkedString, stdOut, stdErr, printOut, printErr);
+                List<IterationResult> result = doFork(server, forkedString, stdOut.file(), stdErr.file(), printOut, printErr);
                 if (!result.isEmpty()) {
                     long pid = server.getClientPid();
 
@@ -670,7 +674,7 @@
                         out.print("# Processing profiler results: ");
                         for (ExternalProfiler profiler : profilersRev) {
                             out.print(profiler.getClass().getSimpleName() + " ");
-                            for (Result profR : profiler.afterTrial(br, pid, stdOut, stdErr)) {
+                            for (Result profR : profiler.afterTrial(br, pid, stdOut.file(), stdErr.file())) {
                                 br.addBenchmarkResult(profR);
                             }
                         }
@@ -682,6 +686,10 @@
 
                 etaAfterBenchmark(params);
                 out.println("");
+
+                // we know these are not needed anymore, proactively delete
+                stdOut.delete();
+                stdErr.delete();
             }
 
             out.endBenchmark(new RunResult(params, results.get(params)).getAggregatedResult());
@@ -699,6 +707,7 @@
             if (server != null) {
                 server.terminate();
             }
+            FileUtils.purgeTemps();
         }
 
         return results;
--- a/jmh-core/src/main/java/org/openjdk/jmh/util/FileUtils.java	Thu Dec 15 22:19:51 2016 +0100
+++ b/jmh-core/src/main/java/org/openjdk/jmh/util/FileUtils.java	Fri Dec 23 16:41:00 2016 +0100
@@ -39,6 +39,32 @@
 
     }
 
+    static final TempFileManager TEMP_FILE_MANAGER = new TempFileManager();
+
+    /**
+     * Creates the temp file, and retains it as long as the reference to it
+     * is reachable.
+     *
+     * @param suffix suffix
+     * @return temp file
+     * @throws IOException
+     */
+    public static TempFile weakTempFile(String suffix) throws IOException {
+        return TEMP_FILE_MANAGER.create(suffix);
+    }
+
+    public static void purgeTemps() {
+        TEMP_FILE_MANAGER.purge();
+    }
+
+    /**
+     * Creates the temp file with given suffix. The file would be removed
+     * on JVM exit, or when caller deletes the file itself.
+     *
+     * @param suffix suffix
+     * @return temporary file
+     * @throws IOException
+     */
     public static File tempFile(String suffix) throws IOException {
         File file = File.createTempFile("jmh", suffix);
         file.deleteOnExit();
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/jmh-core/src/main/java/org/openjdk/jmh/util/TempFile.java	Fri Dec 23 16:41:00 2016 +0100
@@ -0,0 +1,47 @@
+/*
+ * Copyright (c) 2016, Red Hat Inc.
+ * 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.util;
+
+import java.io.*;
+
+public class TempFile {
+    private final File file;
+
+    public TempFile(File file) {
+        this.file = file;
+    }
+
+    public void delete() {
+        file.delete();
+    }
+
+    public String getAbsolutePath() {
+        return file.getAbsolutePath();
+    }
+
+    public File file() {
+        return file;
+    }
+}
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/jmh-core/src/main/java/org/openjdk/jmh/util/TempFileManager.java	Fri Dec 23 16:41:00 2016 +0100
@@ -0,0 +1,72 @@
+/*
+ * Copyright (c) 2016, Red Hat Inc.
+ * 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.util;
+
+import java.io.File;
+import java.io.IOException;
+import java.lang.ref.ReferenceQueue;
+import java.lang.ref.WeakReference;
+import java.util.HashSet;
+import java.util.Set;
+
+public class TempFileManager {
+
+    private final ReferenceQueue<TempFile> rq;
+    private final Set<TempFileReference> refs;
+
+    public TempFileManager() {
+        rq = new ReferenceQueue<>();
+        refs = new HashSet<>();
+    }
+
+    public TempFile create(String suffix) throws IOException {
+        purge();
+        File file = File.createTempFile("jmh", suffix);
+        file.deleteOnExit();
+        TempFile tf = new TempFile(file);
+        refs.add(new TempFileReference(tf, rq));
+        return tf;
+    }
+
+    public void purge() {
+        TempFileReference ref;
+        while ((ref = (TempFileReference) rq.poll()) != null) {
+            if (ref.file != null) {
+                ref.file.delete();
+            }
+            refs.remove(ref);
+        }
+    }
+
+    private static class TempFileReference extends WeakReference<TempFile> {
+        final File file;
+
+        TempFileReference(TempFile referent, ReferenceQueue<? super TempFile> q) {
+            super(referent, q);
+            file = referent.file();
+        }
+    }
+
+}