[prev in list] [next in list] [prev in thread] [next in thread]
List: openjdk-serviceability-dev
Subject: Re: Request for Review: JDK-8008796: SA: Oop.iterateFields() should support CompressedKlassPointers
From: Staffan Larsen <staffan.larsen () oracle ! com>
Date: 2013-02-26 8:16:14
Message-ID: 8F10CF15-4456-42AF-8782-A9DAB571F68B () oracle ! com
[Download RAW message or body]
Looks good. (Not a Reviewer).
/Staffan
On 26 feb 2013, at 03:03, Krystal Mo <krystal.mo@oracle.com> 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: yunda.mly@taobao.com
>
> Thanks,
> Kris
>
> On 2013/2/25 17:14, 云达(Yunda) wrote:
> > Kris,
> >
> > Thanks a lot!
> >
> > Regards,
> > Yunda
> >
> > From: serviceability-dev-bounces@openjdk.java.net \
> > [mailto:serviceability-dev-bounces@openjdk.java.net] On Behalf Of \
> > Krystal Mo
> > Sent: Monday, February 25, 2013 7:19 PM
> > To: serviceability-dev@openjdk.java.net
> > 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.
> > 本电邮(包括任何附件)可能含有机密资料并受法律保护。如您不是正确的收件人，请您立即 \
> > 除本邮件。请不要将本电邮进行复制并用作任何其 \
> > 他用途、或透露本邮件之内容。谢谢。
>
[Attachment #3 (unknown)]
<html><head><meta http-equiv="Content-Type" content="text/html \
charset=utf-8"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; \
-webkit-line-break: after-white-space; ">Looks good. (Not a \
Reviewer).<div><br></div><div>/Staffan</div><div><br><div><div>On 26 feb 2013, at \
03:03, Krystal Mo <<a \
href="mailto:krystal.mo@oracle.com">krystal.mo@oracle.com</a>> wrote:</div><br \
class="Apple-interchange-newline"><blockquote type="cite">
<meta content="text/html; charset=UTF-8" http-equiv="Content-Type">
<div text="#000000" bgcolor="#FFFFFF">
Hi all,<br>
<br>
Could I have a review for this small change, please?<br>
<br>
Webrev: <a class="moz-txt-link-freetext" \
href="http://cr.openjdk.java.net/~kmo/8008796/webrev.00/">http://cr.openjdk.java.net/~kmo/8008796/webrev.00/</a><br>
CR: <a class="moz-txt-link-freetext" \
href="http://bugs.sun.com/view_bug.do?bug_id=8008796">http://bugs.sun.com/view_bug.do?bug_id=8008796</a> \
(should be available externally soon)<br>
<br>
8008796: SA: Oop.iterateFields() should support
CompressedKlassPointers again<br>
Summary: add a missing change from JDK-7054512 so that
Oop.iterateFields() works with UseCompressedKlassPointers<br>
Reviewed-by: ?<br>
Contributed-by: <a class="moz-txt-link-abbreviated" \
href="mailto:yunda.mly@taobao.com">yunda.mly@taobao.com</a><br> <br>
Thanks,<br>
Kris<br>
<br>
<div class="moz-cite-prefix">On 2013/2/25 17:14, 云达(Yunda) wrote:<br>
</div>
<blockquote cite="mid:F6ACE14573EC5F40AEA19AEB2F62E5790168D2@CNHZ-EXMAIL-04.ali.com" \
type="cite"> <meta http-equiv="Content-Type" content="text/html; charset=UTF-8">
<style>
<!--
@font-face
{font-family:宋体}
@font-face
{font-family:宋体}
@font-face
{font-family:Calibri}
@font-face
{font-family:"\@宋体"}
@font-face
{font-family:Tahoma}
p.MsoNormal, li.MsoNormal, div.MsoNormal
{margin:0cm;
margin-bottom:.0001pt;
text-align:justify;
text-justify:inter-ideograph;
font-size:10.5pt;
font-family:"Calibri","sans-serif";
color:black}
a:link, span.MsoHyperlink
{color:blue;
text-decoration:underline}
a:visited, span.MsoHyperlinkFollowed
{color:purple;
text-decoration:underline}
p.msochpdefault, li.msochpdefault, div.msochpdefault
{margin-right:0cm;
margin-left:0cm;
font-size:12.0pt;
font-family:"Calibri","sans-serif";
color:black}
span.emailstyle17
{font-family:"Calibri","sans-serif";
color:windowtext}
span.EmailStyle19
{font-family:"Calibri","sans-serif";
color:#1F497D}
.MsoChpDefault
{font-size:10.0pt}
@page WordSection1
{margin:72.0pt 90.0pt 72.0pt 90.0pt}
div.WordSection1
{}
-->
</style>
<div class="WordSection1"><p class="MsoNormal"><span style="color:#1F497D" \
lang="EN-US">Kris,</span></p><div><span style="color:#1F497D" \
lang="EN-US"> </span><br class="webkit-block-placeholder"></div><p \
class="MsoNormal"><span style="color:#1F497D" lang="EN-US">Thanks a \
lot!</span></p><div><span style="color:#1F497D" lang="EN-US"> </span><br \
class="webkit-block-placeholder"></div><p class="MsoNormal"><span \
style="color:#1F497D" lang="EN-US">Regards,</span></p><p class="MsoNormal"><span \
style="color:#1F497D" lang="EN-US">Yunda</span></p><div><span style="color:#1F497D" \
lang="EN-US"> </span><br class="webkit-block-placeholder"></div> <div \
style="border:none; border-left:solid blue 1.5pt; padding:0cm 0cm 0cm 4.0pt">
<div>
<div style="border:none; border-top:solid #B5C4DF 1.0pt;
padding:3.0pt 0cm 0cm 0cm"><p class="MsoNormal" style="text-align:left" \
align="left"><b><span style="font-size:10.0pt; \
font-family:"Tahoma","sans-serif";
color:windowtext" lang="EN-US">From:</span></b><span \
style="font-size:10.0pt; font-family:"Tahoma","sans-serif";
color:windowtext" lang="EN-US">
<a class="moz-txt-link-abbreviated" \
href="mailto:serviceability-dev-bounces@openjdk.java.net">serviceability-dev-bounces@openjdk.java.net</a>
[<a class="moz-txt-link-freetext" \
href="mailto:serviceability-dev-bounces@openjdk.java.net">mailto:serviceability-dev-bounces@openjdk.java.net</a>]
<b>On Behalf Of </b>Krystal Mo<br>
<b>Sent:</b> Monday, February 25, 2013 7:19 PM<br>
<b>To:</b> <a class="moz-txt-link-abbreviated" \
href="mailto:serviceability-dev@openjdk.java.net">serviceability-dev@openjdk.java.net</a><br>
<b>Subject:</b> Re: Oop.iterateFields() should support
CompressedKlassPointers again</span></p>
</div>
</div><div style="text-align: left; "><span lang="EN-US"> </span><br \
class="webkit-block-placeholder"></div><p class="MsoNormal" \
style="margin-bottom:12.0pt"><span lang="EN-US">Hi Yunda,<br> <br>
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.<br>
<br>
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.<br>
<br>
Thanks,<br>
Kris<br>
<br>
</span></p>
<div><p class="MsoNormal"><span lang="EN-US">On 2013/2/25 2:58, \
</span><span style="font-family:宋体">云达</span><span lang="EN-US">(Yunda) \
wrote:</span></p> </div>
<blockquote style="margin-top:5.0pt; margin-bottom:5.0pt">
<div><p class="MsoNormal"><span lang="EN-US">Hi all,</span></p><div><span \
lang="EN-US"> </span><br class="webkit-block-placeholder"></div><p \
class="MsoNormal"><span lang="EN-US">When I used CLHSDB
just now I met this error:</span></p><p class="MsoNormal"><span \
lang="EN-US">hsdb> inspect
0x00000000ee255080</span></p><p class="MsoNormal"><span \
lang="EN-US">instance of "java/io/InputStream" @ 0x00000000ee255080 @
0x00000000ee255080 (size = 24)</span></p><p class="MsoNormal"><span \
lang="EN-US">Exception in thread "main" java.lang.InternalError: \
unimplemented</span></p><p class="MsoNormal"><span \
lang="EN-US"> at \
sun.jvm.hotspot.oops.Oop.iterateFields(Oop.java:151)</span></p><p \
class="MsoNormal"><span lang="EN-US"> \
at sun.jvm.hotspot.oops.Instance.iterateFields(Instance.java:66)</span></p><p \
class="MsoNormal"><span lang="EN-US"> \
at sun.jvm.hotspot.oops.Oop.iterate(Oop.java:143)</span></p><p \
class="MsoNormal"><span lang="EN-US"> \
at sun.jvm.hotspot.ui.tree.OopTreeNodeAdapter.getChildCount(OopTreeNodeAdapter.java:65)</span></p><p \
class="MsoNormal"><span lang="EN-US"> \
at sun.jvm.hotspot.CommandProcessor$Command.printNode(CommandProcessor.java:231)</span></p><p \
class="MsoNormal"><span lang="EN-US"> \
at sun.jvm.hotspot.CommandProcessor$24.doit(CommandProcessor.java:1008)</span></p><p \
class="MsoNormal"><span lang="EN-US"> \
at sun.jvm.hotspot.CommandProcessor.executeCommand(CommandProcessor.java:1897)</span></p><p \
class="MsoNormal"><span lang="EN-US"> \
at sun.jvm.hotspot.CommandProcessor.executeCommand(CommandProcessor.java:1867)</span></p><p \
class="MsoNormal"><span lang="EN-US"> \
at sun.jvm.hotspot.CommandProcessor.run(CommandProcessor.java:1747)</span></p><p \
class="MsoNormal"><span lang="EN-US"> \
at
sun.jvm.hotspot.CLHSDB.run(CLHSDB.java:91)</span></p><p \
class="MsoNormal"><span lang="EN-US"> \
at sun.jvm.hotspot.CLHSDB.main(CLHSDB.java:35)</span></p><div><span \
lang="EN-US"> </span><br class="webkit-block-placeholder"></div><p \
class="MsoNormal"><span lang="EN-US">I found it's caused by the code of
sun.jvm.hotspot.oops.Oop.iterateFields():</span></p><p \
class="MsoNormal"><span lang="EN-US"> void iterateFields(OopVisitor \
visitor, boolean doVMFields)
{</span></p><p class="MsoNormal"><span \
lang="EN-US"> if
(doVMFields) {</span></p><p class="MsoNormal"><span \
lang="EN-US">
visitor.doCInt(mark, true);</span></p><p class="MsoNormal"><span \
lang="EN-US"> if \
(VM.getVM().isCompressedKlassPointersEnabled()) {</span></p><p \
class="MsoNormal"><span lang="EN-US"> \
</span><b><span style="color:red" lang="EN-US">throw new \
InternalError("unimplemented");</span></b><span lang="EN-US"></span></p><p \
class="MsoNormal"><span lang="EN-US"> } else \
{</span></p><p class="MsoNormal"><span \
lang="EN-US"> \
visitor.doMetadata(klass, true);</span></p><p class="MsoNormal"><span \
lang="EN-US"> }</span></p><p \
class="MsoNormal"><span lang="EN-US"> }</span></p><p \
class="MsoNormal"><span lang="EN-US"> }</span></p><div><span \
lang="EN-US"> </span><br class="webkit-block-placeholder"></div><p \
class="MsoNormal"><span lang="EN-US">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:
</span></p><p class="MsoNormal" style="text-indent:21.0pt"><span \
lang="EN-US">visitor.doMetadata(compressedKlass, true);</span></p><div><span \
lang="EN-US"> </span><br class="webkit-block-placeholder"></div><p \
class="MsoNormal"><span lang="EN-US">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: </span></p><p class="MsoNormal" \
style="text-indent:21.0pt"><span lang="EN-US">// \
compressedKlass = new
CIntField(type.getCIntegerField("_metadata._compressed_klass"),
0);</span></p><p class="MsoNormal"><span lang="EN-US">So the code \
in iterateFields() was changed to throwing an
InternalError accordingly.</span></p><div><span \
lang="EN-US"> </span><br class="webkit-block-placeholder"></div><p \
class="MsoNormal"><span lang="EN-US">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:</span></p><p \
class="MsoNormal"><span lang="EN-US">diff -r 2f881161d085
agent/src/share/classes/sun/jvm/hotspot/oops/Oop.java</span></p><p \
class="MsoNormal"><span lang="EN-US">---
a/agent/src/share/classes/sun/jvm/hotspot/oops/Oop.java
Mon Feb 25 18:25:24 2013 +0800</span></p><p class="MsoNormal"><span \
lang="EN-US">+++ b/agent/src/share/classes/sun/jvm/hotspot/oops/Oop.java
Mon Feb 25 18:47:08 2013 +0800</span></p><p class="MsoNormal"><span \
lang="EN-US">@@ -148,7 +148,7
@@</span></p><p class="MsoNormal"><span \
lang="EN-US"> if
(doVMFields) {</span></p><p class="MsoNormal"><span \
lang="EN-US">
visitor.doCInt(mark, true);</span></p><p class="MsoNormal"><span \
lang="EN-US"> if \
(VM.getVM().isCompressedKlassPointersEnabled()) {</span></p><p \
class="MsoNormal"><span lang="EN-US">- \
throw new
InternalError("unimplemented");</span></p><p \
class="MsoNormal"><span lang="EN-US">+ \
visitor.doMetadata(compressedKlass, true);</span></p><p class="MsoNormal"><span \
lang="EN-US"> } else {</span></p><p \
class="MsoNormal"><span lang="EN-US"> \
visitor.doMetadata(klass, true);</span></p><p class="MsoNormal"><span \
lang="EN-US"> }</span></p><div style="text-align: \
right; "><span lang="EN-US"> </span><br \
class="webkit-block-placeholder"></div><div><span lang="EN-US"> </span><br \
class="webkit-block-placeholder"></div><p class="MsoNormal"><span \
lang="EN-US">Regards,</span></p><p class="MsoNormal"><span \
lang="EN-US">Yunda</span></p><div><span lang="EN-US"> </span><br \
class="webkit-block-placeholder"></div> </div><div style="text-align: left; "><span \
style="font-size:12.0pt; font-family:宋体" lang="EN-US"> </span><br \
class="webkit-block-placeholder"></div>
<div class="MsoNormal" style="text-align:center" align="center"><span \
style="font-size:12.0pt; font-family:宋体" lang="EN-US">
<hr size="2" width="100%" align="center">
</span></div><p class="MsoNormal" style="text-align:left" \
align="left"><span style="font-size:7.5pt; \
font-family:"Arial","sans-serif"; color:gray" lang="EN-US"><br>
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.<br>
<br>
</span><span style="font-size:7.5pt; font-family:宋体;
color:gray">本电邮</span><span style="font-size:7.5pt;
font-family:"Arial","sans-serif";
color:gray" lang="EN-US">(</span><span style="font-size:7.5pt; \
font-family:宋体; color:gray">包括任何 附件</span><span \
style="font-size:7.5pt; font-family:"Arial","sans-serif";
color:gray" lang="EN-US">)</span><span style="font-size:7.5pt; \
font-family:宋体; color:gray">可能含有 \
机密资料并受法律保护。如您不是正确的收件人，请您立即 \
除本邮件。请不要将本电邮进行复制并用作任何其</span><span \
style="font-size:7.5pt; font-family:"Arial","sans-serif";
color:gray">
</span><span style="font-size:7.5pt; font-family:宋体;
color:gray">他用途、或透露本邮件之内容。谢谢。</span><span \
style="font-size:12.0pt; font-family:宋体" lang="EN-US"></span></p> \
</blockquote><div style="text-align: left; "><span style="font-size:12.0pt; \
font-family:宋体" lang="EN-US"> </span><br \
class="webkit-block-placeholder"></div> </div>
</div>
<br>
<hr>
<font size="1" color="Gray" face="Arial"><br>
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.<br>
<br>
本电邮(包括任何附件)可能含有机密资料并受法律保护。如您不是正确的收件人，请您立即 \
除本邮件。请不要将本电邮进行复制并用作任何其 \
他用途、或透露本邮件之内容。谢谢。<br> </font>
</blockquote>
<br>
</div>
</blockquote></div><br></div></body></html>
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic