6610094: Add generic support for platform MXBeans of any type

[Daniel Fuchs]

Hy Mandy,

Here are my comment.

1) ManagementFactory.java: lines 103-106
    I am not sure I really like the idea of adding new methods
    to existing interfaces.

2) Logging.java: The implementation of getObjectName() is
    strange - I would have expected the ObjectName to be stored in
    a static final field.
    ObjectNames are immutable, so it is safe to have a static - you
    don't need to return a new object each time.

    Eamonn as recently added a public internal
    utility method to create an ObjectName from a String
    (com.sun.jmx.mbeanserver.Util.newObjectName(String))
http://hg.openjdk.java.net/jdk7/tl/jdk/file/2965459a8ee7/src/share/classes/com/sun/jmx/mbeanserver/Util.java
    so you could probably call that to initialize your static
    field...

3) ClassLoadingImpl.java, CompilationImpl.java, HotSpotDiagnostic.java,
    MemoryImpl.java, OperatingSystemImpl.java, RuntimeImpl.java,
    ThreadImpl.java

    OK I see you're using another Util.newObjectName() here - but
    the ObjectName could still be a static final field in these classes.

4) I see that sun.management.ManagementFactory is still there.
    Shouldn't it be merged with ManagementFactoryHelper?

5) I was a bit puzzled by the fact that PlatformComponent
    defines several components which contain each a
    different implementation of the same MBean (all sharing the
    same ObjectName).

    For instance if we look at OPRATING_SYSTEM, SUN_OPERATING_SYSTEM,
    SUN_UNIX_OPERATING_SYSTEM, it took me a while to understand that
    registration in getPlatformMBeanServer() worked because all these
    enum pointed towards the same MBean instance...

    Maybe some comment would have helped... ;-)

6) Maybe there should be a special case in the unit test which
    should try to guess which of the  *OPERATING_SYSTEM case should
    be implemented, and verify that it is indeed the
    correct implementation which is registered, and compare with
    the result of the three calls:
 
ManagementFactory.getPlatformMXBeans(java.lang.management.OperatingSystemMXBean.class);
ManagementFactory.getPlatformMXBeans(com.sun.management.OperatingSystemMXBean.class);
ManagementFactory.getPlatformMXBeans(com.sun.management.UnixOperatingSystemMXBean.class);

Best regards,

-- daniel
http://blogs.sun.com/jmxetc



Mandy Chung wrote:
> Please review the fix for:
>   6610094: Add generic support for platform MXBeans of any type
> 
> Problem:
> 
> The java.lang.management API defines the management interfaces for the Java
> virtual machine.  The management interface for other components in the 
> platform
> will reside in its own package.  For example, the management interface 
> for the
> logging facility is java.util.logging.LoggingMXBean.
> 
> There is no easy way to find out what management interfaces are defined in
> the Java platform.  In addition, when a new management interface is 
> added in
> the platform, it needs to provide a factory method to obtain the platform
> MXBeans (e.g. java.util.logging.LogManager.getLoggerMXBean()).  We 
> expect that
> NIO and the new module system will define their management interfaces in 
> JDK7.
> 
> Solution:
> 
> Add a new java.lang.management.PlatformManagedObject interface which 
> provides
> a method to return the object name. All existing MXBean interfaces will be
> modified to extend this interface.
> 
> (Note: In the interface summary, the methods count for the *MXBean 
> interfaces
>       reflects the number of methods added resulting from extending the
>       PlatformManagedObject interface)
> 
> Add new methods the in java.lang.management.ManagementFactory class:
> 1) A method to return the list of platform MXBeans that implement a given
> management interface in the running JVM
> 
> 2) A method to return the list of platform MXBean proxies that implement 
> a given
> management interface and in the given remote MBeanServerConnection
> 
> 3) A method to return all MXBean interfaces for monitoring and managing the
> Java platform
> 
> Webrev:
>  Attached webrev.zip.  This webrev also contains some code clean up 
> including to throw AssertionError instead of InternalError.
> 
> Thanks
> Mandy