[prev in list] [next in list] [prev in thread] [next in thread]
List: openjdk-serviceability-dev
Subject: =?gb2312?B?tPC4tDogW1JGUl04MjE1NjIyOiBBZGQgZHVtcCB0byBmaWxlIHN1cHBvcnQg?= =?gb2312?Q?for_jmap_histo?
From: <zanglin5 () jd ! com>
Date: 2019-01-11 2:25:27
Message-ID: 70826c967ec543df9febd907812d4361 () jd ! com
[Download RAW message or body]
[Attachment #2 (text/plain)]
Hi Jc,
Thanks a lot. it is not a necessary change, I forget to omit it:)
BRs,
Lin
________________________________
·¢¼þÈË: JC Beyler <jcbeyler@google.com>
·¢ËÍʱ¼ä: 2019Äê1Ô 11ÈÕ 0:58:22
ÊÕ¼þÈË: ê°ÁÕ
³ËÍ: Hohensee, Paul; serviceability-dev@openjdk.java.net
Ö÷Ìâ: Re: [RFR]8215622: Add dump to file support for jmap histo
Hi Lin,
Small nit:
http://cr.openjdk.java.net/~xiaofeya/8215622/webrev.02/src/java.base/share/native/libjli/java.c.udiff.html
needs a copyright update,
Jc
On Wed, Jan 9, 2019 at 7:48 PM ê°ÁÕ <zanglin5@jd.com<mailto:zanglin5@jd.com>> wrote:
Dear All,
I have updated the refined webrev at \
http://cr.openjdk.java.net/~xiaofeya/8215622/webrev.02/ Would you like to help \
review? Thanks!
BRs,
Lin
From: ê°ÁÕ
Sent: Wednesday, January 9, 2019 11:00 AM
To: 'JC Beyler' <jcbeyler@google.com<mailto:jcbeyler@google.com>>
Cc: Hohensee, Paul <hohensee@amazon.com<mailto:hohensee@amazon.com>>; \
serviceability-dev@openjdk.java.net<mailto:serviceability-dev@openjdk.java.net>
Subject: RE: [RFR]8215622: Add dump to file support for jmap histo
Dear JC,
Thanks to point it out, I processed the ¡°-file=¡± case in JMap.java but \
forgot to do it in attachListener.cpp. I will do it in next webrev.
Cheers,
Lin
From: JC Beyler <jcbeyler@google.com<mailto:jcbeyler@google.com>>
Sent: Wednesday, January 9, 2019 10:51 AM
To: ê°ÁÕ <zanglin5@jd.com<mailto:zanglin5@jd.com>>
Cc: Hohensee, Paul <hohensee@amazon.com<mailto:hohensee@amazon.com>>; \
serviceability-dev@openjdk.java.net<mailto:serviceability-dev@openjdk.java.net>
Subject: Re: [RFR]8215622: Add dump to file support for jmap histo
Hi Lin,
Inlined as well :-)
On Tue, Jan 8, 2019 at 6:23 PM ê°ÁÕ <zanglin5@jd.com<mailto:zanglin5@jd.com>> wrote:
Dear JC,
Thanks for your comments, I inlined my comments here:
http://cr.openjdk.java.net/~xiaofeya/8215622/webrev.01/src/hotspot/share/services/attachListener.cpp.udiff.html
- Should we do like the rest of the file and declare variables when needed instead \
of doing them all at the start?
--- (Lin) I will do that in next webrev.
- Should the method return JNI_ERR if the file cannot be created (because if not, \
then why fail if no file is passed at all?)
--- (Lin) The logic is that when user use ¡°-file=<filename>¡±, the file must be \
created successful, otherwise the ¡°-file=¡± not work, which break user¡¯s expection, \
so it fail here. If user not specify ¡°-file=¡±, it indicate that user not expect to \
dump to file, so the outputStream will be used. Do you think it is reasonable?
No that is reasonable BUT your code currently allows the user to do "--file="; in \
this absurd case, your code prints out "No dump file specified" and just continues. \
Why not make that fail as well?
The bigger issue I see is the passing of NULL for a filename, why do we not do things \
where you just really pass "-file=<file>" to the attachListener.cpp and handle the \
parsing there?; it would then make more sense to me: we either pass "-file=<file>" or \
not but we no longer have a "maybe there is or not a file, so maybe there is a NULL \
there".
---(Lin) This is similar with what I have done in webrev00, but I think maybe \
processing arguments in JMap.java is more reasonable, I think logically, it is \
JMap¡¯s responsibility to parsed it¡¯s command line arguments, and then pass it to \
attachListener. The attachListener just hearing from the socket and get command and \
parsed arguments. And one more reason maybe that in java it is easy to get the \
canonical path from the API getCanonicalPath(), which I guess maybe a little \
complicate to do it cross platform in attachListener.cpp.
I think it's a style choice perhaps? I'd rather have the code look at the arguments \
and see if it is --file or if it is --live or --all and then figure out what to do \
instead of having now "null or a file" for arg0. But I can see the conversation go \
both ways in this case.
Thanks!
Jc
All other comments will be handled in the next webrev. Thanks a lot for your review \
and suggestions.
Cheers,
Lin
From: JC Beyler <jcbeyler@google.com<mailto:jcbeyler@google.com>>
Sent: Wednesday, January 9, 2019 1:42 AM
To: ê°ÁÕ <zanglin5@jd.com<mailto:zanglin5@jd.com>>
Cc: Hohensee, Paul <hohensee@amazon.com<mailto:hohensee@amazon.com>>; \
serviceability-dev@openjdk.java.net<mailto:serviceability-dev@openjdk.java.net>
Subject: Re: [RFR]8215622: Add dump to file support for jmap histo
Hi Lin,
I have a few nits:
http://cr.openjdk.java.net/~xiaofeya/8215622/webrev.01/src/jdk.attach/aix/classes/sun/tools/attach/VirtualMachineImpl.java.udiff.html
http://cr.openjdk.java.net/~xiaofeya/8215622/webrev.01/src/jdk.attach/linux/classes/sun/tools/attach/VirtualMachineImpl.java.udiff.html
http://cr.openjdk.java.net/~xiaofeya/8215622/webrev.01/src/jdk.attach/macosx/classes/sun/tools/attach/VirtualMachineImpl.java.udiff.html
You could fix the spaces arount the for loop you changed:
+ for (int i=0; i<4; i++) {
to
+ for (int i = 0; i < 4; i++) {
http://cr.openjdk.java.net/~xiaofeya/8215622/webrev.01/src/hotspot/share/services/attachListener.cpp.udiff.html
- Should we do like the rest of the file and declare variables when needed instead \
of doing them all at the start?
- Should the method return JNI_ERR if the file cannot be created (because if not, \
then why fail if no file is passed at all?)
http://cr.openjdk.java.net/~xiaofeya/8215622/webrev.01/src/jdk.jcmd/share/classes/sun/tools/jmap/JMap.java.udiff.html
+ filename = opt.substring(5);
+ if (filename != null) {
-> I don't see how filename can be null here
-> same filename could just be declared in the if
+ } else if (subopt.startsWith("file=")) {
+ filename = parseFileName(subopt);
-> So actually you are testing twice if the string starts with "file=", maybe remove \
the one in the method?
-> in the option string printouts, you don't specify that all is an option as well, \
is that normal?
The bigger issue I see is the passing of NULL for a filename, why do we not do things \
where you just really pass "-file=<file>" to the attachListener.cpp and handle the \
parsing there?; it would then make more sense to me: we either pass "-file=<file>" or \
not but we no longer have a "maybe there is or not a file, so maybe there is a NULL \
there".
Thanks,
Jc
On Mon, Jan 7, 2019 at 2:11 AM ê°ÁÕ <zanglin5@jd.com<mailto:zanglin5@jd.com>> wrote:
Hi Paul,
I think it is not necessary to have a loop for argument processing, since there \
is not so much arguments to handle. And I made a new webrev \
http://cr.openjdk.java.net/~xiaofeya/8215622/webrev.01/
Hi All,
May I also ask your help for reviewing this webrev?
webrev http://cr.openjdk.java.net/~xiaofeya/8215622/webrev.01/
Bug: https://bugs.openjdk.java.net/browse/JDK-8215622
Thanks!
Lin
________________________________
·¢¼þÈË: serviceability-dev \
<serviceability-dev-bounces@openjdk.java.net<mailto:serviceability-dev-bounces@openjdk.java.net>> \
´ú±í ê°ÁÕ <zanglin5@jd.com<mailto:zanglin5@jd.com>> ·¢ËÍʱ¼ä: 2019Äê1Ô 3ÈÕ 10:21
ÊÕ¼þÈË: Hohensee, Paul; \
serviceability-dev@openjdk.java.net<mailto:serviceability-dev@openjdk.java.net> Ö÷Ìâ: \
[·¢¼þµØַαÔì,¶ñÒâÓʼþ,Îðµã] RE: [RFR]8215622: Add dump to file support for jmap \
histo
Dear Paul,
Thanks a lot for your review! I am trying to remend the patch
One problem is that in future, I will add more options for histo, such as \
¡°parallel¡±, ¡°incremental¡±.
so do you thinking option processing with loop in heap_inspection() in \
attachListener.cpp would
be more reasonable than the ¡°if else¡± ?
Thanks
BRs,
Lin
From: Hohensee, Paul <hohensee@amazon.com<mailto:hohensee@amazon.com>>
Sent: Saturday, December 29, 2018 3:54 AM
To: ê°ÁÕ <zanglin5@jd.com<mailto:zanglin5@jd.com>>; \
serviceability-dev@openjdk.java.net<mailto:serviceability-dev@openjdk.java.net>
Subject: Re: [RFR]8215622: Add dump to file support for jmap histo
Update the copyright dates to 2019 please, since this won¡¯t get pushed until then.
In JMap.java, I¡¯d emulate dump() and use
String filename = null;
String subopts[] = options.split(",");
for (String subopt : subopts){
if (subopt.isEmpty() || subopt.equals("all")) {
// pass
} else if (subopt.equals("live")) {
liveopt = "-live";
} else if (subopt.startsWith("file=")) {
// file=<file> - check that <file> is specified
if (subopt.length() > 5) {
filename = subopt.substring(5);
}
} else {
usage(1);
}
}
// get the canonical path - important to avoid just passing
// a "heap.bin" and having the dump created in the target VM
// working directory rather than the directory where jmap
// is executed.
filename = new File(filename).getCanonicalPath();
// inspectHeap is not the same as jcmd GC.class_histogram
executeCommandForPid(pid, "inspectheap", filename, liveopt);
I.e., use an enhanced for loop to scan the array, and duplicate dump()¡¯s \
executeCommandForPid() argument order, as well as dump()¡¯s ¡°file=<>¡± check (the \
code that starts with ¡°if (subopt.startsWith¡±) and canonicalization. Actually, \
better to factor the latter out into its own method and use it from both histo() and \
dump().
The argument checking code in heap_inspection() in attachListener.cpp can be \
simplified along the lines of dump_heap(). I.e., you don¡¯t need to loop over the \
argument list. To match up with dump_heap()¡¯s info messages, the info message string \
at the end should be ¡°Heap inspection file created: %s¡±.
Thanks,
Paul
From: serviceability-dev \
<serviceability-dev-bounces@openjdk.java.net<mailto:serviceability-dev-bounces@openjdk.java.net>> \
on behalf of ê°ÁÕ <zanglin5@jd.com<mailto:zanglin5@jd.com>>
Date: Thursday, December 20, 2018 at 11:03 PM
To: "serviceability-dev@openjdk.java.net<mailto:serviceability-dev@openjdk.java.net>" \
<serviceability-dev@openjdk.java.net<mailto:serviceability-dev@openjdk.java.net>>
Subject: [RFR]8215622: Add dump to file support for jmap histo
Hi All,
May I ask your help to review this patch for enhance jmap ¨Chisto.
It add the ¡°file=<path>¡± arguments that allow jmap ¨Chisto outputs data to file \
directly.
This patch is also part of the enhancement described in \
https://bugs.openjdk.java.net/browse/JDK-8214535.
Webrev: http://cr.openjdk.java.net/~xiaofeya/8215622/webrev.00/
Bug: https://bugs.openjdk.java.net/browse/JDK-8215622
Thanks.
BRs,
Lin
--
Thanks,
Jc
--
Thanks,
Jc
--
Thanks,
Jc
[Attachment #3 (text/html)]
<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=gb2312">
</head>
<body>
<style type="text/css" style="display:none;"><!-- P {margin-top:0;margin-bottom:0;} \
--></style> <div id="divtagdefaultwrapper" \
style="font-size:12pt;color:#000000;font-family:Calibri,Helvetica,sans-serif;" \
dir="ltr"> Hi Jc,
<div> Thanks a lot. it is not a necessary change, I forget to omit \
it:) </div> <div><br>
</div>
<div>BRs,</div>
<div>Lin</div>
</div>
<hr style="display:inline-block;width:98%" tabindex="-1">
<div id="divRplyFwdMsg" dir="ltr"><font face="Calibri, sans-serif" \
style="font-size:11pt" color="#000000"><b>·¢¼þÈË:</b> JC Beyler \
<jcbeyler@google.com><br> <b>·¢ËÍʱ¼ä:</b> 2019Äê1Ô 11ÈÕ 0:58:22<br>
<b>ÊÕ¼þÈË:</b> ê°ÁÕ<br>
<b>³ËÍ:</b> Hohensee, Paul; serviceability-dev@openjdk.java.net<br>
<b>Ö÷Ìâ:</b> Re: [RFR]8215622: Add dump to file support for jmap histo</font>
<div> </div>
</div>
<div>
<div dir="ltr">
<div dir="ltr">Hi Lin,
<div><br>
</div>
<div>Small nit:</div>
<div><a href="http://cr.openjdk.java.net/~xiaofeya/8215622/webrev.02/src/java.base/sha \
re/native/libjli/java.c.udiff.html">http://cr.openjdk.java.net/~xiaofeya/8215622/webrev.02/src/java.base/share/native/libjli/java.c.udiff.html</a><br>
</div>
<div><br>
</div>
<div>needs a copyright update,</div>
<div>Jc</div>
<div><br>
</div>
</div>
</div>
<br>
<div class="gmail_quote">
<div dir="ltr">On Wed, Jan 9, 2019 at 7:48 PM ê°ÁÕ <<a \
href="mailto:zanglin5@jd.com">zanglin5@jd.com</a>> wrote:<br> </div>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid \
rgb(204,204,204);padding-left:1ex"> <div lang="ZH-CN">
<div class="gmail-m_6631777943017640093WordSection1">
<p class="MsoNormal"><span lang="EN-US" style="font-size:10.5pt;font-family:"MS \
PGothic",sans-serif;color:rgb(31,73,125)">Dear All, <u></u><u></u></span></p>
<p class="MsoNormal"><span lang="EN-US" style="font-size:10.5pt;font-family:"MS \
PGothic",sans-serif;color:rgb(31,73,125)"> I \
have updated the refined webrev at <a \
href="http://cr.openjdk.java.net/~xiaofeya/8215622/webrev.02/" target="_blank"> \
http://cr.openjdk.java.net/~xiaofeya/8215622/webrev.02/</a><u></u><u></u></span></p> \
<p class="MsoNormal"><span lang="EN-US" style="font-size:10.5pt;font-family:"MS \
PGothic",sans-serif;color:rgb(31,73,125)"> \
Would you like to help review? Thanks!<u></u><u></u></span></p> <p \
class="MsoNormal"><span lang="EN-US" style="font-size:10.5pt;font-family:"MS \
PGothic",sans-serif;color:rgb(31,73,125)"><u></u> <u></u></span></p> <p \
class="MsoNormal"><span lang="EN-US" style="font-size:10.5pt;font-family:"MS \
PGothic",sans-serif;color:rgb(31,73,125)">BRs,<u></u><u></u></span></p> <p \
class="MsoNormal"><span lang="EN-US" style="font-size:10.5pt;font-family:"MS \
PGothic",sans-serif;color:rgb(31,73,125)">Lin<u></u><u></u></span></p> <div>
<div style="border-right:none;border-bottom:none;border-left:none;border-top:1pt \
solid rgb(225,225,225);padding:3pt 0cm 0cm"> <p class="MsoNormal"><b><span \
lang="EN-US" style="font-size:11pt;font-family:Calibri,sans-serif">From:</span></b><span \
lang="EN-US" style="font-size:11pt;font-family:Calibri,sans-serif"> </span><span \
style="font-size:11pt">ê°ÁÕ</span><span lang="EN-US" \
style="font-size:11pt;font-family:Calibri,sans-serif"> <br>
<b>Sent:</b> Wednesday, January 9, 2019 11:00 AM<br>
<b>To:</b> 'JC Beyler' <<a href="mailto:jcbeyler@google.com" \
target="_blank">jcbeyler@google.com</a>><br> <b>Cc:</b> Hohensee, Paul <<a \
href="mailto:hohensee@amazon.com" target="_blank">hohensee@amazon.com</a>>; <a \
href="mailto:serviceability-dev@openjdk.java.net" \
target="_blank">serviceability-dev@openjdk.java.net</a><br> <b>Subject:</b> RE: \
[RFR]8215622: Add dump to file support for jmap histo<u></u><u></u></span></p> </div>
</div>
<p class="MsoNormal"><span lang="EN-US"><u></u> <u></u></span></p>
<p class="MsoNormal"><span lang="EN-US" style="font-size:10.5pt;font-family:"MS \
PGothic",sans-serif;color:rgb(31,73,125)">Dear JC, <u></u><u></u></span></p>
<p class="MsoNormal"><span lang="EN-US" style="font-size:10.5pt;font-family:"MS \
PGothic",sans-serif;color:rgb(31,73,125)"> \
Thanks to point it out, I processed the </span><span \
style="font-size:10.5pt;font-family:"MS \
PGothic",sans-serif;color:rgb(31,73,125)">¡°<span \
lang="EN-US">-file=</span>¡±<span lang="EN-US"> case in JMap.java but forgot to do it \
in attachListener.cpp. I will do it in next webrev.<u></u><u></u></span></span></p> \
<p class="MsoNormal"><span lang="EN-US" style="font-size:10.5pt;font-family:"MS \
PGothic",sans-serif;color:rgb(31,73,125)"><u></u> <u></u></span></p> <p \
class="MsoNormal"><span lang="EN-US" style="font-size:10.5pt;font-family:"MS \
PGothic",sans-serif;color:rgb(31,73,125)">Cheers,<u></u><u></u></span></p> <p \
class="MsoNormal"><span lang="EN-US" style="font-size:10.5pt;font-family:"MS \
PGothic",sans-serif;color:rgb(31,73,125)">Lin<u></u><u></u></span></p> <p \
class="MsoNormal"><span lang="EN-US" \
style="font-size:10.5pt;font-family:µÈÏß;color:rgb(31,73,125)">
<u></u><u></u></span></p>
<p class="MsoNormal"><b><span lang="EN-US" \
style="font-size:11pt;font-family:Calibri,sans-serif">From:</span></b><span \
lang="EN-US" style="font-size:11pt;font-family:Calibri,sans-serif"> JC Beyler <<a \
href="mailto:jcbeyler@google.com" target="_blank">jcbeyler@google.com</a>> <br>
<b>Sent:</b> Wednesday, January 9, 2019 10:51 AM<br>
<b>To:</b> </span><span style="font-size:11pt">ê°ÁÕ</span><span lang="EN-US" \
style="font-size:11pt;font-family:Calibri,sans-serif"> <<a \
href="mailto:zanglin5@jd.com" target="_blank">zanglin5@jd.com</a>><br> <b>Cc:</b> \
Hohensee, Paul <<a href="mailto:hohensee@amazon.com" \
target="_blank">hohensee@amazon.com</a>>; <a \
href="mailto:serviceability-dev@openjdk.java.net" \
target="_blank">serviceability-dev@openjdk.java.net</a><br> <b>Subject:</b> Re: \
[RFR]8215622: Add dump to file support for jmap histo<u></u><u></u></span></p> <p \
class="MsoNormal"><span lang="EN-US"><u></u> <u></u></span></p> <div>
<div>
<p class="MsoNormal"><span lang="EN-US">Hi Lin,<u></u><u></u></span></p>
<div>
<p class="MsoNormal"><span lang="EN-US"><u></u> <u></u></span></p>
</div>
<div>
<p class="MsoNormal"><span lang="EN-US">Inlined as well :-)<u></u><u></u></span></p>
</div>
</div>
<p class="MsoNormal"><span lang="EN-US"><u></u> <u></u></span></p>
<div>
<div>
<p class="MsoNormal"><span lang="EN-US">On Tue, Jan 8, 2019 at 6:23 PM \
</span>ê°ÁÕ<span lang="EN-US"> <<a href="mailto:zanglin5@jd.com" \
target="_blank">zanglin5@jd.com</a>> wrote:<u></u><u></u></span></p> </div>
<blockquote style="border-top:none;border-right:none;border-bottom:none;border-left:1pt \
solid rgb(204,204,204);padding:0cm 0cm 0cm 6pt;margin:5pt 0cm 5pt 4.8pt"> <div>
<div>
<p class="MsoNormal"><span lang="EN-US" \
style="font-size:10.5pt;font-family:µÈÏß;color:rgb(31,73,125)">Dear JC, </span><span \
lang="EN-US"><u></u><u></u></span></p> <p class="MsoNormal"><span lang="EN-US" \
style="font-size:10.5pt;font-family:µÈÏß;color:rgb(31,73,125)"> Thanks \
for your comments, I inlined my comments here:</span><span \
lang="EN-US"><u></u><u></u></span></p> <p class="MsoNormal"><span lang="EN-US"><a \
href="http://cr.openjdk.java.net/~xiaofeya/8215622/webrev.01/src/hotspot/share/services/attachListener.cpp.udiff.html" \
target="_blank">http://cr.openjdk.java.net/~xiaofeya/8215622/webrev.01/src/hotspot/share/services/attachListener.cpp.udiff.html</a><u></u><u></u></span></p>
<p class="MsoNormal"><span lang="EN-US"> - Should we do like the rest of the \
file and declare variables when needed instead of doing them all at the \
start?<u></u><u></u></span></p> <p class="MsoNormal"><span \
lang="EN-US"> </span><span lang="EN-US" style="font-family:"MS \
PGothic",sans-serif">--- (Lin) I will do that in next webrev.</span><span \
lang="EN-US"><u></u><u></u></span></p> <p class="MsoNormal"><span lang="EN-US"> \
- Should the method return JNI_ERR if the file cannot be created (because if not, \
then why fail if no file is passed at all?)<u></u><u></u></span></p> <p \
class="MsoNormal"><span lang="EN-US"> </span><span lang="EN-US" \
style="font-family:"MS PGothic",sans-serif"> --- (Lin) The logic is \
that when user use </span><span style="font-family:"MS \
PGothic",sans-serif">¡°<span lang="EN-US">-file=<filename></span>¡±<span \
lang="EN-US">, the file must be created successful, otherwise the </span>¡°<span \
lang="EN-US">-file=</span>¡±<span lang="EN-US"> not work, which break \
user</span>¡¯<span lang="EN-US">s expection, so it fail here. If user not specify \
</span>¡°<span lang="EN-US">-file=</span>¡±<span lang="EN-US">, it indicate that user \
not expect to dump to file, so the outputStream will be used. Do you think it is \
reasonable?</span></span><span lang="EN-US"><u></u><u></u></span></p> <p \
class="MsoNormal"><span lang="EN-US" style="font-family:"MS \
PGothic",sans-serif"> </span><span lang="EN-US"><u></u><u></u></span></p> \
</div> </div>
</blockquote>
<div>
<p class="MsoNormal"><span lang="EN-US"><u></u> <u></u></span></p>
</div>
<div>
<p class="MsoNormal"><span lang="EN-US">No that is reasonable BUT your code currently \
allows the user to do "--file="; in this absurd case, your code prints out \
"No dump file specified" and just continues. Why not make that fail as \
well?<u></u><u></u></span></p> </div>
<div>
<p class="MsoNormal"><span lang="EN-US"> <u></u><u></u></span></p>
</div>
<blockquote style="border-top:none;border-right:none;border-bottom:none;border-left:1pt \
solid rgb(204,204,204);padding:0cm 0cm 0cm 6pt;margin:5pt 0cm 5pt 4.8pt"> <div>
<div>
<p class="MsoNormal"><span lang="EN-US">The bigger issue I see is the passing of NULL \
for a filename, why do we not do things where you just really pass \
"-file=<file>" to the attachListener.cpp and handle the parsing \
there?; it would then make more sense to me: we either pass \
"-file=<file>" or not but we no longer have a "maybe there is or \
not a file, so maybe there is a NULL there".<u></u><u></u></span></p> <p \
class="MsoNormal" style="text-indent:21pt"><span lang="EN-US" \
style="font-family:"MS PGothic",sans-serif">---(Lin) This is similar with \
what I have done in webrev00, but I think maybe processing arguments in JMap.java is \
more reasonable, I think logically, it is JMap</span><span \
style="font-family:"MS PGothic",sans-serif">¡¯<span lang="EN-US">s \
responsibility to parsed it</span>¡¯<span lang="EN-US">s command line arguments, and \
then pass it to attachListener. The attachListener just hearing from the socket and \
get command and parsed arguments. And one more reason maybe that in java it is easy \
to get the canonical path from the API getCanonicalPath(), which I guess maybe a \
little complicate to do it cross platform in attachListener.cpp.</span></span><span \
lang="EN-US"><u></u><u></u></span></p> </div>
</div>
</blockquote>
<div>
<p class="MsoNormal"><span lang="EN-US"><u></u> <u></u></span></p>
</div>
<div>
<p class="MsoNormal"><span lang="EN-US">I think it's a style choice perhaps? I'd \
rather have the code look at the arguments and see if it is --file or if it is --live \
or --all and then figure out what to do instead of having now "null or a \
file" for arg0. But I can see the conversation go both ways in this \
case.<u></u><u></u></span></p> </div>
<div>
<p class="MsoNormal"><span lang="EN-US"><u></u> <u></u></span></p>
</div>
<div>
<p class="MsoNormal"><span lang="EN-US">Thanks!<u></u><u></u></span></p>
</div>
<div>
<p class="MsoNormal"><span lang="EN-US">Jc<u></u><u></u></span></p>
</div>
<div>
<p class="MsoNormal"><span lang="EN-US"> <u></u><u></u></span></p>
</div>
<blockquote style="border-top:none;border-right:none;border-bottom:none;border-left:1pt \
solid rgb(204,204,204);padding:0cm 0cm 0cm 6pt;margin:5pt 0cm 5pt 4.8pt"> <div>
<div>
<p class="MsoNormal" style="text-indent:21pt"><span lang="EN-US" \
style="font-family:"MS PGothic",sans-serif"> </span><span \
lang="EN-US"><u></u><u></u></span></p> <p class="MsoNormal" \
style="text-indent:21pt"><span lang="EN-US" style="font-family:"MS \
PGothic",sans-serif">All other comments will be handled in the next webrev. \
Thanks a lot for your review and suggestions.</span><span \
lang="EN-US"><u></u><u></u></span></p> <p class="MsoNormal" \
style="text-indent:21pt"><span lang="EN-US" style="font-family:"MS \
PGothic",sans-serif"> </span><span lang="EN-US"><u></u><u></u></span></p> \
<p class="MsoNormal" style="text-indent:21pt"><span lang="EN-US" \
style="font-family:"MS PGothic",sans-serif">Cheers, </span><span \
lang="EN-US"><u></u><u></u></span></p> <p class="MsoNormal" \
style="text-indent:21pt"><span lang="EN-US" style="font-family:"MS \
PGothic",sans-serif">Lin</span><span lang="EN-US"><u></u><u></u></span></p> <p \
class="MsoNormal"><span lang="EN-US" \
style="font-size:10.5pt;font-family:µÈÏß;color:rgb(31,73,125)"> </span><span \
lang="EN-US"><u></u><u></u></span></p> <p class="MsoNormal"><span lang="EN-US" \
style="font-size:10.5pt;font-family:µÈÏß;color:rgb(31,73,125)"> </span><span \
lang="EN-US"><u></u><u></u></span></p> <p class="MsoNormal"><b><span lang="EN-US" \
style="font-size:11pt;font-family:Calibri,sans-serif">From:</span></b><span \
lang="EN-US" style="font-size:11pt;font-family:Calibri,sans-serif"> JC Beyler <<a \
href="mailto:jcbeyler@google.com" target="_blank">jcbeyler@google.com</a>> <br>
<b>Sent:</b> Wednesday, January 9, 2019 1:42 AM<br>
<b>To:</b> </span><span style="font-size:11pt">ê°ÁÕ</span><span lang="EN-US" \
style="font-size:11pt;font-family:Calibri,sans-serif"> <<a \
href="mailto:zanglin5@jd.com" target="_blank">zanglin5@jd.com</a>><br> <b>Cc:</b> \
Hohensee, Paul <<a href="mailto:hohensee@amazon.com" \
target="_blank">hohensee@amazon.com</a>>; <a \
href="mailto:serviceability-dev@openjdk.java.net" \
target="_blank">serviceability-dev@openjdk.java.net</a><br> <b>Subject:</b> Re: \
[RFR]8215622: Add dump to file support for jmap histo</span><span \
lang="EN-US"><u></u><u></u></span></p> <p class="MsoNormal"><span \
lang="EN-US"> <u></u><u></u></span></p> <div>
<div>
<div>
<div>
<div>
<div>
<div>
<div>
<div>
<div>
<p class="MsoNormal"><span lang="EN-US">Hi Lin,<u></u><u></u></span></p>
<div>
<p class="MsoNormal"><span lang="EN-US"> <u></u><u></u></span></p>
</div>
<div>
<p class="MsoNormal"><span lang="EN-US">I have a few nits:<u></u><u></u></span></p>
</div>
<div>
<p class="MsoNormal"><span lang="EN-US"> <a \
href="http://cr.openjdk.java.net/~xiaofeya/8215622/webrev.01/src/jdk.attach/aix/classes/sun/tools/attach/VirtualMachineImpl.java.udiff.html" \
target="_blank">http://cr.openjdk.java.net/~xiaofeya/8215622/webrev.01/src/jdk.attach/ \
aix/classes/sun/tools/attach/VirtualMachineImpl.java.udiff.html</a><u></u><u></u></span></p>
</div>
<div>
<p class="MsoNormal"><span lang="EN-US"> <a \
href="http://cr.openjdk.java.net/~xiaofeya/8215622/webrev.01/src/jdk.attach/linux/classes/sun/tools/attach/VirtualMachineImpl.java.udiff.html" \
target="_blank">http://cr.openjdk.java.net/~xiaofeya/8215622/webrev.01/src/jdk.attach/ \
linux/classes/sun/tools/attach/VirtualMachineImpl.java.udiff.html</a><u></u><u></u></span></p>
</div>
<div>
<p class="MsoNormal"><span lang="EN-US"> <a \
href="http://cr.openjdk.java.net/~xiaofeya/8215622/webrev.01/src/jdk.attach/macosx/classes/sun/tools/attach/VirtualMachineImpl.java.udiff.html" \
target="_blank">http://cr.openjdk.java.net/~xiaofeya/8215622/webrev.01/src/jdk.attach/ \
macosx/classes/sun/tools/attach/VirtualMachineImpl.java.udiff.html</a><u></u><u></u></span></p>
</div>
<div>
<p class="MsoNormal"><span lang="EN-US"> <u></u><u></u></span></p>
</div>
<div>
<p class="MsoNormal"><span lang="EN-US">You could fix the spaces arount the for loop \
you changed:<u></u><u></u></span></p> </div>
<div>
<div>
<p class="MsoNormal"><span lang="EN-US"> <u></u><u></u></span></p>
</div>
<div>
<p class="MsoNormal"><span lang="EN-US">+ \
for (int i=0; i<4; i++) {<u></u><u></u></span></p> </div>
</div>
<div>
<p class="MsoNormal"><span lang="EN-US">to<u></u><u></u></span></p>
</div>
<div>
<div>
<p class="MsoNormal"><span lang="EN-US"> <u></u><u></u></span></p>
</div>
<div>
<p class="MsoNormal"><span lang="EN-US">+ \
for (int i = 0; i < 4; i++) {<u></u><u></u></span></p> </div>
</div>
<div>
<p class="MsoNormal"><span lang="EN-US"> <u></u><u></u></span></p>
</div>
<div>
<p class="MsoNormal"><span lang="EN-US"> <u></u><u></u></span></p>
</div>
<div>
<p class="MsoNormal"><span lang="EN-US"><a \
href="http://cr.openjdk.java.net/~xiaofeya/8215622/webrev.01/src/hotspot/share/services/attachListener.cpp.udiff.html" \
target="_blank">http://cr.openjdk.java.net/~xiaofeya/8215622/webrev.01/src/hotspot/share/services/attachListener.cpp.udiff.html</a><u></u><u></u></span></p>
</div>
<div>
<p class="MsoNormal"><span lang="EN-US"> - Should we do like the rest of the \
file and declare variables when needed instead of doing them all at the \
start?<u></u><u></u></span></p> </div>
<div>
<p class="MsoNormal"><span lang="EN-US"> - Should the method return JNI_ERR if \
the file cannot be created (because if not, then why fail if no file is passed at \
all?)<u></u><u></u></span></p> </div>
<div>
<p class="MsoNormal"><span lang="EN-US"> <u></u><u></u></span></p>
</div>
<div>
<p class="MsoNormal"><span lang="EN-US"><a \
href="http://cr.openjdk.java.net/~xiaofeya/8215622/webrev.01/src/jdk.jcmd/share/classes/sun/tools/jmap/JMap.java.udiff.html" \
target="_blank">http://cr.openjdk.java.net/~xiaofeya/8215622/webrev.01/src/jdk.jcmd/share/classes/sun/tools/jmap/JMap.java.udiff.html</a><u></u><u></u></span></p>
</div>
<div>
<div>
<p class="MsoNormal"><span lang="EN-US">+ \
filename = opt.substring(5);<u></u><u></u></span></p> </div>
<div>
<p class="MsoNormal"><span lang="EN-US">+ \
if (filename != null) {<u></u><u></u></span></p> </div>
</div>
<div>
<p class="MsoNormal"><span lang="EN-US"> -> I don't see how filename \
can be null here<u></u><u></u></span></p> </div>
<div>
<p class="MsoNormal"><span lang="EN-US"> -> same filename could just be \
declared in the if<u></u><u></u></span></p> </div>
<div>
<p class="MsoNormal"><span lang="EN-US"> <u></u><u></u></span></p>
</div>
<div>
<p class="MsoNormal"><span lang="EN-US"> <u></u><u></u></span></p>
</div>
<div>
<div>
<p class="MsoNormal"><span lang="EN-US"> <u></u><u></u></span></p>
</div>
<div>
<p class="MsoNormal"><span lang="EN-US">+ \
} else if (subopt.startsWith("file=")) {<u></u><u></u></span></p> \
</div> <div>
<p class="MsoNormal"><span lang="EN-US">+ \
filename = parseFileName(subopt);<u></u><u></u></span></p> \
</div> </div>
<div>
<p class="MsoNormal"><span lang="EN-US"> <u></u><u></u></span></p>
</div>
<div>
<p class="MsoNormal"><span lang="EN-US">-> So actually you are testing twice if \
the string starts with "file=", maybe remove the one in the \
method?<u></u><u></u></span></p> </div>
<div>
<p class="MsoNormal"><span lang="EN-US"> <u></u><u></u></span></p>
</div>
<div>
<p class="MsoNormal"><span lang="EN-US">-> in the option string printouts, you \
don't specify that all is an option as well, is that normal?<u></u><u></u></span></p> \
</div> <div>
<p class="MsoNormal"><span lang="EN-US"> <u></u><u></u></span></p>
</div>
<div>
<p class="MsoNormal"><span lang="EN-US"> <u></u><u></u></span></p>
</div>
<div>
<p class="MsoNormal"><span lang="EN-US">The bigger issue I see is the passing of NULL \
for a filename, why do we not do things where you just really pass \
"-file=<file>" to the attachListener.cpp and handle the parsing \
there?; it would then make more sense to me: we either pass \
"-file=<file>" or not but we no longer have a "maybe there is or \
not a file, so maybe there is a NULL there".<u></u><u></u></span></p> </div>
<div>
<p class="MsoNormal"><span lang="EN-US"> <u></u><u></u></span></p>
</div>
<div>
<p class="MsoNormal"><span lang="EN-US">Thanks,<u></u><u></u></span></p>
</div>
<div>
<p class="MsoNormal"><span lang="EN-US">Jc<u></u><u></u></span></p>
</div>
</div>
</div>
</div>
</div>
</div>
</div>
</div>
</div>
</div>
</div>
<p class="MsoNormal"><span lang="EN-US"> <u></u><u></u></span></p>
<div>
<div>
<p class="MsoNormal"><span lang="EN-US">On Mon, Jan 7, 2019 at 2:11 AM \
</span>ê°ÁÕ<span lang="EN-US"> <<a href="mailto:zanglin5@jd.com" \
target="_blank">zanglin5@jd.com</a>> wrote:<u></u><u></u></span></p> </div>
<blockquote style="border-top:none;border-right:none;border-bottom:none;border-left:1pt \
solid rgb(204,204,204);padding:0cm 0cm 0cm 6pt;margin:5pt 0cm 5pt 4.8pt"> <div>
<div id="gmail-m_6631777943017640093gmail-m_8109715663797045723gmail-m_-8327135552520610154divtagdefaultwrapper">
<p><span lang="EN-US" style="font-family:Calibri,sans-serif;color:black">Hi \
Paul, </span><span lang="EN-US"><u></u><u></u></span></p> <div>
<p class="MsoNormal"><span lang="EN-US" \
style="font-family:Calibri,sans-serif;color:black"> I think it is not \
necessary to have a loop for argument processing, since there is not so much \
arguments to handle. </span><span lang="EN-US"><u></u><u></u></span></p> \
</div> <div>
<p class="MsoNormal"><span lang="EN-US" \
style="font-family:Calibri,sans-serif;color:black"> And I made a new \
webrev <a href="http://cr.openjdk.java.net/~xiaofeya/8215622/webrev.01/" \
id="gmail-m_6631777943017640093gmail-m_8109715663797045723gmail-m_-8327135552520610154LPlnk886791" \
target="_blank">http://cr.openjdk.java.net/~xiaofeya/8215622/webrev.01/</a></span><span \
lang="EN-US"><u></u><u></u></span></p> </div>
<div>
<p class="MsoNormal"><span lang="EN-US" \
style="font-family:Calibri,sans-serif;color:black"> </span><span \
lang="EN-US"><u></u><u></u></span></p> </div>
<div>
<p class="MsoNormal"><span lang="EN-US" \
style="font-family:Calibri,sans-serif;color:black">Hi All, </span><span \
lang="EN-US"><u></u><u></u></span></p> </div>
<div>
<p class="MsoNormal"><span lang="EN-US" \
style="font-family:Calibri,sans-serif;color:black"> May I also ask your \
help for reviewing this webrev?</span><span lang="EN-US"><u></u><u></u></span></p> \
</div> <div>
<p class="MsoNormal"><span lang="EN-US" \
style="font-family:Calibri,sans-serif;color:black"> webrev <a \
href="http://cr.openjdk.java.net/~xiaofeya/8215622/webrev.01/" \
id="gmail-m_6631777943017640093gmail-m_8109715663797045723gmail-m_-8327135552520610154LPlnk477888" \
target="_blank">http://cr.openjdk.java.net/~xiaofeya/8215622/webrev.01/</a></span><span \
lang="EN-US"><u></u><u></u></span></p> </div>
<div>
<p class="MsoNormal"><span lang="EN-US" \
style="font-family:Calibri,sans-serif;color:black"> Bug: <a \
href="https://bugs.openjdk.java.net/browse/JDK-8215622" \
id="gmail-m_6631777943017640093gmail-m_8109715663797045723gmail-m_-8327135552520610154LPlnk662293" \
target="_blank">https://bugs.openjdk.java.net/browse/JDK-8215622</a> </span><span \
lang="EN-US"><u></u><u></u></span></p> </div>
<div>
<p class="MsoNormal"><span lang="EN-US" \
style="font-family:Calibri,sans-serif;color:black"> </span><span \
lang="EN-US"><u></u><u></u></span></p> </div>
<div>
<p class="MsoNormal"><span lang="EN-US" \
style="font-family:Calibri,sans-serif;color:black">Thanks!</span><span \
lang="EN-US"><u></u><u></u></span></p> </div>
<div>
<p class="MsoNormal"><span lang="EN-US" \
style="font-family:Calibri,sans-serif;color:black">Lin</span><span \
lang="EN-US"><u></u><u></u></span></p> </div>
<div>
<p class="MsoNormal"><span lang="EN-US" \
style="font-family:Calibri,sans-serif;color:black"> </span><span \
lang="EN-US"><u></u><u></u></span></p> <div>
<div class="MsoNormal" align="center" style="text-align:center"><span lang="EN-US" \
style="font-family:Calibri,sans-serif;color:black"> <hr size="3" width="98%" \
align="center"> </span></div>
<div id="gmail-m_6631777943017640093gmail-m_8109715663797045723gmail-m_-8327135552520610154divRplyFwdMsg">
<p class="MsoNormal"><b><span \
style="font-size:11pt;color:black">·¢¼þÈË</span></b><b><span lang="EN-US" \
style="font-size:11pt;font-family:Calibri,sans-serif;color:black">:</span></b><span \
lang="EN-US" style="font-size:11pt;font-family:Calibri,sans-serif;color:black"> \
serviceability-dev <<a href="mailto:serviceability-dev-bounces@openjdk.java.net" \
target="_blank">serviceability-dev-bounces@openjdk.java.net</a>> </span><span \
style="font-size:11pt;color:black">´ú±í</span><span \
style="font-size:11pt;font-family:Calibri,sans-serif;color:black"> </span><span \
style="font-size:11pt;color:black">ê°ÁÕ</span><span lang="EN-US" \
style="font-size:11pt;font-family:Calibri,sans-serif;color:black"> <<a \
href="mailto:zanglin5@jd.com" target="_blank">zanglin5@jd.com</a>><br> \
</span><b><span style="font-size:11pt;color:black">·¢ËÍʱ¼ä</span></b><b><span \
lang="EN-US" style="font-size:11pt;font-family:Calibri,sans-serif;color:black">:</span></b><span \
lang="EN-US" style="font-size:11pt;font-family:Calibri,sans-serif;color:black"> \
2019</span><span style="font-size:11pt;color:black">Äê</span><span lang="EN-US" \
style="font-size:11pt;font-family:Calibri,sans-serif;color:black">1</span><span \
style="font-size:11pt;color:black">Ô </span><span lang="EN-US" \
style="font-size:11pt;font-family:Calibri,sans-serif;color:black">3</span><span \
style="font-size:11pt;color:black">ÈÕ</span><span lang="EN-US" \
style="font-size:11pt;font-family:Calibri,sans-serif;color:black"> 10:21<br>
</span><b><span style="font-size:11pt;color:black">ÊÕ¼þÈË</span></b><b><span \
lang="EN-US" style="font-size:11pt;font-family:Calibri,sans-serif;color:black">:</span></b><span \
lang="EN-US" style="font-size:11pt;font-family:Calibri,sans-serif;color:black"> \
Hohensee, Paul; <a href="mailto:serviceability-dev@openjdk.java.net" \
target="_blank">serviceability-dev@openjdk.java.net</a><br> </span><b><span \
style="font-size:11pt;color:black">Ö÷Ìâ</span></b><b><span lang="EN-US" \
style="font-size:11pt;font-family:Calibri,sans-serif;color:black">:</span></b><span \
lang="EN-US" style="font-size:11pt;font-family:Calibri,sans-serif;color:black"> \
[</span><span style="font-size:11pt;color:black">·¢¼þµØַαÔì</span><span \
lang="EN-US" style="font-size:11pt;font-family:Calibri,sans-serif;color:black">,</span><span \
style="font-size:11pt;color:black">¶ñÒâÓʼþ</span><span lang="EN-US" \
style="font-size:11pt;font-family:Calibri,sans-serif;color:black">,</span><span \
style="font-size:11pt;color:black">Îðµã</span><span lang="EN-US" \
style="font-size:11pt;font-family:Calibri,sans-serif;color:black">] RE: \
[RFR]8215622: Add dump to file support for jmap histo</span><span lang="EN-US" \
style="font-family:Calibri,sans-serif;color:black"> </span><span \
lang="EN-US"><u></u><u></u></span></p> <div>
<p class="MsoNormal"><span lang="EN-US" \
style="font-family:Calibri,sans-serif;color:black"> </span><span \
lang="EN-US"><u></u><u></u></span></p> </div>
</div>
<div>
<div>
<p style="margin:0cm 0cm 0.0001pt;text-align:justify"><span lang="EN-US" \
style="font-size:10.5pt;font-family:µÈÏß;color:rgb(31,73,125)">Dear Paul, \
</span><span lang="EN-US"><u></u><u></u></span></p> <p style="margin:0cm 0cm \
0.0001pt;text-align:justify"><span lang="EN-US" \
style="font-size:10.5pt;font-family:µÈÏß;color:rgb(31,73,125)"> \
Thanks a lot for your review! I am trying to remend the patch</span><span \
lang="EN-US"><u></u><u></u></span></p> <p style="margin:0cm 0cm \
0.0001pt;text-align:justify"><span lang="EN-US" \
style="font-size:10.5pt;font-family:µÈÏß;color:rgb(31,73,125)"> \
One problem is that in future, I will add more options for histo, such as \
</span><span style="font-size:10.5pt;font-family:µÈÏß;color:rgb(31,73,125)">¡°<span \
lang="EN-US">parallel</span>¡±<span lang="EN-US">, </span>¡°<span \
lang="EN-US">incremental</span>¡±<span lang="EN-US">. </span></span><span \
lang="EN-US"><u></u><u></u></span></p> <p style="margin:0cm 0cm \
0.0001pt;text-align:justify;text-indent:21pt"><span lang="EN-US" \
style="font-size:10.5pt;font-family:µÈÏß;color:rgb(31,73,125)">so do you thinking \
option processing with loop in heap_inspection() in attachListener.cpp \
would</span><span lang="EN-US"><u></u><u></u></span></p> <p style="margin:0cm 0cm \
0.0001pt;text-align:justify;text-indent:21pt"><span lang="EN-US" \
style="font-size:10.5pt;font-family:µÈÏß;color:rgb(31,73,125)">be more reasonable \
than the </span><span \
style="font-size:10.5pt;font-family:µÈÏß;color:rgb(31,73,125)">¡°<span \
lang="EN-US">if else</span>¡±<span lang="EN-US"> ? </span></span><span \
lang="EN-US"><u></u><u></u></span></p> <p style="margin:0cm 0cm \
0.0001pt;text-align:justify;text-indent:21pt"><span lang="EN-US" \
style="font-size:10.5pt;font-family:µÈÏß;color:rgb(31,73,125)">Thanks</span><span \
lang="EN-US"><u></u><u></u></span></p> <p style="margin:0cm 0cm \
0.0001pt;text-align:justify"><span lang="EN-US" \
style="font-size:10.5pt;font-family:µÈÏß;color:rgb(31,73,125)"> </span><span \
lang="EN-US"><u></u><u></u></span></p> <p style="margin:0cm 0cm \
0.0001pt;text-align:justify"><span lang="EN-US" \
style="font-size:10.5pt;font-family:µÈÏß;color:rgb(31,73,125)">BRs,</span><span \
lang="EN-US"><u></u><u></u></span></p> <p style="margin:0cm 0cm \
0.0001pt;text-align:justify"><span lang="EN-US" \
style="font-size:10.5pt;font-family:µÈÏß;color:rgb(31,73,125)">Lin</span><span \
lang="EN-US"><u></u><u></u></span></p> <p style="margin:0cm 0cm \
0.0001pt;text-align:justify"><span lang="EN-US" \
style="font-size:10.5pt;font-family:µÈÏß;color:rgb(31,73,125)"> </span><span \
lang="EN-US"><u></u><u></u></span></p> <div>
<p style="margin:0cm 0cm 0.0001pt;text-align:justify"><span lang="EN-US" \
style="font-size:14pt;font-family:΢ÈíÑźÚ,sans-serif;color:rgb(89,87,87)"> </span><span \
lang="EN-US"><u></u><u></u></span></p> <p style="margin:0cm 0cm \
0.0001pt;text-align:justify"><span lang="EN-US" \
style="font-size:14pt;font-family:΢ÈíÑźÚ,sans-serif;color:rgb(89,87,87)"> </span><span \
lang="EN-US"><u></u><u></u></span></p> </div>
<div>
<div style="border-right:none;border-bottom:none;border-left:none;border-top:1pt \
solid rgb(225,225,225);padding:3pt 0cm 0cm"> <p style="margin:0cm 0cm \
0.0001pt;text-align:justify"><b><span lang="EN-US" \
style="font-size:11pt;font-family:Calibri,sans-serif;color:black">From:</span></b><span \
lang="EN-US" style="font-size:11pt;font-family:Calibri,sans-serif;color:black"> \
Hohensee, Paul <<a href="mailto:hohensee@amazon.com" \
target="_blank">hohensee@amazon.com</a>> <br> <b>Sent:</b> Saturday, December 29, \
2018 3:54 AM<br> <b>To:</b> </span><span \
style="font-size:11pt;color:black">ê°ÁÕ</span><span lang="EN-US" \
style="font-size:11pt;font-family:Calibri,sans-serif;color:black"> <<a \
href="mailto:zanglin5@jd.com" target="_blank">zanglin5@jd.com</a>>; <a \
href="mailto:serviceability-dev@openjdk.java.net" \
target="_blank">serviceability-dev@openjdk.java.net</a><br> <b>Subject:</b> Re: \
[RFR]8215622: Add dump to file support for jmap histo</span><span \
lang="EN-US"><u></u><u></u></span></p> </div>
</div>
<p style="margin:0cm 0cm 0.0001pt;text-align:justify"><span lang="EN-US" \
style="font-size:10.5pt;font-family:µÈÏß;color:black"> </span><span \
lang="EN-US"><u></u><u></u></span></p> <p style="margin:0cm 0cm \
0.0001pt;text-align:justify"><span lang="EN-US" \
style="font-size:11pt;font-family:Calibri,sans-serif;color:black">Update the \
copyright dates to 2019 please, since this won¡¯t get pushed until then.</span><span \
lang="EN-US"><u></u><u></u></span></p> <p style="margin:0cm 0cm \
0.0001pt;text-align:justify"><span lang="EN-US" \
style="font-size:11pt;font-family:Calibri,sans-serif;color:black"> </span><span \
lang="EN-US"><u></u><u></u></span></p> <p style="margin:0cm 0cm \
0.0001pt;text-align:justify"><span lang="EN-US" \
style="font-size:11pt;font-family:Calibri,sans-serif;color:black">In JMap.java, I¡¯d \
emulate dump() and use</span><span lang="EN-US"><u></u><u></u></span></p> <p \
style="margin:0cm 0cm 0.0001pt;text-align:justify"><span lang="EN-US" \
style="font-size:11pt;font-family:Calibri,sans-serif;color:black"> </span><span \
lang="EN-US"><u></u><u></u></span></p> <pre><span lang="EN-US" \
style="font-size:10pt;font-family:"Courier \
New";color:black"> String filename = \
null;</span><span lang="EN-US"><u></u><u></u></span></pre> <pre><span lang="EN-US" \
style="font-size:10pt;font-family:"Courier New";color:black"> \
String subopts[] = \
options.split(",");</span><span lang="EN-US"><u></u><u></u></span></pre> <p \
style="margin:0cm 0cm 0.0001pt;text-align:justify"><span lang="EN-US" \
style="font-size:11pt;font-family:Calibri,sans-serif;color:black"> </span><span \
lang="EN-US"><u></u><u></u></span></p> <pre><span lang="EN-US" \
style="font-size:10pt;font-family:"Courier \
New";color:black"> for (String subopt \
: subopts){</span><span lang="EN-US"><u></u><u></u></span></pre> <pre><span \
lang="EN-US" style="font-size:10pt;font-family:"Courier \
New";color:black"> \
if (subopt.isEmpty() || subopt.equals("all")) {</span><span \
lang="EN-US"><u></u><u></u></span></pre> <pre><span lang="EN-US" \
style="font-size:10pt;font-family:"Courier \
New";color:black"> \
// pass</span><span lang="EN-US"><u></u><u></u></span></pre> <pre><span lang="EN-US" \
style="font-size:10pt;font-family:"Courier \
New";color:black"> \
} else if (subopt.equals("live")) {</span><span \
lang="EN-US"><u></u><u></u></span></pre> <pre><span lang="EN-US" \
style="font-size:10pt;font-family:"Courier \
New";color:black"> \
liveopt = "-live";</span><span lang="EN-US"><u></u><u></u></span></pre> \
<pre><span lang="EN-US" style="font-size:10pt;font-family:"Courier \
New";color:black"> \
} else if (subopt.startsWith("file=")) {</span><span \
lang="EN-US"><u></u><u></u></span></pre> <pre><span lang="EN-US" \
style="font-size:10pt;font-family:"Courier New";color:black"> \
// \
file=<file> - check that <file> is specified</span><span \
lang="EN-US"><u></u><u></u></span></pre> <pre><span lang="EN-US" \
style="font-size:10pt;font-family:"Courier New";color:black"> \
if \
(subopt.length() > 5) {</span><span lang="EN-US"><u></u><u></u></span></pre> \
<pre><span lang="EN-US" style="font-size:10pt;font-family:"Courier \
New";color:black"> \
filename \
= subopt.substring(5);</span><span lang="EN-US"><u></u><u></u></span></pre> \
<pre><span lang="EN-US" style="font-size:10pt;font-family:"Courier \
New";color:black"> \
}</span><span lang="EN-US"><u></u><u></u></span></pre> <pre><span lang="EN-US" \
style="font-size:10pt;font-family:"Courier \
New";color:black"> \
} else {</span><span lang="EN-US"><u></u><u></u></span></pre> <pre><span lang="EN-US" \
style="font-size:10pt;font-family:"Courier \
New";color:black"> \
usage(1);</span><span lang="EN-US"><u></u><u></u></span></pre> <pre><span \
lang="EN-US" style="font-size:10pt;font-family:"Courier \
New";color:black"> \
}</span><span lang="EN-US"><u></u><u></u></span></pre> <pre><span lang="EN-US" \
style="font-size:10pt;font-family:"Courier \
New";color:black"> }</span><span \
lang="EN-US"><u></u><u></u></span></pre> <pre><span lang="EN-US" \
style="font-size:10pt;font-family:"Courier \
New";color:black"> </span><span lang="EN-US"><u></u><u></u></span></pre> \
<pre><span lang="EN-US" style="font-size:10pt;font-family:"Courier \
New";color:black"> // get the \
canonical path - important to avoid just passing</span><span \
lang="EN-US"><u></u><u></u></span></pre> <pre><span lang="EN-US" \
style="font-size:10pt;font-family:"Courier \
New";color:black"> // a \
"heap.bin" and having the dump created in the target VM</span><span \
lang="EN-US"><u></u><u></u></span></pre> <pre><span lang="EN-US" \
style="font-size:10pt;font-family:"Courier \
New";color:black"> // working \
directory rather than the directory where jmap</span><span \
lang="EN-US"><u></u><u></u></span></pre> <pre><span lang="EN-US" \
style="font-size:10pt;font-family:"Courier \
New";color:black"> // is \
executed.</span><span lang="EN-US"><u></u><u></u></span></pre> <pre><span \
lang="EN-US" style="font-size:10pt;font-family:"Courier \
New";color:black"> filename = \
new File(filename).getCanonicalPath();</span><span \
lang="EN-US"><u></u><u></u></span></pre> <pre><span lang="EN-US" \
style="font-size:10pt;font-family:"Courier New";color:black"> \
// inspectHeap is not the same as jcmd \
GC.class_histogram</span><span lang="EN-US"><u></u><u></u></span></pre> <pre><span \
lang="EN-US" style="font-size:10pt;font-family:"Courier New";color:black"> \
executeCommandForPid(pid, \
"inspectheap", filename, liveopt);</span><span \
lang="EN-US"><u></u><u></u></span></pre> <pre><span lang="EN-US" \
style="font-size:10pt;font-family:"Courier \
New";color:black"> </span><span lang="EN-US"><u></u><u></u></span></pre> <p \
style="margin:0cm 0cm 0.0001pt;text-align:justify"><span lang="EN-US" \
style="font-size:11pt;font-family:Calibri,sans-serif;color:black">I.e., use an \
enhanced for loop to scan the array, and duplicate dump()¡¯s executeCommandForPid() \
argument order, as well as dump()¡¯s ¡°file=<>¡± check (the code that starts \
with ¡°if (subopt.startsWith¡±) and canonicalization. Actually, better to factor the \
latter out into its own method and use it from both histo() and dump().</span><span \
lang="EN-US"><u></u><u></u></span></p> <p style="margin:0cm 0cm \
0.0001pt;text-align:justify"><span lang="EN-US" \
style="font-size:11pt;font-family:Calibri,sans-serif;color:black"> </span><span \
lang="EN-US"><u></u><u></u></span></p> <p style="margin:0cm 0cm \
0.0001pt;text-align:justify"><span lang="EN-US" \
style="font-size:11pt;font-family:Calibri,sans-serif;color:black">The argument \
checking code in heap_inspection() in attachListener.cpp can be simplified along the \
lines of dump_heap(). I.e., you don¡¯t need to loop over the argument list. To match \
up with dump_heap()¡¯s info messages, the info message string at the end should be \
¡°Heap inspection file created: %s¡±.</span><span \
lang="EN-US"><u></u><u></u></span></p> <p style="margin:0cm 0cm \
0.0001pt;text-align:justify"><span lang="EN-US" \
style="font-size:11pt;font-family:Calibri,sans-serif;color:black"> </span><span \
lang="EN-US"><u></u><u></u></span></p> <p style="margin:0cm 0cm \
0.0001pt;text-align:justify"><span lang="EN-US" \
style="font-size:11pt;font-family:Calibri,sans-serif;color:black">Thanks,</span><span \
lang="EN-US"><u></u><u></u></span></p> <p style="margin:0cm 0cm \
0.0001pt;text-align:justify"><span lang="EN-US" \
style="font-size:11pt;font-family:Calibri,sans-serif;color:black"> </span><span \
lang="EN-US"><u></u><u></u></span></p> <p style="margin:0cm 0cm \
0.0001pt;text-align:justify"><span lang="EN-US" \
style="font-size:11pt;font-family:Calibri,sans-serif;color:black">Paul</span><span \
lang="EN-US"><u></u><u></u></span></p> <p style="margin:0cm 0cm \
0.0001pt;text-align:justify"><span lang="EN-US" \
style="font-size:11pt;font-family:Calibri,sans-serif;color:black"> </span><span \
lang="EN-US"><u></u><u></u></span></p> <div \
style="border-right:none;border-bottom:none;border-left:none;border-top:1pt solid \
rgb(181,196,223);padding:3pt 0cm 0cm"> <p style="margin:0cm 0cm \
0.0001pt;text-align:justify"><b><span lang="EN-US" \
style="font-family:µÈÏß;color:black">From: </span></b><span lang="EN-US" \
style="font-family:µÈÏß;color:black">serviceability-dev <<a \
href="mailto:serviceability-dev-bounces@openjdk.java.net" target="_blank"><span \
style="color:rgb(5,99,193)">serviceability-dev-bounces@openjdk.java.net</span></a>> \
on behalf of </span><span style="font-family:µÈÏß;color:black">ê°ÁÕ<span \
lang="EN-US"> <<a href="mailto:zanglin5@jd.com" target="_blank"><span \
style="color:rgb(5,99,193)">zanglin5@jd.com</span></a>><br> <b>Date: </b>Thursday, \
December 20, 2018 at 11:03 PM<br> <b>To: </b>"<a \
href="mailto:serviceability-dev@openjdk.java.net" target="_blank"><span \
style="color:rgb(5,99,193)">serviceability-dev@openjdk.java.net</span></a>" \
<<a href="mailto:serviceability-dev@openjdk.java.net" target="_blank"><span \
style="color:rgb(5,99,193)">serviceability-dev@openjdk.java.net</span></a>><br> \
<b>Subject: </b>[RFR]8215622: Add dump to file support for jmap \
histo</span></span><span lang="EN-US"><u></u><u></u></span></p> </div>
<div>
<p style="margin:0cm 0cm 0.0001pt;text-align:justify"><span lang="EN-US" \
style="font-size:11pt;font-family:µÈÏß;color:black"> </span><span \
lang="EN-US"><u></u><u></u></span></p> </div>
<p style="margin:0cm 0cm 0.0001pt;text-align:justify"><span lang="EN-US" \
style="font-size:10.5pt;font-family:µÈÏß;color:black">Hi All, </span><span \
lang="EN-US"><u></u><u></u></span></p> <p style="margin:0cm 0cm \
0.0001pt;text-align:justify"><span lang="EN-US" \
style="font-size:10.5pt;font-family:µÈÏß;color:black"> \
May I ask your help to review this patch for enhance jmap </span><span \
style="font-size:10.5pt;font-family:µÈÏß;color:black">¨C<span lang="EN-US">histo. \
</span></span><span lang="EN-US"><u></u><u></u></span></p> <p style="margin:0cm 0cm \
0.0001pt;text-align:justify;text-indent:21pt"><span lang="EN-US" \
style="font-size:10.5pt;font-family:µÈÏß;color:black">It add the </span><span \
style="font-size:10.5pt;font-family:µÈÏß;color:black">¡°<span \
lang="EN-US">file=<path></span>¡±<span lang="EN-US"> arguments that allow jmap \
</span>¨C<span lang="EN-US">histo outputs data to file directly.</span></span><span \
lang="EN-US"><u></u><u></u></span></p> <p style="margin:0cm 0cm \
0.0001pt;text-align:justify;text-indent:21pt"><span lang="EN-US" \
style="font-size:10.5pt;font-family:µÈÏß;color:black">This patch is also part of the \
enhancement described in <a href="https://bugs.openjdk.java.net/browse/JDK-8214535" \
target="_blank"><span \
style="color:rgb(5,99,193)">https://bugs.openjdk.java.net/browse/JDK-8214535</span></a>.</span><span \
lang="EN-US"><u></u><u></u></span></p> <p style="margin:0cm 0cm \
0.0001pt;text-align:justify;text-indent:21pt"><span lang="EN-US" \
style="font-size:10.5pt;font-family:µÈÏß;color:black"> </span><span \
lang="EN-US"><u></u><u></u></span></p> <p style="margin:0cm 0cm \
0.0001pt;text-align:justify;text-indent:21pt"><span lang="EN-US" \
style="font-size:10.5pt;font-family:µÈÏß;color:black">Webrev: <a \
href="http://cr.openjdk.java.net/~xiaofeya/8215622/webrev.00/" target="_blank"> <span \
style="color:rgb(5,99,193)">http://cr.openjdk.java.net/~xiaofeya/8215622/webrev.00/</span></a></span><span \
lang="EN-US"><u></u><u></u></span></p> <p style="margin:0cm 0cm \
0.0001pt;text-align:justify;text-indent:21pt"><span lang="EN-US" \
style="font-size:10.5pt;font-family:µÈÏß;color:black">Bug: <a \
href="https://bugs.openjdk.java.net/browse/JDK-8215622" target="_blank"><span \
style="color:rgb(5,99,193)">https://bugs.openjdk.java.net/browse/JDK-8215622</span></a></span><span \
lang="EN-US"><u></u><u></u></span></p> <p style="margin:0cm 0cm \
0.0001pt;text-align:justify;text-indent:21pt"><span lang="EN-US" \
style="font-size:10.5pt;font-family:µÈÏß;color:black"> </span><span \
lang="EN-US"><u></u><u></u></span></p> <p style="margin:0cm 0cm \
0.0001pt;text-align:justify;text-indent:21pt"><span lang="EN-US" \
style="font-size:10.5pt;font-family:µÈÏß;color:black">Thanks.</span><span \
lang="EN-US"><u></u><u></u></span></p> <p style="margin:0cm 0cm \
0.0001pt;text-align:justify;text-indent:21pt"><span lang="EN-US" \
style="font-size:10.5pt;font-family:µÈÏß;color:black"> </span><span \
lang="EN-US"><u></u><u></u></span></p> <p style="margin:0cm 0cm \
0.0001pt;text-align:justify"><span lang="EN-US" \
style="font-size:10.5pt;font-family:µÈÏß;color:black">BRs,</span><span \
lang="EN-US"><u></u><u></u></span></p> <p style="margin:0cm 0cm \
0.0001pt;text-align:justify"><span lang="EN-US" \
style="font-size:10.5pt;font-family:µÈÏß;color:black">Lin </span><span \
lang="EN-US"><u></u><u></u></span></p> <p style="margin:0cm 0cm \
0.0001pt;text-align:justify"><span lang="EN-US" \
style="font-size:10.5pt;font-family:µÈÏß;color:black"> </span><span \
lang="EN-US"><u></u><u></u></span></p> <p style="margin:0cm 0cm \
0.0001pt;text-align:justify"><span lang="EN-US" \
style="font-size:14pt;font-family:΢ÈíÑźÚ,sans-serif;color:rgb(89,87,87)"> </span><span \
lang="EN-US"><u></u><u></u></span></p> <p style="margin:0cm 0cm \
0.0001pt;text-align:justify"><span lang="EN-US" \
style="font-size:10.5pt;font-family:µÈÏß;color:black"> </span><span \
lang="EN-US"><u></u><u></u></span></p> </div>
</div>
</div>
</div>
</div>
</div>
</blockquote>
</div>
<p class="MsoNormal"><span lang="EN-US"><br clear="all">
<u></u><u></u></span></p>
<div>
<p class="MsoNormal"><span lang="EN-US"> <u></u><u></u></span></p>
</div>
<p class="MsoNormal"><span lang="EN-US">-- <u></u><u></u></span></p>
<div>
<div>
<div>
<p class="MsoNormal"><span lang="EN-US"> <u></u><u></u></span></p>
</div>
<p class="MsoNormal"><span lang="EN-US">Thanks,<u></u><u></u></span></p>
<div>
<p class="MsoNormal"><span lang="EN-US">Jc<u></u><u></u></span></p>
</div>
</div>
</div>
</div>
</div>
</blockquote>
</div>
<p class="MsoNormal"><span lang="EN-US"><br clear="all">
<u></u><u></u></span></p>
<div>
<p class="MsoNormal"><span lang="EN-US"><u></u> <u></u></span></p>
</div>
<p class="MsoNormal"><span lang="EN-US">-- <u></u><u></u></span></p>
<div>
<div>
<div>
<p class="MsoNormal"><span lang="EN-US"><u></u> <u></u></span></p>
</div>
<p class="MsoNormal"><span lang="EN-US">Thanks,<u></u><u></u></span></p>
<div>
<p class="MsoNormal"><span lang="EN-US">Jc<u></u><u></u></span></p>
</div>
</div>
</div>
</div>
</div>
</div>
</blockquote>
</div>
<br clear="all">
<div><br>
</div>
-- <br>
<div dir="ltr" class="gmail_signature">
<div dir="ltr">
<div><br>
</div>
Thanks,
<div>Jc</div>
</div>
</div>
</div>
</body>
</html>
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic