[prev in list] [next in list] [prev in thread] [next in thread] 

List:       openjdk-serviceability-dev
Subject:    6610094: Add generic support for platform MXBeans of any type
From:       Mandy.Chung () Sun ! COM (Mandy Chung)
Date:       2008-04-03 20:49:00
Message-ID: 47F542BC.4090508 () Sun ! com
[Download RAW message or body]

Daniel Fuchs wrote:
> 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.

This statement is a clarification in the specification for M&M support 
for the platform. For a platform management interface, only the JDK 
vendor will implement these interfaces and thus there is no 
compatibility issue.  The other alternative to add new methods for 
existing MXBeans is to add new MXBean interfaces in new JDK releases 
which is overkilled. We considered these alternatives and decided in JDK 
5 when we added the M&M API.

> 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.

The MXBean interfaces can be used locally in the application in a 
non-JMX context. Thus the ObjectName is lazily created when needed.

> 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...
> 
Eamonn also mentioned that too. Thanks.

> 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.

Same reason as above.

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

It was one class but it is splitted into 2 classes.

I like Eamonn's suggestion to rename sun.management.ManagementFactory to 
something else so that the java.lang.management implementation is much 
cleaner. However, the HotSpot VM depends on a few methods in the 
sun.management.ManagementFactory class for constructing the memory pool 
and managers and so we have to keep this interface contract.

> 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... ;-)

Sure. I'll add some comment.

> 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); \
> 

Good suggestion. I'll look into this.

Thanks
Mandy

> 
> 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
> 


[prev in list] [next in list] [prev in thread] [next in thread]