8204571: Add support for launching multiple startup recordings
authormgronlun
Tue, 19 Jun 2018 19:16:08 +0200
changeset 50664 857ce291c70c
parent 50663 0e9d1d4ab692
child 50665 9eaaa711fef5
8204571: Add support for launching multiple startup recordings Reviewed-by: egahlin
src/hotspot/share/jfr/recorder/jfrRecorder.cpp
src/hotspot/share/jfr/recorder/service/jfrOptionSet.cpp
src/hotspot/share/jfr/recorder/service/jfrOptionSet.hpp
test/jdk/jdk/jfr/jvm/TestLogOutput.java
test/jdk/jdk/jfr/startupargs/TestMultipleStartupRecordings.java
--- a/src/hotspot/share/jfr/recorder/jfrRecorder.cpp	Tue Jun 19 10:12:35 2018 -0700
+++ b/src/hotspot/share/jfr/recorder/jfrRecorder.cpp	Tue Jun 19 19:16:08 2018 +0200
@@ -87,10 +87,28 @@
 
 static JfrStartFlightRecordingDCmd* _startup_recording = NULL;
 
+static void release_startup_recording() {
+  if (_startup_recording != NULL) {
+    delete _startup_recording;
+    _startup_recording = NULL;
+  }
+}
+
+static void teardown_startup_support() {
+  release_startup_recording();
+  JfrOptionSet::release_startup_recordings();
+}
+
+
 // Parsing options here to detect errors as soon as possible
