Looks good. (Not a Reviewer). /Staffan
On 26 feb 2013, at 03:03, Krystal Mo <[email protected]> wrote: > Hi all, > > Could I have a review for this small change, please? > > Webrev: http://cr.openjdk.java.net/~kmo/8008796/webrev.00/ > CR: http://bugs.sun.com/view_bug.do?bug_id=8008796 (should be available > externally soon) > > 8008796: SA: Oop.iterateFields() should support CompressedKlassPointers again > Summary: add a missing change from JDK-7054512 so that Oop.iterateFields() > works with UseCompressedKlassPointers > Reviewed-by: ? > Contributed-by: [email protected] > > Thanks, > Kris > > On 2013/2/25 17:14, 云达(Yunda) wrote: >> Kris, >> >> Thanks a lot! >> >> Regards, >> Yunda >> >> From: [email protected] >> [mailto:[email protected]] On Behalf Of Krystal Mo >> Sent: Monday, February 25, 2013 7:19 PM >> To: [email protected] >> Subject: Re: Oop.iterateFields() should support CompressedKlassPointers again >> >> Hi Yunda, >> >> Thanks for fixing this. I actually have the exact same fix for this problem, >> along with fixes for some other problems in SA in my local workspace, but >> haven't come around to sending it out for public review yet. >> >> I think this is just a missed change in JDK-7054512. I have created a JIRA >> issue for you: JDK-8008796: SA: Oop.iterateFields() should support >> CompressedKlassPointers again. I'll prepare a webrev and send it out for >> official review later. >> >> Thanks, >> Kris >> >> >> On 2013/2/25 2:58, 云达(Yunda) wrote: >> Hi all, >> >> When I used CLHSDB just now I met this error: >> hsdb> inspect 0x00000000ee255080 >> instance of "java/io/InputStream" @ 0x00000000ee255080 @ 0x00000000ee255080 >> (size = 24) >> Exception in thread "main" java.lang.InternalError: unimplemented >> at sun.jvm.hotspot.oops.Oop.iterateFields(Oop.java:151) >> at sun.jvm.hotspot.oops.Instance.iterateFields(Instance.java:66) >> at sun.jvm.hotspot.oops.Oop.iterate(Oop.java:143) >> at >> sun.jvm.hotspot.ui.tree.OopTreeNodeAdapter.getChildCount(OopTreeNodeAdapter.java:65) >> at >> sun.jvm.hotspot.CommandProcessor$Command.printNode(CommandProcessor.java:231) >> at >> sun.jvm.hotspot.CommandProcessor$24.doit(CommandProcessor.java:1008) >> at >> sun.jvm.hotspot.CommandProcessor.executeCommand(CommandProcessor.java:1897) >> at >> sun.jvm.hotspot.CommandProcessor.executeCommand(CommandProcessor.java:1867) >> at sun.jvm.hotspot.CommandProcessor.run(CommandProcessor.java:1747) >> at sun.jvm.hotspot.CLHSDB.run(CLHSDB.java:91) >> at sun.jvm.hotspot.CLHSDB.main(CLHSDB.java:35) >> >> I found it’s caused by the code of sun.jvm.hotspot.oops.Oop.iterateFields(): >> void iterateFields(OopVisitor visitor, boolean doVMFields) { >> if (doVMFields) { >> visitor.doCInt(mark, true); >> if (VM.getVM().isCompressedKlassPointersEnabled()) { >> throw new InternalError("unimplemented"); >> } else { >> visitor.doMetadata(klass, true); >> } >> } >> } >> >> When compressed oops( which is by default) are used an InternalError of >> “unimplemented” will be throwed. But actually it can be implemented easily >> by just one line of code: >> visitor.doMetadata(compressedKlass, true); >> >> I checked the hotspot-rt repo and I found it was implemented this way before >> changeset 3601, the main implementation of NPG. Since 3601 changed a whole >> lot of stuff, the ‘compressedKlass’ field couldn’t get the right value, as >> in line 51: >> // compressedKlass = new >> CIntField(type.getCIntegerField("_metadata._compressed_klass"), 0); >> So the code in iterateFields() was changed to throwing an InternalError >> accordingly. >> >> But the ‘compressedKlass’ field can get the right value now. So I think it’s >> time to change the code back and here’s the diff against the latest >> hotspot-rt: >> diff -r 2f881161d085 agent/src/share/classes/sun/jvm/hotspot/oops/Oop.java >> --- a/agent/src/share/classes/sun/jvm/hotspot/oops/Oop.java Mon Feb 25 >> 18:25:24 2013 +0800 >> +++ b/agent/src/share/classes/sun/jvm/hotspot/oops/Oop.java Mon Feb 25 >> 18:47:08 2013 +0800 >> @@ -148,7 +148,7 @@ >> if (doVMFields) { >> visitor.doCInt(mark, true); >> if (VM.getVM().isCompressedKlassPointersEnabled()) { >> - throw new InternalError("unimplemented"); >> + visitor.doMetadata(compressedKlass, true); >> } else { >> visitor.doMetadata(klass, true); >> } >> >> >> Regards, >> Yunda >> >> >> >> This email (including any attachments) is confidential and may be legally >> privileged. If you received this email in error, please delete it >> immediately and do not copy it or use it for any purpose or disclose its >> contents to any other person. Thank you. >> >> 本电邮(包括任何 附件)可能含有 机密资料并受法律保护。如您不是正确的收件人，请您立即删除本邮件。请不要将本电邮进行复制并用作任何其 >> 他用途、或透露本邮件之内容。谢谢。 >> >> >> >> This email (including any attachments) is confidential and may be legally >> privileged. If you received this email in error, please delete it >> immediately and do not copy it or use it for any purpose or disclose its >> contents to any other person. Thank you. >> >> 本电邮(包括任何附件)可能含有机密资料并受法律保护。如您不是正确的收件人，请您立即删除本邮件。请不要将本电邮进行复制并用作任何其 >> 他用途、或透露本邮件之内容。谢谢。 >
