Thanks, Alan. On 30 jun 2014, at 15:25, Alan Bateman <[email protected]> wrote:
> 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 > >
