[10] RFR(M) 8182701: Modify JVMCI to allow Graal Compiler to expose platform MBean

mandy chung mandy.chung at oracle.com
Tue Aug 22 16:07:08 UTC 2017

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.
> Thanks for the review.
> -jt

More information about the core-libs-dev mailing list