-static bool parse_startup_recording(TRAPS) {
-  assert(StartFlightRecording != NULL, "invariant");
-  CmdLine cmdline(StartFlightRecording, strlen(StartFlightRecording), true);
+static bool parse_recording_options(const char* options, TRAPS) {
+  assert(options != NULL, "invariant");
+  if (_startup_recording != NULL) {
+    delete _startup_recording;
+  }
+  CmdLine cmdline(options, strlen(options), true);
+  _startup_recording = new (ResourceObj::C_HEAP, mtTracing) JfrStartFlightRecordingDCmd(tty, true);
+  assert(_startup_recording != NULL, "invariant");
   _startup_recording->parse(&cmdline, ',', THREAD);
   if (HAS_PENDING_EXCEPTION) {
     java_lang_Throwable::print(PENDING_EXCEPTION, tty);
@@ -100,31 +118,61 @@
   return true;
 }
 
-static bool initialize_startup_recording(TRAPS) {
-  if (StartFlightRecording != NULL) {
-    _startup_recording = new (ResourceObj::C_HEAP, mtTracing) JfrStartFlightRecordingDCmd(tty, true);
-    return _startup_recording != NULL && parse_startup_recording(THREAD);
+static bool validate_recording_options(TRAPS) {
+  const GrowableArray<const char*>* startup_options = JfrOptionSet::startup_recordings();
+  if (startup_options == NULL) {
+    return true;
+  }
+  const int length = startup_options->length();
+  assert(length >= 1, "invariant");
+  for (int i = 0; i < length; ++i) {
+    if (!parse_recording_options(startup_options->at(i), THREAD)) {
+      return false;
+    }
   }
   return true;
 }
 
-static bool startup_recording(TRAPS) {
-  if (_startup_recording == NULL) {
-    return true;
-  }
-  log_trace(jfr, system)("Starting up Jfr startup recording");
+static bool launch_recording(TRAPS) {
+  assert(_startup_recording != NULL, "invariant");
+  log_trace(jfr, system)("Starting a recording");
   _startup_recording->execute(DCmd_Source_Internal, Thread::current());
-  delete _startup_recording;
-  _startup_recording = NULL;
   if (HAS_PENDING_EXCEPTION) {
-    log_debug(jfr, system)("Exception while starting Jfr startup recording");
+    log_debug(jfr, system)("Exception while starting a recording");
     CLEAR_PENDING_EXCEPTION;
     return false;
   }
-  log_trace(jfr, system)("Finished starting Jfr startup recording");
+  log_trace(jfr, system)("Finished starting a recording");
   return true;
 }
 
+static bool launch_recordings(const GrowableArray<const char*>* startup_options, TRAPS) {
+  assert(startup_options != NULL, "invariant");
+  const int length = startup_options->length();
+  assert(length >= 1, "invariant");
+  if (length == 1) {
+    // already parsed and ready, launch it
+    return launch_recording(THREAD);
+  }
+  for (int i = 0; i < length; ++i) {
+    parse_recording_options(startup_options->at(i), THREAD);
+    if (!launch_recording(THREAD)) {
+      return false;
+    }
+  }
+  return true;
+}
+
+static bool startup_recordings(TRAPS) {
+  const GrowableArray<const char*>* startup_options = JfrOptionSet::startup_recordings();
+  if (startup_options == NULL) {
+    return true;
+  }
+  const bool ret = launch_recordings(startup_options, THREAD);
+  teardown_startup_support();
+  return ret;
+}
+
 static void log_jdk_jfr_module_resolution_error(TRAPS) {
   LogTarget(Error, jfr, system) lt_error;
   LogTargetHandle handle(lt_error);
@@ -141,7 +189,7 @@
   if (!register_jfr_dcmds()) {
     return false;
   }
-  if (!initialize_startup_recording(thread)) {
+  if (!validate_recording_options(thread)) {
     return false;
   }
   if (in_graph) {
@@ -159,7 +207,7 @@
     log_jdk_jfr_module_resolution_error(thread);
     return false;
   }
-  return startup_recording(thread);
+  return startup_recordings(thread);
 }
 
 static bool _created = false;
--- a/src/hotspot/share/jfr/recorder/service/jfrOptionSet.cpp	Tue Jun 19 10:12:35 2018 -0700
+++ b/src/hotspot/share/jfr/recorder/service/jfrOptionSet.cpp	Tue Jun 19 19:16:08 2018 +0200
@@ -34,6 +34,7 @@
 #include "runtime/thread.inline.hpp"
 #include "services/diagnosticArgument.hpp"
 #include "services/diagnosticFramework.hpp"
+#include "utilities/growableArray.hpp"
 #include "utilities/ostream.hpp"
 
 struct ObsoleteOption {
@@ -664,42 +665,51 @@
   return true;
 }
 
-/*
-
-to support starting multiple startup recordings
-
-static const char* start_flight_recording_option_original = NULL;
-static const char* flight_recorder_option_original = NULL;
-
-static void copy_option_string(const JavaVMOption* option, const char** addr) {
-  assert(option != NULL, "invariant");
-  assert(option->optionString != NULL, "invariant");
-  const size_t length = strlen(option->optionString);
-  *addr = JfrCHeapObj::new_array<char>(length + 1);
-  assert(*addr != NULL, "invarinat");
-  strncpy((char*)*addr, option->optionString, length + 1);
-  assert(strncmp(*addr, option->optionString, length + 1) == 0, "invariant");
-}
-
-copy_option_string(*option, &start_flight_recording_option_original);
-copy_option_string(*option, &flight_recorder_option_original);
-*/
+static GrowableArray<const char*>* startup_recording_array = NULL;
 
 bool JfrOptionSet::parse_start_flight_recording_option(const JavaVMOption** option, char* tail) {
   assert(option != NULL, "invariant");
   assert(tail != NULL, "invariant");
   assert((*option)->optionString != NULL, "invariant");
   assert(strncmp((*option)->optionString, "-XX:StartFlightRecording", 24) == 0, "invariant");
+  const char* param_string = NULL;
   if (*tail == '\0') {
     // Add dummy dumponexit=false so -XX:StartFlightRecording can be used without a parameter.
     // The existing option->optionString points to stack memory so no need to deallocate.
     const_cast<JavaVMOption*>(*option)->optionString = (char*)"-XX:StartFlightRecording=dumponexit=false";
+    param_string = (*option)->optionString + 25;
   } else {
     *tail = '='; // ":" -> "="
+    param_string = tail + 1;
   }
+  assert(param_string != NULL, "invariant");
+  const size_t param_length = strlen(param_string);
+
+  if (startup_recording_array == NULL) {
+    startup_recording_array = new (ResourceObj::C_HEAP, mtTracing) GrowableArray<const char*>(8, true, mtTracing);
+  }
+  assert(startup_recording_array != NULL, "invariant");
+  char* startup_options = NEW_C_HEAP_ARRAY(char, param_length + 1, mtTracing);
+  strncpy(startup_options, param_string, strlen(param_string) + 1);
+  assert(strncmp(param_string, startup_options, param_length) == 0, "invariant");
+  startup_recording_array->append(startup_options);
   return false;
 }
 
+const GrowableArray<const char*>* JfrOptionSet::startup_recordings() {
+  return startup_recording_array;
+}
+
+void JfrOptionSet::release_startup_recordings() {
+  if (startup_recording_array != NULL) {
+    for (int i = 0; i < startup_recording_array->length(); ++i) {
+      FREE_C_HEAP_ARRAY(char, startup_recording_array->at(i));
+    }
+  }
+  delete startup_recording_array;
+  DEBUG_ONLY(startup_recording_array = NULL;)
+}
+
 bool JfrOptionSet::parse_flight_recorder_option(const JavaVMOption** option, char* tail) {
   assert(option != NULL, "invariant");
   assert(tail != NULL, "invariant");
--- a/src/hotspot/share/jfr/recorder/service/jfrOptionSet.hpp	Tue Jun 19 10:12:35 2018 -0700
+++ b/src/hotspot/share/jfr/recorder/service/jfrOptionSet.hpp	Tue Jun 19 19:16:08 2018 +0200
@@ -29,6 +29,9 @@
 #include "memory/allocation.hpp"
 #include "utilities/exceptions.hpp"
 
+template <typename>
+class GrowableArray;
+
 //
 // Command-line options and defaults
 //
@@ -78,6 +81,8 @@
   static bool parse_start_flight_recording_option(const JavaVMOption** option, char* tail);
   static bool parse_flight_recorder_option(const JavaVMOption** option, char* tail);
 
+  static const GrowableArray<const char*>* startup_recordings();
+  static void release_startup_recordings();
 };
 
 #endif // SHARE_VM_JFR_RECORDER_SERVICE_JFROPTIONSET_HPP
--- a/test/jdk/jdk/jfr/jvm/TestLogOutput.java	Tue Jun 19 10:12:35 2018 -0700
+++ b/test/jdk/jdk/jfr/jvm/TestLogOutput.java	Tue Jun 19 19:16:08 2018 +0200
@@ -42,7 +42,7 @@
     public static void main(String[] args) throws Exception {
         final String fileName = "jfr_trace.txt";
         final List<String>findWhat = new ArrayList<>();
-        findWhat.add("Starting up Jfr startup recording");
+        findWhat.add("Starting a recording");
         findWhat.add("Flight Recorder initialized");
         boolean passed = false;
         List<String> matches = new ArrayList<String>(findWhat);
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/test/jdk/jdk/jfr/startupargs/TestMultipleStartupRecordings.java	Tue Jun 19 19:16:08 2018 +0200
@@ -0,0 +1,108 @@
+/*
+ * Copyright (c) 2018, 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 jdk.jfr.startupargs;
+
+import jdk.test.lib.Asserts;
+import jdk.test.lib.process.OutputAnalyzer;
+import jdk.test.lib.process.ProcessTools;
+
+/*
+ * @test
+ * @key jfr
+ *
+ * @library /test/lib
+ *
+ * @run main jdk.jfr.startupargs.TestMultipleStartupRecordings
+ */
+public class TestMultipleStartupRecordings {
+
+    private static final String START_FLIGHT_RECORDING = "-XX:StartFlightRecording";
+    private static final String FLIGHT_RECORDER_OPTIONS = "-XX:FlightRecorderOptions";
+
+    static class MainClass {
+        public static void main(String[] args) {
+        }
+    }
+
+    private static void test(ProcessBuilder pb, String... expectedOutputs) throws Exception {
+        OutputAnalyzer output = new OutputAnalyzer(pb.start());
+        for (String s : expectedOutputs) {
+            output.shouldContain(s);
+        }
+    }
+
+    private static void launchUnary(String options) throws Exception {
+        String recording1 = START_FLIGHT_RECORDING + (options != null ? options : "");
+        ProcessBuilder pb = ProcessTools.createJavaProcessBuilder(true, recording1, MainClass.class.getName());
+        test(pb, "Started recording 1");
+    }
+
+    private static void launchBinary(String options1, String options2) throws Exception {
+        String recording1 = START_FLIGHT_RECORDING + (options1 != null ? options1 : "");
+        String recording2 = START_FLIGHT_RECORDING + (options2 != null ? options2 : "");
+        ProcessBuilder pb = ProcessTools.createJavaProcessBuilder(true, recording1, recording2, MainClass.class.getName());
+        test(pb, "Started recording 1", "Started recording 2");
+    }
+
+    private static void launchTernary(String options1, String options2, String options3) throws Exception {
+        String recording1 = START_FLIGHT_RECORDING + (options1 != null ? options1 : "");
+        String recording2 = START_FLIGHT_RECORDING + (options2 != null ? options2 : "");
+        String recording3 = START_FLIGHT_RECORDING + (options3 != null ? options3 : "");
+        ProcessBuilder pb = ProcessTools.createJavaProcessBuilder(true, recording1, recording2, recording3, MainClass.class.getName());
+        test(pb, "Started recording 1", "Started recording 2", "Started recording 3");
+    }
+
+    private static void testDefault() throws Exception {
+        System.out.println("testDefault");
+        launchUnary(null);
+        launchBinary(null, null);
+        launchTernary(null, null, null);
+    }
+
+    private static void testColonDelimited() throws Exception {
+        launchBinary(":name=myrecording1,filename=myrecording1.jfr", ":filename=myrecording2.jfr,name=myrecording2");
+    }
+
+    private static void testMixed() throws Exception {
+        launchTernary(":maxage=2d,maxsize=5GB", "=dumponexit=true,maxage=10m,", ":name=myrecording,maxage=10m,filename=myrecording.jfr,disk=false");
+    }
+
+    private static void testWithFlightRecorderOptions() throws Exception {
+        System.out.println("testWithFlightRecorderOptions");
+        String flightRecorderOptions = FLIGHT_RECORDER_OPTIONS + "=maxchunksize=8m";
+        String recording1 = START_FLIGHT_RECORDING + "=filename=recording1.jfr";
+        String recording2 = START_FLIGHT_RECORDING + "=name=myrecording,filename=recording2.jfr";
+        ProcessBuilder pb = ProcessTools.createJavaProcessBuilder(true, flightRecorderOptions, recording1, recording2, MainClass.class.getName());
+        test(pb, "Started recording 1", "Started recording 2");
+    }
+
+    public static void main(String[] args) throws Exception {
+        testDefault();
+        testColonDelimited();
+        testMixed();
+        testWithFlightRecorderOptions();
+    }
+}