RFR - [RFE] JDK-8007141 : Introduce Dynamic MBean exposing the perf counters

[Daniel Fuchs]

Hi Harsha,

Not a review either (at least not a complete one).

PlatformMBeanProviderImpl:

  281             @Override
  282             public Set<Class<? extends DynamicMBean>> 
mbeanInterfaces() {
  283                 return Collections.emptySet();
  284             }

For future maintainers, I think it would be good to copy
the comment at line 250 and insert it before line 283 too:
  250                     // DynamicMBean cannot be used to find an 
MBean by ManagementFactory

HotSpotPerfCounterMBean:

Exception handling: I have the feeling that this part might
be a bit over-engineered. If you look at javax.management.StandardMBean
(which is a canonical implementation of DynamicMBean) and
then transitively at MBeanSupport and PerInterface where this class
eventually delegates to, you will see that:

1. You can throw NPE in setAttribute when attribute == null (no need for
    all the wrapping)
2. You should throw AttributeNotFoundException when attempting to read
    a write-only attribute or write a read-only attribute
3. You can throw NPE if AttributeList is null

  179             String typeName = "java.lang.String";

That looks like a hack.

What if at some point the value of that attribute becomes non null,
and its class is *not* java.lang.String?
Then MBeanAttributeInfo would be lying.
If that ever happens - then it would be more consistent to coerce
the value to its declared type (String) before sending it as a
a result for getAttribute.

Or better, find a way to figure out what should be the class name
of the value even if the value is null (isn't there some metadata
available for these perf counters somewhere?).


PerfCounterMBeanTest:

testGetAttribute should verify that class of the returned value
corresponds to what was declared in the MBeanAttributeInfo for the
corresponding attribute.

cheers,

-- daniel

On 26/02/17 16:50, Harsha Wardhana B wrote:
> Hi All,
>
> Please review the below RFE,
>
> JDK-8007141 <http://JDK-8007141> : Introduce Dynamic MBean exposing the
> perf counters.
>
> with webrev at,
>
> http://cr.openjdk.java.net/~hb/8007141/webrev.00/
>
> Appreciate if I can get inputs for below.
>
> 1. Location of HotSpotPerfCounterMBean. It is located at
> src/jdk.management/share/classes/com/sun/management/internal. It can be
> moved to src/jdk.management/share/classes/com/sun/management if required.
>
> 2. Javadoc comments for HotSpotPerfCounterMBean. Not sure if it is adequate.
>
> 3. Description for each attribute of MBean. I am using  getUnits(),
> getVariability(), and getFlags() for description. I am not sure if that
> is the right description. Appreciate inputs from someone who knows perf
> counters well.
>
> Thanks
>
> Harsha
>
>