CODETOOLS-7902851: jcstress: Provide the ability to capture VM output

Author: shipilev

jcstress-core/src/main/java/org/openjdk/jcstress/EmbeddedExecutor.java

@@ -38,7 +38,6 @@
 import java.util.concurrent.Executors;
 import java.util.concurrent.ThreadFactory;
 import java.util.concurrent.atomic.AtomicInteger;
-import java.util.function.Consumer;
 
 public class EmbeddedExecutor {
 
@@ -83,11 +82,11 @@ private Runnable task(TestConfig config, List<Integer> acquiredCPUs) {
                 o.run();
             } catch (ClassFormatError | NoClassDefFoundError | NoSuchMethodError | NoSuchFieldError e) {
                 TestResult result = new TestResult(config, Status.API_MISMATCH);
-                result.addAuxData(StringUtils.getStacktrace(e));
+                result.addMessage(StringUtils.getStacktrace(e));
                 sink.add(result);
             } catch (Throwable ex) {
                 TestResult result = new TestResult(config, Status.TEST_ERROR);
-                result.addAuxData(StringUtils.getStacktrace(ex));
+                result.addMessage(StringUtils.getStacktrace(ex));
                 sink.add(result);
             } finally {
                 if (acquiredCPUs != null) {

jcstress-core/src/main/java/org/openjdk/jcstress/TestExecutor.java

@@ -80,7 +80,7 @@ public TestConfig onJobRequest(String token) {
 
             @Override
             public void onResult(String token, TestResult result) {
-                sink.add(result);
+                vmByToken.get(token).recordResult(result);
             }
         }) : null;
         embeddedExecutor = new EmbeddedExecutor(sink, cpuLayout);
@@ -127,19 +127,10 @@ private List<Integer> acquireCPUs(int cpus) {
 
     private void processReadyVMs() {
         for (VM vm : vmByToken.values()) {
-            try {
-                if (!vm.checkTermination()) continue;
-            } catch (ForkFailedException e) {
-                TestConfig task = vm.getTask();
-                TestResult result = new TestResult(task, Status.VM_ERROR);
-                for (String i : e.getInfo()) {
-                    result.addAuxData(i);
-                }
-                sink.add(result);
+            if (vm.checkCompleted(sink)) {
+                vmByToken.remove(vm.token, vm);
+                cpuLayout.release(vm.claimedCPUs);
             }
-
-            vmByToken.remove(vm.token, vm);
-            cpuLayout.release(vm.claimedCPUs);
         }
     }
 
@@ -155,6 +146,7 @@ private static class VM {
         private Process process;
         private boolean processed;
         private IOException pendingException;
+        private TestResult result;
 
         public VM(String host, int port, String token, TestConfig task, List<Integer> claimedCPUs) {
             this.host = host;
@@ -258,52 +250,82 @@ void start() {
             }
         }
 
-        boolean checkTermination() {
+        public synchronized TestConfig jobRequest() {
+            if (processed) {
+                return null;
+            }
+            processed = true;
+            return getTask();
+        }
+
+        public TestConfig getTask() {
+            return task;
+        }
+
+        public boolean checkCompleted(TestResultCollector sink) {
+            // There is a pending exception that terminated the target VM.
             if (pendingException != null) {
-                throw new ForkFailedException(pendingException.getMessage());
+                dumpFailure(sink, Collections.singleton(pendingException.getMessage()), Collections.emptyList());
+                return true;
             }
 
+            // Process is still alive, no need to ask about the status.
             if (process.isAlive()) {
                 return false;
-            } else {
-                // Try to poll the exit code, and fail if it's not zero.
+            }
+
+            // Try to poll the exit code, and fail if it's not zero.
+            try {
+                int ecode = process.waitFor();
+
+                List<String> out = new ArrayList<>();
                 try {
-                    int ecode = process.waitFor();
-                    if (ecode != 0) {
-                        List<String> output = new ArrayList<>();
-                        try {
-                            output.addAll(Files.readAllLines(stdout.toPath()));
-                        } catch (IOException e) {
-                            output.add("Failed to read stdout: " + e.getMessage());
-                        }
-                        try {
-                            output.addAll(Files.readAllLines(stderr.toPath()));
-                        } catch (IOException e) {
-                            output.add("Failed to read stderr: " + e.getMessage());
-                        }
-                        throw new ForkFailedException(output);
-                    }
-                } catch (InterruptedException ex) {
-                    throw new ForkFailedException(ex.getMessage());
-                } finally {
-                    // The process is definitely dead, remove the temporary files.
-                    stdout.delete();
-                    stderr.delete();
+                    out.addAll(Files.readAllLines(stdout.toPath()));
+                } catch (IOException e) {
+                    out.add("Failed to read stdout: " + e.getMessage());
                 }
-                return true;
+
+                List<String> err = new ArrayList<>();
+                try {
+                    err.addAll(Files.readAllLines(stderr.toPath()));
+                } catch (IOException e) {
+                    err.add("Failed to read stderr: " + e.getMessage());
+                }
+
+                if (ecode != 0) {
+                    dumpFailure(sink, out, err);
+                } else {
+                    result.addVMOut(out);
+                    result.addVMErr(err);
+                    sink.add(result);
+                }
+            } catch (InterruptedException ex) {
+                dumpFailure(sink, Collections.singleton(ex.getMessage()), Collections.emptyList());
+            } finally {
+                // The process is definitely dead, remove the temporary files.
+                stdout.delete();
+                stderr.delete();
             }
+            return true;
         }
 
-        public synchronized TestConfig jobRequest() {
-            if (processed) {
-                return null;
+        private void dumpFailure(TestResultCollector sink, Collection<String> out, Collection<String> err) {
+            TestConfig task = getTask();
+            TestResult result = new TestResult(task, Status.VM_ERROR);
+            for (String i : out) {
+                result.addMessage(i);
             }
-            processed = true;
-            return getTask();
+            for (String i : err) {
+                result.addMessage(i);
+            }
+            sink.add(result);
         }
 
-        public TestConfig getTask() {
-            return task;
+        public void recordResult(TestResult r) {
+            if (result != null) {
+                throw new IllegalStateException("VM had already published a result.");
+            }
+            result = r;
         }
     }
 

jcstress-core/src/main/java/org/openjdk/jcstress/infra/collectors/TestResult.java

@@ -45,21 +45,41 @@ public class TestResult implements Serializable {
     private final Status status;
     private final Multiset<String> states;
     private volatile Environment env;
-    private final List<String> auxData;
+    private final List<String> messages;
+    private final List<String> vmOut;
+    private final List<String> vmErr;
 
     public TestResult(TestConfig config, Status status) {
         this.config = config;
         this.status = status;
         this.states = new HashMultiset<>();
-        this.auxData = new ArrayList<>();
+        this.messages = new ArrayList<>();
+        this.vmOut = new ArrayList<>();
+        this.vmErr = new ArrayList<>();
     }
 
     public void addState(String result, long count) {
         states.add(result, count);
     }
 
-    public void addAuxData(String data) {
-        auxData.add(data);
+    public void addMessage(String msg) {
+        messages.add(msg);
+    }
+
+    public void addVMOut(String msg) {
+        vmOut.add(msg);
+    }
+
+    public void addVMOut(Collection<String> msg) {
+        vmOut.addAll(msg);
+    }
+
+    public void addVMErr(String msg) {
+        vmErr.add(msg);
+    }
+
+    public void addVMErr(Collection<String> msg) {
+        vmErr.addAll(msg);
     }
 
     public void setEnv(Environment e) {
@@ -78,8 +98,16 @@ public Status status() {
         return status;
     }
 
-    public List<String> getAuxData() {
-        return auxData;
+    public List<String> getMessages() {
+        return messages;
+    }
+
+    public List<String> getVmOut() {
+        return vmOut;
+    }
+
+    public List<String> getVmErr() {
+        return vmErr;
     }
 
     public long getTotalCount() {

jcstress-core/src/main/java/org/openjdk/jcstress/infra/grading/HTMLReportPrinter.java

@@ -427,10 +427,10 @@ public void emitTestReport(PrintWriter o, Collection<TestResult> results, TestIn
         o.println("<h3>Auxiliary data</h3>");
 
         for (TestResult r : sorted) {
-            if (!r.getAuxData().isEmpty()) {
+            if (!r.getMessages().isEmpty()) {
                 o.println("<p><b>" + r.getConfig() + "</b></p>");
                 o.println("<pre>");
-                for (String data : r.getAuxData()) {
+                for (String data : r.getMessages()) {
                     o.println(data);
                 }
                 o.println("</pre>");

jcstress-core/src/main/java/org/openjdk/jcstress/infra/grading/ReportUtils.java

@@ -91,7 +91,7 @@ private static TestResult merged(TestConfig config, Collection<TestResult> merge
                 stateCounts.add(s, r.getCount(s));
             }
             env = r.getEnv();
-            auxData.addAll(r.getAuxData());
+            auxData.addAll(r.getMessages());
         }
 
         TestResult root = new TestResult(config, status);
@@ -103,7 +103,7 @@ private static TestResult merged(TestConfig config, Collection<TestResult> merge
         root.setEnv(env);
 
         for (String data : auxData) {
-            root.addAuxData(data);
+            root.addMessage(data);
         }
         return root;
     }
@@ -160,19 +160,63 @@ public static void printDetails(PrintWriter pw, TestResult r, boolean inProgress
     }
 
     public static void printMessages(PrintWriter pw, TestResult r) {
-        if (!r.getAuxData().isEmpty()) {
-            pw.println("    Messages: ");
-            for (String data : r.getAuxData()) {
-                if (skipMessage(data)) continue;
-                pw.println("        " + data);
+        boolean errMsgsPrinted = false;
+        for (String data : r.getMessages()) {
+            if (skipMessage(data)) continue;
+            if (!errMsgsPrinted) {
+                pw.println("    Messages: ");
+                errMsgsPrinted = true;
             }
+            pw.println("        " + data);
+        }
+        if (errMsgsPrinted) {
+            pw.println();
+        }
+
+        boolean vmOutPrinted = false;
+        for (String data : r.getVmOut()) {
+            if (skipMessage(data)) continue;
+            if (!vmOutPrinted) {
+                pw.println("    VM output stream: ");
+                vmOutPrinted = true;
+            }
+            pw.println("        " + data);
+        }
+        if (vmOutPrinted) {
+            pw.println();
+        }
+
+        boolean vmErrPrinted = false;
+        for (String data : r.getVmErr()) {
+            if (skipMessage(data)) continue;
+            if (!vmErrPrinted) {
+                pw.println("    VM error stream: ");
+                vmErrPrinted = true;
+            }
+            pw.println("        " + data);
+        }
+        if (vmErrPrinted) {
             pw.println();
         }
     }
 
     private static boolean skipMessage(String data) {
-        if (data != null && data.startsWith("Warning: 'NoSuchMethodError' on register of sun.hotspot.WhiteBox"))
+        if (data == null) {
             return true;
+        }
+
+        if (data.startsWith("Warning: 'NoSuchMethodError' on register of sun.hotspot.WhiteBox")) {
+            return true;
+        }
+
+        if (data.contains("Option MaxRAMFraction was deprecated in version") ||
+            data.contains("Option MinRAMFraction was deprecated in version")) {
+            return true;
+        }
+
+        if (data.contains("compiler directives added")) {
+            return true;
+        }
 
         return false;
     }

jcstress-core/src/main/java/org/openjdk/jcstress/infra/runners/Runner.java

@@ -97,7 +97,7 @@ public void run() {
     private TestResult prepareResult(Status status) {
         TestResult result = new TestResult(config, status);
         for (String msg : messages) {
-            result.addAuxData(msg);
+            result.addMessage(msg);
         }
         messages.clear();
         return result;
@@ -112,7 +112,7 @@ protected void dumpFailure(Status status, String message) {
     protected void dumpFailure(Status status, String message, Throwable aux) {
         messages.add(message);
         TestResult result = prepareResult(status);
-        result.addAuxData(StringUtils.getStacktrace(aux));
+        result.addMessage(StringUtils.getStacktrace(aux));
         collector.add(result);
     }
 

jcstress-core/src/test/java/org/openjdk/jcstress/EmbeddedExecutorTest.java

@@ -55,7 +55,7 @@ public void testReportTestError() throws IOException {
         TestResult testResult = sink.getTestResults().iterator().next();
         Assert.assertEquals("Should report a test error", Status.TEST_ERROR, testResult.status());
 
-        String errorMessage = testResult.getAuxData().get(0);
+        String errorMessage = testResult.getMessages().get(0);
         Assert.assertEquals("Should report the test error reason", "java.lang.ClassNotFoundException: my.missing.ClassName",
                 getFirstLine(errorMessage));
     }