JFR dump to Jcmd current working directory by default

If the filename supplied is not an absolute path, Jcmd current work
directory is used instead of the target VM location;
Throw an error if duration option doesn't have time units.

Signed-off-by: Jason Feng <[email protected]>

Author: JasonFengJ9

jcl/src/java.base/share/classes/openj9/internal/tools/attach/target/DiagnosticUtils.java

@@ -23,8 +23,12 @@
 
 package openj9.internal.tools.attach.target;
 
+import java.io.File;
 import java.io.PrintWriter;
 import java.io.StringWriter;
+import java.nio.file.InvalidPathException;
+import java.nio.file.Path;
+import java.nio.file.Paths;
 import java.util.Arrays;
 import java.util.HashMap;
 import java.util.Map;
@@ -144,6 +148,9 @@ public class DiagnosticUtils {
 	private static final String DIAGNOSTICS_JFR_START = "JFR.start";
 	private static final String DIAGNOSTICS_JFR_DUMP = "JFR.dump";
 	private static final String DIAGNOSTICS_JFR_STOP = "JFR.stop";
+
+	private static final int ERROR_NO_TIME_UNIT = -1;
+	private static final int ERROR_NO_TIME_DURATION = -2;
 /*[ENDIF] JFR_SUPPORT */
 
 	/**
@@ -209,6 +216,35 @@ public static String makeThreadPrintCommand(boolean lockedSynchronizers) {
 	 * @return formatted string
 	 */
 	public static String makeJcmdCommand(String[] options, int skip) {
+		/*[IF JFR_SUPPORT]*/
+		if (options.length >= 2) {
+			// there is a jcmd command
+			if (DIAGNOSTICS_JFR_START.equalsIgnoreCase(options[1])) {
+				// search JFR.start options
+				for (int i = 2; i < options.length; i++) {
+					String option = options[i];
+					IPC.logMessage("makeJcmdCommand: option = ", option);
+					if (option.startsWith("filename=")) {
+						String fileName = option.substring(option.indexOf("=") + 1);
+						try {
+							Path filePath = Paths.get(fileName);
+							if (!filePath.isAbsolute()) {
+								// default recording file path is jcmd current working directory
+								String jcmdPWD = Paths.get("").toAbsolutePath().toString();
+								fileName = jcmdPWD + File.separator + fileName;
+								IPC.logMessage("makeJcmdCommand: absolute filename = ", fileName);
+								// replace existing entry with an absolute jcmd pwd path for the target VM
+								options[i] = "filename=" + fileName;
+							}
+						} catch (InvalidPathException ipe) {
+							// ignore this exception and keep the original entry
+							IPC.logMessage("makeJcmdCommand: ipe = ", ipe.getMessage());
+						}
+					}
+				}
+			}
+		}
+		/*[ENDIF] JFR_SUPPORT */
 		String cmd = String.join(DIAGNOSTICS_OPTION_SEPARATOR, Arrays.asList(options).subList(skip, options.length));
 		return cmd;
 	}
@@ -415,13 +451,25 @@ private static long convertToMilliseconds(String timeValue) {
 			timeInMilli = TimeUnit.DAYS.toMillis(time);
 			break;
 		default:
-			// no unit or unrecognized unit, assume milliseconds
-			timeInMilli = time;
+			// no unit or unrecognized unit, return ERROR_NO_TIME_UNIT
+			timeInMilli = ERROR_NO_TIME_UNIT;
 			break;
 		}
 		return timeInMilli;
 	}
 
