RFR: 8080: Add support for enabling jfr on native images

Christoph Langer clanger at openjdk.org
Tue Jun 13 06:51:51 UTC 2023


On Thu, 8 Jun 2023 14:01:20 GMT, Alex Macdonald <aptmac at openjdk.org> wrote:

> This PR addresses JMC-8080 [[0]](https://bugs.openjdk.org/browse/JMC-8080), in which it would be nice to add support for enabling flight recorder on GraalVM native images.
> 
> Currently, trying to use the flight recording wizard on a connected native image ends up with an error dialog:
> ![native-image](https://github.com/openjdk/jmc/assets/10425301/a04b49db-922c-451e-89d8-0cc951e3bbda)
> 
> The proposed solution here adds checks to determine if the vm is of type Substrate VM, and check if there is a flightrecording mbean registered.
> 
> [0] https://bugs.openjdk.org/browse/JMC-8080

Looks good overall, just two minor comments.

application/org.openjdk.jmc.rjmx/src/main/java/org/openjdk/jmc/rjmx/ConnectionToolkit.java line 378:

> 376: 	 */
> 377: 	public static boolean isSubstrateVM(IConnectionHandle connectionHandle) {
> 378: 		String vmName = getVMName(connectionHandle);

There's also a null check in JavaVMVersionToolkit.isSubstrateVMName, so we can skip it here. But probably it's ok to keep as well.

core/org.openjdk.jmc.common/src/main/java/org/openjdk/jmc/common/version/JavaVMVersionToolkit.java line 125:

> 123: 	 *            the VM name to check.
> 124: 	 * @return {@code true} if it is a Substrate VM, {@code false} if it isn't or if was not
> 125: 	 *         possible to tell.

text: "... if it isn't or if it was not ..." (another it required)

-------------

Marked as reviewed by clanger (Committer).

PR Review: https://git.openjdk.org/jmc/pull/491#pullrequestreview-1476367900
PR Review Comment: https://git.openjdk.org/jmc/pull/491#discussion_r1227606973
PR Review Comment: https://git.openjdk.org/jmc/pull/491#discussion_r1227605207


More information about the jmc-dev mailing list