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

Alex Macdonald aptmac at openjdk.org
Tue Jun 13 13:37:53 UTC 2023


On Tue, 13 Jun 2023 06:44:45 GMT, Christoph Langer <clanger 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
>
> 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.

Nice catch, although it's setup this way for similar functions so I might side with keeping it as is. The difference being that with ConnectionToolkit we can check the VM given a connection handle, whereas JavaVMVersionToolkit just does the string comparison. It also just so happens that ConnectionToolkit will grab the vm name and delegate to JavaVMVersionToolkit, but they're both public and suit different needs.

`JVMType.getJVMType()` uses the `JavaVMVersionToolkit.is*VMName()` and it might be nice to keep the null check for any potential problems there.

> 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)

Nice catch, haha I had copied the text from the neighbouring functions, I'll touch those up as well.

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

PR Review Comment: https://git.openjdk.org/jmc/pull/491#discussion_r1228144210
PR Review Comment: https://git.openjdk.org/jmc/pull/491#discussion_r1228145043


More information about the jmc-dev mailing list