|
Thanks Staffan! And I really appreciate
all your reviews. Also thanks to Mikael for the DCMD test review.
Cheers,
Chris
On 2/12/15 1:19 AM, Staffan Larsen wrote:
Looks good!
Thanks for providing incremental webrevs - very
helpful!
Thanks,
/Staffan
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
Ok. I'll pull the latest hs-rt and update the test.
Thanks for pointing that out.
Chris
|
