eclipse-openj9 / openj9

Eclipse OpenJ9: A Java Virtual Machine for OpenJDK that's optimized for small footprint, fast start-up, and high throughput. Builds on Eclipse OMR (https://github.com/eclipse/omr) and combines with the Extensions for OpenJDK for OpenJ9 repo.
Other
3.27k stars 720 forks source link

OpenJ9DiagnosticsMXBeanImpl should work in the absence of the openj9.jvm module #6399

Open keithc-ca opened 5 years ago

keithc-ca commented 5 years ago

The implementation should not depend upon com.ibm.jvm.Dump but instead have its own native methods which can reuse code used by the native methods of com.ibm.jvm.Dump.

pshipton commented 5 years ago

Note a similar change will be required for jcmd https://github.com/eclipse/openj9/issues/5164

pdbain-ibm commented 5 years ago

The change in question is https://github.com/eclipse/openj9/pull/6589.

pdbain-ibm commented 5 years ago

OpenJ9DiagnosticsMXBeanImpl calls a number of Dump's non-native methods too.

pshipton commented 5 years ago

@keithc-ca shouldn't openj9.jvm just be a prereq of jdk.managment?

JasonFengJ9 commented 5 years ago

https://github.com/eclipse/openj9/blob/4df4d440243b69e46db1109575f2bc20e7de9fd5/jcl/src/jdk.management/share/classes/openj9/lang/management/internal/OpenJ9DiagnosticsMXBeanImpl.java#L27-L37

com.ibm.jvm.Dump is referenced only in Java 8 which has not modules.

pshipton commented 5 years ago

@JasonFengJ9 https://github.com/eclipse/openj9/blob/4df4d440243b69e46db1109575f2bc20e7de9fd5/jcl/src/jdk.management/share/classes/openj9/lang/management/internal/OpenJ9DiagnosticsMXBeanImpl.java#L317-L329

pdbain-ibm commented 5 years ago

The Dump methods are used in Java 9 and up via Method objects.

pdbain-ibm commented 5 years ago

Could not jdk.management require openj9.jvm and call those methods directly, since they are public?

keithc-ca commented 5 years ago

module-info.java.extra cannot add extra requires clauses.

pdbain-ibm commented 5 years ago

jdk.management requires java.management, which contains package com.ibm.java.lang.management.internal. I suggest making openj9.jvm require java.management and moving the implementations of the com.ibm.jvm.Dump methods into com.ibm.java.lang.management.internal. That removes the disparity between Java 8 and modular Java in OpenJ9DiagnosticsMXBeanImpl.

pdbain-ibm commented 5 years ago

I took a crack at it and it looks feasible. Here is the work in progress: https://github.com/pdbain-ibm/openj9/tree/mxbean. The native changes are bogus but should be easy to fix.