> On Nov 10, 2015, at 9:04 PM, Mandy Chung <mandy.ch...@oracle.com> wrote: > > Hi Shura, > > Thanks for doing it and it’s good to see the unnecessary dependency to > java.management eliminated. > > The new jdk.testlibrary.management package name is fine. It’s okay to keep > the class name InputArguments as Jaroslav suggests and it’s easier to tell > what this class is about.
The reason I have refactored this is because there is more useful methods in RuntimeMXBean than just the getInputArguments(). If we are ever in a need in another library method dealing with RuntimeMXBean, then, a class RuntimeMXBeanTool, or similar, is a natural enough place to add them. Since also there is the ThreadMXBeanTool, it looks homogenous. We can, of course, have the InputArguments class the way it is and still have other classes wrapping RuntimeMXBean calls. http://cr.openjdk.java.net/~shurailine/8139430/webrev.05/ Shura > > There is a copy of ProcessTools and InputArguments in the hotspot repository > under > hotspot/test/testlibrary/jdk/test/lib/ > > Are we planning to remove this duplicated copy? If not, same patch should be > applied to those copy. It’s fine to separate this and push the hotspot test > library change via hotspot-rt repo. Christian can probably sponsor the patch > for you. > > I’ll sponsor the jdk change and push it to jdk9/dev once you send me an > updated patch. > > Mandy > > >> On Nov 9, 2015, at 10:12 AM, Alexandre (Shura) Iline >> <alexandre.il...@oracle.com> wrote: >> >> Hi >> >> I have just realized that an NPE could also be possible in >> test/lib/testlibrary/jdk/testlibrary/Platform.java so it should be updated >> also: >> http://cr.openjdk.java.net/~shurailine/8139430/webrev.04/ >> >> Shura >> >>> On Nov 9, 2015, at 8:54 PM, Alexandre (Shura) Iline >>> <alexandre.il...@oracle.com> wrote: >>> >>> Hi, >>> >>> This is one of the ways to fix 8139430: >>> http://cr.openjdk.java.net/~shurailine/8139430/webrev.03 >>> >>> Could you please take a look on it? >>> >>> A few comments. >>> >>> The biggest dependency on java.management was in >>> jdk.testlibrary.ProcessTools.getProcessId() method. I have changed the >>> method to use the new process API, which helped to avoid updating a lot of >>> code. >>> >>> I am moving the rest library code which depended on java.management and >>> jdk.management to a new “management" subpackage of the jdk library: >>> jdk.testlibrary.management. Another possibility I have considered is “ext” >>> or “extended” subpackage, which would have all the classes which depend on >>> anything but java.base. With “ext” there would be no way to keep the >>> desired granularity with the test modules dependencies. >>> >>> The methods which worth moving fit well into two classes: >>> jdk.testlibrary.management.ThreadMXBeanTool - to include a couple of method >>> which previously belonged to the TestThread utility methods. >>> jdk.testlibrary.management.RuntimeMXBeanTool - previously named as >>> jdk.testlibrary.InputArguments >>> >>> None of moved/renamed test library methods were used anywhere in tests, so >>> no test updates needed: >>> jdk.testlibrary.InputArguments is not used, instead every test which needs >>> that functionality directly calls RuntimeMXBean.getInputArguments() >>> jdk.testlibrary.TestThread.waitUntilBlockingOnObject(Thread, Thread.State, >>> Object) and jdk.testlibrary.TestThread.waitUntilInNative(Thread) also were >>> not used as there is a similar class outside of the test library: >>> ./closed/com/oracle/jfr/common/jrockit/TestThread.java >>> >>> Please let me know what you think. >>> >>> Shura >> >
