RFR: 7069: Move rjmx bundle from application to core [v7]

Christoph Langer clanger at openjdk.org
Tue Dec 5 08:21:50 UTC 2023


On Mon, 4 Dec 2023 22:03:58 GMT, Alex Macdonald <aptmac at openjdk.org> wrote:

>> This PR addresses JMC-6627 [[0]](https://bugs.openjdk.org/browse/JMC-6627) in which it would be nice to bring some bundles from application to core. Most notably, **flightrecorder.configuration** which has a PR open for review [[1]](https://github.com/openjdk/jmc/pull/469) and is also tracked by JMC-7307 [[2]](https://bugs.openjdk.org/browse/JMC-7307), and **RJMX** which is also tracked by JMC-7069 [[3]](https://bugs.openjdk.org/browse/JMC-7069).
>> 
>> While #469 is still under review, I copied a branch of it and squashed all the commits down into one commit [[4]](https://github.com/openjdk/jmc/commit/72c08b3f65671d16fcbb1333a5782e10ac9c874f) for easier rebasing. Review comments for flightrecorder.configuration should go towards #469.  RJMX does have reliance on the flightrecorder.configuration changes (and some still in ui.common), so the second commit [[5]](https://github.com/openjdk/jmc/commit/f1c828dd46f24d9075cd3eedbf5a15bb9daf9a72) here is where this PR really starts. Once #469 is merged I can rebase this PR so that it handles RJMX.
>> 
>> As mentioned in JMC-6627 [[0]](https://bugs.openjdk.org/browse/JMC-6627), because of the way Eclipse is intertwined with the RJMX code, this isn't a straightforward movement of classes. There are some parts that will need to stay on the side of jmc/application, but much of the code can come over to jmc/core with modification. This is accomplished by having a `rjmx.common` module in core, and keeping regular `rjmx` in application.
>> 
>> There are a handful of classes (e.g., `NameConverter`, `SyntheticAttributeRepository`, `SyntheticNotificationRepository`, ..) that use Eclipse to initialize them by parsing through values in plugin.xml, or use classes (like Persistence and Preferences) that are closely tied to the Eclipse RCP side of things. I brought over these classes to core and kept the code that initializes the values from extensions on the application side, so they could be set from application -> core when running JMC, but for third-party applications that want to use the RJMX code then they might not need/require this initialization.
>> 
>> [0] https://bugs.openjdk.org/browse/JMC-6627
>> [1] https://github.com/openjdk/jmc/pull/469
>> [2] https://bugs.openjdk.org/browse/JMC-7307
>> [3] https://bugs.openjdk.org/browse/JMC-7069
>> [4] https://github.com/openjdk/jmc/commit/72c08b3f65671d16fcbb1333a5782e10ac9c874f
>> [5] https://github.com/openjdk/jmc/commit/f1c828dd46f24d9075cd3eedbf5a15bb9daf9a72
>
> Alex Macdonald has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Address initial review comments

I spotted a few Javadoc errors. You'll find them in the logs when building core.

core/org.openjdk.jmc.common/src/main/java/org/openjdk/jmc/common/labelingrules/NameConverter.java line 48:

> 46: 
> 47: /**
> 48:  * Converts names according to rules specified with the {@value #LABELING_RULES_EXTENSION_POINT}

This causes a Javadoc error, same in line 143 ` * {@value #LABELING_RULES_EXTENSION_POINT} extension point.`

core/org.openjdk.jmc.flightrecorder.configuration/src/main/java/org/openjdk/jmc/flightrecorder/configuration/IFlightRecorderService.java line 50:

> 48: 
> 49: /**
> 50:  * This is the interface for the JDK Flight Recorder controller.

Line 53 below has a Javadoc error: `error: reference not found
 * {@link IConnectionHandle#getServiceOrNull(Class)}` and there are a few more Javadoc errors in this file.

core/org.openjdk.jmc.rjmx.common/src/main/java/org/openjdk/jmc/rjmx/common/ISyntheticNotification.java line 36:

> 34: 
> 35: import javax.management.MBeanServerConnection;
> 36: import javax.management.modelmbean.ModelMBeanNotificationBroadcaster;

Line 63 uses unsupported tag <tt> for Javadoc.

core/org.openjdk.jmc.rjmx.common/src/main/java/org/openjdk/jmc/rjmx/common/RJMXCorePlugin.java line 40:

> 38: 
> 39: /**
> 40:  * There is one instance of the RJMX plugin available from {@link RJMXPlugin#getDefault()}. The

This line causes a Javadoc error.

core/org.openjdk.jmc.rjmx.common/src/main/java/org/openjdk/jmc/rjmx/common/services/jfr/package-info.java line 5:

> 3:  * 
> 4:  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
> 5:  *

Line 67-69 below exhibit Javadoc errors.

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

Marked as reviewed by clanger (Committer).

PR Review: https://git.openjdk.org/jmc/pull/531#pullrequestreview-1764287671
PR Review Comment: https://git.openjdk.org/jmc/pull/531#discussion_r1415090339
PR Review Comment: https://git.openjdk.org/jmc/pull/531#discussion_r1415095911
PR Review Comment: https://git.openjdk.org/jmc/pull/531#discussion_r1415105274
PR Review Comment: https://git.openjdk.org/jmc/pull/531#discussion_r1415100667
PR Review Comment: https://git.openjdk.org/jmc/pull/531#discussion_r1415103135


More information about the jmc-dev mailing list