+	/**
+	 * Parse a time parameter, and return the duration in milliseconds.
+	 * If the time unit is missing, ERROR_NO_TIME_UNIT is returned.
+	 * If the paramName is not in parameters, ERROR_NO_TIME_DURATION is returned.
+	 *
+	 * @param paramName the parameter name
+	 * @param parameters the parameter array
+	 *
+	 * @return the duration in milliseconds,
+	 *         ERROR_NO_TIME_UNIT if no time unit,
+	 *         ERROR_NO_TIME_DURATION if paramName wasn't found.
+	 */
 	private static long parseTimeParameter(String paramName, String[] parameters) {
 		for (String param : parameters) {
 			if (param.startsWith(paramName + "=")) {
@@ -431,7 +479,7 @@ private static long parseTimeParameter(String paramName, String[] parameters) {
 				}
 			}
 		}
-		return -1;
+		return ERROR_NO_TIME_DURATION;
 	}
 
 	private static String parseStringParameter(String paramName, String[] parameters, String defaultValue) {
@@ -448,35 +496,33 @@ private static String parseStringParameter(String paramName, String[] parameters
 
 	private static DiagnosticProperties doJFR(String diagnosticCommand) {
 		DiagnosticProperties result = null;
-
 		// split the command and arguments
 		String[] parts = diagnosticCommand.split(DIAGNOSTICS_OPTION_SEPARATOR);
 		IPC.logMessage("doJFR: ", diagnosticCommand);
-
 		// ensure there's at least one part for the command
 		if (parts.length == 0) {
 			return DiagnosticProperties.makeErrorProperties("Error: No JFR command specified");
 		}
-
 		String command = parts[0].trim();
 		String[] parameters = Arrays.copyOfRange(parts, 1, parts.length);
-
 		String fileName = parseStringParameter("filename", parameters, null);
 		IPC.logMessage("doJFR: filename = ", fileName);
-		boolean setFileName = (fileName != null) && !fileName.isEmpty();
-		if (setFileName) {
-			if (!VM.setJFRRecordingFileName(fileName)) {
-				return DiagnosticProperties.makeErrorProperties("setJFRRecordingFileName failed");
-			} else {
-				jfrRecordingFileName = fileName;
-			}
-		}
 
 		try {
 			if (command.equalsIgnoreCase(DIAGNOSTICS_JFR_START)) {
 				if (VM.isJFRRecordingStarted()) {
 					result = DiagnosticProperties.makeErrorProperties("One JFR recording is in progress [" + jfrRecordingFileName + "], only one recording is allowed at a time.");
 				} else {
+					// only JFR.start command is allowed to change the recording filename
+					boolean setFileName = (fileName != null) && !fileName.isEmpty();
+					if (setFileName) {
+						// the recording filename should be set before VM.startJFR() which invokes JFRWriter:openJFRFile()
+						if (!VM.setJFRRecordingFileName(fileName)) {
+							return DiagnosticProperties.makeErrorProperties("setJFRRecordingFileName() failed");
+						} else {
+							jfrRecordingFileName = fileName;
+						}
+					}
 					VM.startJFR();
 					long duration = parseTimeParameter("duration", parameters);
 					IPC.logMessage("doJFR: duration = " + duration);
@@ -488,6 +534,8 @@ public void run() {
 							}
 						};
 						timer.schedule(jfrDumpTask, duration);
+					} if (duration == ERROR_NO_TIME_UNIT) {
+						return DiagnosticProperties.makeErrorProperties("The duration doesn't have a time unit.");
 					} else {
 						// the recording is on until JFR.stop
 					}

runtime/vm/jfr.cpp

@@ -1127,12 +1127,15 @@ jfrSamplingThreadProc(void *entryArg)
 jboolean
 setJFRRecordingFileName(J9JavaVM *vm, char *fileName)
 {
-	UDATA defaultFileNameLen = strlen(DEFAULT_JFR_FILE_NAME);
-	if ((defaultFileNameLen != strlen(vm->jfrState.jfrFileName))
-	|| (0 != strncmp(DEFAULT_JFR_FILE_NAME, vm->jfrState.jfrFileName, defaultFileNameLen))
-	) {
-		PORT_ACCESS_FROM_JAVAVM(vm);
-		j9mem_free_memory(vm->jfrState.jfrFileName);
+	if (NULL != vm->jfrState.jfrFileName) {
+		UDATA defaultFileNameLen = strlen(DEFAULT_JFR_FILE_NAME);
+		const char *jfrFileName = vm->jfrState.jfrFileName;
+		if ((defaultFileNameLen != strlen(jfrFileName))
+				|| (0 != strncmp(DEFAULT_JFR_FILE_NAME, jfrFileName, defaultFileNameLen))
+		) {
+			PORT_ACCESS_FROM_JAVAVM(vm);
+			j9mem_free_memory(vm->jfrState.jfrFileName);
+		}
 	}
 	vm->jfrState.jfrFileName = fileName;
 	VM_JFRWriter::closeJFRFile(vm);