On 8/22/17 6:19 AM, Jaroslav Tulach wrote:
Thanks for your comments, Mandy.
On pondělí 21. srpna 2017 12:42:09 CEST mandy chung wrote:
cc'ing serviceability-dev which is the right mailing list for platform
management discussion.
JVMCI is currently named as `jdk.internal.vm.ci` (a JDK internal
module). I suppose this new module is intended to be kept as an
internal module?
The module doesn't export any API - e.g. it is internal. The current name is
jdk.vm.ci.management - shall I change that to something like
jdk.internal.vm.ci.management? Or some other (shorter) suggestion?
Will get back to you on any suggestion.
JVMCIMBeans.java
55 @Override
56 public Set<Class<?>> mbeanInterfaces() {
57 return Collections.singleton(mbean.getClass());
58 }
This mbeanInterfaces method should return the MBean interface class but
not the class of the mbean implementation. This allows the platform
mbean to be looked up from ManagementFactory.getPlatformMXBean. If this
is a dynamic mbean, then this method simply returns an empty list (see
DiagnosticCommandMBean).
I am almost 100% sure that the bean(s) exposed from Truffle is going to be
DynamicMBean - we compose the attributes dynamically, that wouldn't work with
reflection.
I'll change the code to return an empty list if the bean is DynamicMBean.
To support standard mbean,
HotSpotJVMCICompilerFactory::mbeans method would need to include the
mbean interface type in addition to the name and the mbean
implementation object, i.e. may need to define a specific type for it.
As we don't have any use for this yet I suggest to keep things simple.
Since this is mainly for Truffle, keeping it simple is good.
I believe following contract could work:
public Set<Class<?>> mbeanInterfaces() {
if (mbean instanceof DynamicMBean) {
return Collections.emptySet();
} else {
Class<?>[] interfaces = mbean.getClass().getInterfaces();
return new HashSet<>(Arrays.asList(interfaces));
}
}
This isn't exactly right when there is one DynamicMBean and one
StandardMBean. What about making this version only supporting
DynamicMBean - i.e. the provider constructor throws IAE if the mbean
instance is not DynamicMBean and mbeanInterfaces method simply returns
an empty set. This could be enhanced in the future if there is a need
to support standard mbean.
if this is acceptable, I would change documentation of the mbean() method to
mention how the set of mbeanInterfaces() is computed. OK?
A comment would help.
Mandy
Thanks for the review.
-jt
