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
