Looks good! Thanks for providing incremental webrevs - very helpful!
Thanks, /Staffan > On 12 feb 2015, at 09:31, Chris Plummer <chris.plum...@oracle.com> wrote: > > I suppose it would be nice if I included links to the webrevs: > > http://cr.openjdk.java.net/~cjplummer/8054888/webrev.02-03/ > <http://cr.openjdk.java.net/~cjplummer/8054888/webrev.02-03/> > http://cr.openjdk.java.net/~cjplummer/8054888/webrev.03/ > <http://cr.openjdk.java.net/~cjplummer/8054888/webrev.03/> > > First one is just the latest changes described in the previous email. 2nd one > includes all changes. > > thanks, > > Chris > > On 2/12/15 12:28 AM, Chris Plummer wrote: >> Ok, hopefully the last webrev. :) >> >> -JPRT found a compiler error on Solaris. classfile/systemDictionary.hpp >> needed to be included due to the reference to >> SystemDictionary::Object_klass(). I assume this turned up on Solaris because >> it doesn't use precompiled headers. >> >> -I noticed I had inadvertently removed a ResourceMark in >> KlassInfoHisto::print_class_stats(). I think maybe I had done a cut-n-paste >> rather than copy-n-paste. This was in webrev.01 but was pretty inconspicuous >> in the webrev so went noticed. >> >> -I changed another WARNING to ERROR as Staffan requested. >> >> -I updated to the latest JDK9 sources and made the needed changes to the >> DCMD test as Mikael requested. >> >> thanks, >> >> Chris >> >> On 2/11/15 9:38 AM, Chris Plummer wrote: >>> On 2/11/15 2:12 AM, Mikael Auno wrote: >>>> On 2015-02-11 04:13, Chris Plummer wrote: >>>>>> In general I think this looks very good. Simple and well-commented >>>>>> code to follow. I am missing a test, though. Please look at the >>>>>> hotspot/test/serviceability/dcmd set of tests. >>>>> Added. >>>> Your test is based on DcmdUtil.java which was removed last week (see >>>> [0]). If you pull new changes from hs-rt/hotspot, you'll see plenty of >>>> tests using the new DCMD utility classes in testlibrary. Also, the new >>>> tests are divided in different subdirectories depending on the commands, >>>> so as your test exercises VM.class_hierarchy it should go in >>>> .../dcmd/vm/ as opposed to just .../dcmd/. >>>> >>>> Thanks, >>>> Mikael >>>> >>>> [0] >>>> http://hg.openjdk.java.net/jdk9/hs-rt/hotspot/file/tip/test/serviceability/dcmd >>>> >>>> <http://hg.openjdk.java.net/jdk9/hs-rt/hotspot/file/tip/test/serviceability/dcmd> >>>> >>> Ok. I'll pull the latest hs-rt and update the test. Thanks for pointing >>> that out. >>> >>> Chris >> >
