On 30/06/2014 12:22, Staffan Larsen wrote:
All,
The jvmstat implementation has three different protocols (file, local and rmi).
These protocol implementations are currently loaded with Class.forName() using
a specific scheme to create the class name. A more standard approach is to use
ServiceLoader for this instead. This also makes it easier to break out
different implementation classes for different packagings.
webrev: http://cr.openjdk.java.net/~sla/8048710/webrev.00/
bug: https://bugs.openjdk.java.net/browse/JDK-8048710
This looks good to me.
One comment on monitoredHostServiceLoader is that it specifies the class
loader as null and so will search the system class loader. I think this
is okay as it provides a bit of flexibility to deploy other protocols on
the class path if needed.
One other comment is that using ServiceLoader with a security manager
often requires additional permissions to be granted because it involve
scanning the class path. I don't know if we have any tests that use
jvmstat with a security manager, just something to mention in case it
comes up.
-Alan
