[prev in list] [next in list] [prev in thread] [next in thread] 

List:       openjdk-serviceability-dev
Subject:    Re: [RFR]8215622: Add dump to file support for jmap histo
From:       "Hohensee, Paul" <hohensee () amazon ! com>
Date:       2018-12-28 19:53:38
Message-ID: FC57C8D0-AF53-4244-8B36-DD8F43307FD3 () amazon ! com
[Download RAW message or body]

[Attachment #2 (text/plain)]

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> on behalf of \
                臧琳 <zanglin5@jd.com>
Date: Thursday, December 20, 2018 at 11:03 PM
To: "serviceability-dev@openjdk.java.net" <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 –histo.
It add the "file=<path>" arguments that allow jmap –histo 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


[Attachment #3 (text/html)]

<html xmlns:o="urn:schemas-microsoft-com:office:office" \
xmlns:w="urn:schemas-microsoft-com:office:word" \
xmlns:m="http://schemas.microsoft.com/office/2004/12/omml" \
xmlns="http://www.w3.org/TR/REC-html40"> <head>
<meta http-equiv="Content-Type" content="text/html; charset=utf-8">
<meta name="Generator" content="Microsoft Word 15 (filtered medium)">
<style><!--
/* Font Definitions */
@font-face
	{font-family:"Cambria Math";
	panose-1:2 4 5 3 5 4 6 3 2 4;}
@font-face
	{font-family:DengXian;
	panose-1:2 1 6 0 3 1 1 1 1 1;}
@font-face
	{font-family:Calibri;
	panose-1:2 15 5 2 2 2 4 3 2 4;}
@font-face
	{font-family:"\@DengXian";
	panose-1:2 1 6 0 3 1 1 1 1 1;}
@font-face
	{font-family:微软雅黑;
	panose-1:2 11 6 4 2 2 2 2 2 4;}
/* Style Definitions */
p.MsoNormal, li.MsoNormal, div.MsoNormal
	{margin:0in;
	margin-bottom:.0001pt;
	text-align:justify;
	font-size:10.5pt;
	font-family:DengXian;}
a:link, span.MsoHyperlink
	{mso-style-priority:99;
	color:#0563C1;
	text-decoration:underline;}
a:visited, span.MsoHyperlinkFollowed
	{mso-style-priority:99;
	color:#954F72;
	text-decoration:underline;}
pre
	{mso-style-priority:99;
	mso-style-link:"HTML Preformatted Char";
	margin:0in;
	margin-bottom:.0001pt;
	font-size:10.0pt;
	font-family:"Courier New";}
p.MsoListParagraph, li.MsoListParagraph, div.MsoListParagraph
	{mso-style-priority:34;
	margin-top:0in;
	margin-right:0in;
	margin-bottom:0in;
	margin-left:.5in;
	margin-bottom:.0001pt;
	text-align:justify;
	font-size:10.5pt;
	font-family:DengXian;}
p.msonormal0, li.msonormal0, div.msonormal0
	{mso-style-name:msonormal;
	mso-margin-top-alt:auto;
	margin-right:0in;
	mso-margin-bottom-alt:auto;
	margin-left:0in;
	font-size:11.0pt;
	font-family:"Calibri",sans-serif;}
span.EmailStyle18
	{mso-style-type:personal;
	font-family:DengXian;
	color:windowtext;}
span.EmailStyle19
	{mso-style-type:personal-reply;
	font-family:"Calibri",sans-serif;
	color:windowtext;}
span.HTMLPreformattedChar
	{mso-style-name:"HTML Preformatted Char";
	mso-style-priority:99;
	mso-style-link:"HTML Preformatted";
	font-family:"Courier New";}
span.new
	{mso-style-name:new;}
.MsoChpDefault
	{mso-style-type:export-only;
	font-size:10.0pt;}
@page WordSection1
	{size:8.5in 11.0in;
	margin:1.0in 1.25in 1.0in 1.25in;}
div.WordSection1
	{page:WordSection1;}
/* List Definitions */
@list l0
	{mso-list-id:523517809;
	mso-list-type:hybrid;
	mso-list-template-ids:-1628670472 67698703 67698713 67698715 67698703 67698713 \
67698715 67698703 67698713 67698715;} @list l0:level1
	{mso-level-tab-stop:none;
	mso-level-number-position:left;
	text-indent:-.25in;}
@list l0:level2
	{mso-level-number-format:alpha-lower;
	mso-level-tab-stop:none;
	mso-level-number-position:left;
	text-indent:-.25in;}
@list l0:level3
	{mso-level-number-format:roman-lower;
	mso-level-tab-stop:none;
	mso-level-number-position:right;
	text-indent:-9.0pt;}
@list l0:level4
	{mso-level-tab-stop:none;
	mso-level-number-position:left;
	text-indent:-.25in;}
@list l0:level5
	{mso-level-number-format:alpha-lower;
	mso-level-tab-stop:none;
	mso-level-number-position:left;
	text-indent:-.25in;}
@list l0:level6
	{mso-level-number-format:roman-lower;
	mso-level-tab-stop:none;
	mso-level-number-position:right;
	text-indent:-9.0pt;}
@list l0:level7
	{mso-level-tab-stop:none;
	mso-level-number-position:left;
	text-indent:-.25in;}
@list l0:level8
	{mso-level-number-format:alpha-lower;
	mso-level-tab-stop:none;
	mso-level-number-position:left;
	text-indent:-.25in;}
@list l0:level9
	{mso-level-number-format:roman-lower;
	mso-level-tab-stop:none;
	mso-level-number-position:right;
	text-indent:-9.0pt;}
ol
	{margin-bottom:0in;}
ul
	{margin-bottom:0in;}
--></style>
</head>
<body lang="EN-US" link="#0563C1" vlink="#954F72">
<div class="WordSection1">
<p class="MsoNormal"><span \
style="font-size:11.0pt;font-family:&quot;Calibri&quot;,sans-serif">Update the \
copyright dates to 2019 please, since this won't get pushed until \
then.<o:p></o:p></span></p> <p class="MsoNormal"><span \
style="font-size:11.0pt;font-family:&quot;Calibri&quot;,sans-serif"><o:p>&nbsp;</o:p></span></p>
 <p class="MsoNormal"><span \
style="font-size:11.0pt;font-family:&quot;Calibri&quot;,sans-serif">In JMap.java, I'd \
emulate dump() and use<o:p></o:p></span></p> <p class="MsoNormal"><span \
style="font-size:11.0pt;font-family:&quot;Calibri&quot;,sans-serif"><o:p>&nbsp;</o:p></span></p>
 <pre>&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp; String filename = \
null;<o:p></o:p></pre> <pre> &nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;String \
subopts[] = options.split(&quot;,&quot;);<o:p></o:p></pre> <p class="MsoNormal"><span \
style="font-size:11.0pt;font-family:&quot;Calibri&quot;,sans-serif"><o:p>&nbsp;</o:p></span></p>
 <pre>&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp; for (String subopt : \
subopts){<o:p></o:p></pre> \
<pre>&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp; if \
(subopt.isEmpty() || subopt.equals(&quot;all&quot;)) {<o:p></o:p></pre> \
<pre>&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp; \
// pass<o:p></o:p></pre> \
<pre>&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp; } else if \
(subopt.equals(&quot;live&quot;)) {<o:p></o:p></pre> \
<pre>&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp; \
liveopt = &quot;-live&quot;;<o:p></o:p></pre> \
<pre>&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp; } else if \
(subopt.startsWith(&quot;file=&quot;)) {<o:p></o:p></pre> <pre>&nbsp;&nbsp; \
&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;// \
file=&lt;file&gt; - check that &lt;file&gt; is specified<o:p></o:p></pre> <pre> \
&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;if \
(subopt.length() &gt; 5) {<o:p></o:p></pre> <pre> \
&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;filename \
= subopt.substring(5);<o:p></o:p></pre> \
<pre>&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp; \
}<o:p></o:p></pre> <pre>&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp; \
} else {<o:p></o:p></pre> \
<pre>&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp; \
usage(1);<o:p></o:p></pre> \
<pre>&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp; \
}<o:p></o:p></pre> <pre>&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp; \
}<o:p></o:p></pre> <pre><o:p>&nbsp;</o:p></pre>
<pre>&nbsp;&nbsp;&nbsp; &nbsp;&nbsp;&nbsp;&nbsp;&nbsp;// get the canonical path - \
important to avoid just passing<o:p></o:p></pre> <pre>&nbsp;&nbsp;&nbsp; \
&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;// a &quot;heap.bin&quot; and having the dump created \
in the target VM<o:p></o:p></pre> <pre>&nbsp;&nbsp;&nbsp; \
&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;// working directory rather than the directory where \
jmap<o:p></o:p></pre> <pre>&nbsp;&nbsp;&nbsp; &nbsp;&nbsp;&nbsp;&nbsp;&nbsp;// is \
executed.<o:p></o:p></pre> <pre>&nbsp;&nbsp;&nbsp;&nbsp; \
&nbsp;&nbsp;&nbsp;&nbsp;filename = new \
File(filename).getCanonicalPath();<o:p></o:p></pre> <pre>&nbsp; \
&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;// inspectHeap is not the same as jcmd \
GC.class_histogram<o:p></o:p></pre> <pre> \
&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;executeCommandForPid(pid, \
&quot;inspectheap&quot;, filename, liveopt);<o:p></o:p></pre> \
<pre><o:p>&nbsp;</o:p></pre> <p class="MsoNormal"><span \
style="font-size:11.0pt;font-family:&quot;Calibri&quot;,sans-serif">I.e., use an \
enhanced for loop to scan the array, and duplicate dump()'s executeCommandForPid() \
argument order, as well as dump()'s "file=&lt;&gt;" 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().<o:p></o:p></span></p> <p class="MsoNormal"><span \
style="font-size:11.0pt;font-family:&quot;Calibri&quot;,sans-serif"><o:p>&nbsp;</o:p></span></p>
 <p class="MsoNormal"><span \
style="font-size:11.0pt;font-family:&quot;Calibri&quot;,sans-serif">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".<o:p></o:p></span></p> <p class="MsoNormal"><span \
style="font-size:11.0pt;font-family:&quot;Calibri&quot;,sans-serif"><o:p>&nbsp;</o:p></span></p>
 <p class="MsoNormal"><span \
style="font-size:11.0pt;font-family:&quot;Calibri&quot;,sans-serif">Thanks,<o:p></o:p></span></p>
 <p class="MsoNormal"><span \
style="font-size:11.0pt;font-family:&quot;Calibri&quot;,sans-serif"><o:p>&nbsp;</o:p></span></p>
 <p class="MsoNormal"><span \
style="font-size:11.0pt;font-family:&quot;Calibri&quot;,sans-serif">Paul<o:p></o:p></span></p>
 <p class="MsoNormal"><span \
style="font-size:11.0pt;font-family:&quot;Calibri&quot;,sans-serif"><o:p>&nbsp;</o:p></span></p>
 <div style="border:none;border-top:solid #B5C4DF 1.0pt;padding:3.0pt 0in 0in 0in">
<p class="MsoNormal"><b><span style="font-size:12.0pt;color:black">From: \
</span></b><span style="font-size:12.0pt;color:black">serviceability-dev \
&lt;serviceability-dev-bounces@openjdk.java.net&gt; on behalf of 臧琳 \
&lt;zanglin5@jd.com&gt;<br> <b>Date: </b>Thursday, December 20, 2018 at 11:03 PM<br>
<b>To: </b>&quot;serviceability-dev@openjdk.java.net&quot; \
&lt;serviceability-dev@openjdk.java.net&gt;<br> <b>Subject: </b>[RFR]8215622: Add \
dump to file support for jmap histo<o:p></o:p></span></p> </div>
<div>
<p class="MsoNormal"><span style="font-size:11.0pt"><o:p>&nbsp;</o:p></span></p>
</div>
<p class="MsoNormal">Hi All, <o:p></o:p></p>
<p class="MsoNormal">&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp; May I ask your help to \
review this patch for enhance jmap –histo. <o:p></o:p></p>
<p class="MsoNormal" style="text-indent:21.0pt">It add the "file=&lt;path&gt;" \
arguments that allow jmap –histo outputs data to file directly.<o:p></o:p></p> <p \
class="MsoNormal" style="text-indent:21.0pt">This patch is also part of the \
enhancement described in <a \
href="https://bugs.openjdk.java.net/browse/JDK-8214535">https://bugs.openjdk.java.net/browse/JDK-8214535</a>.<o:p></o:p></p>
 <p class="MsoNormal" style="text-indent:21.0pt">&nbsp;<o:p></o:p></p>
<p class="MsoNormal" style="text-indent:21.0pt">Webrev: <a \
href="http://cr.openjdk.java.net/~xiaofeya/8215622/webrev.00/"> \
http://cr.openjdk.java.net/~xiaofeya/8215622/webrev.00/</a><o:p></o:p></p> <p \
class="MsoNormal" style="text-indent:21.0pt">Bug: <a \
href="https://bugs.openjdk.java.net/browse/JDK-8215622"> \
https://bugs.openjdk.java.net/browse/JDK-8215622</a><o:p></o:p></p> <p \
class="MsoNormal" style="text-indent:21.0pt">&nbsp;<o:p></o:p></p> <p \
class="MsoNormal" style="text-indent:21.0pt">Thanks.<o:p></o:p></p> <p \
class="MsoNormal" style="text-indent:21.0pt">&nbsp;<o:p></o:p></p> <p \
class="MsoNormal">BRs,<o:p></o:p></p> <p class="MsoNormal">Lin&nbsp;&nbsp; \
<o:p></o:p></p> <p class="MsoNormal">&nbsp;<o:p></o:p></p>
<p class="MsoNormal"><span \
style="font-size:14.0pt;font-family:微软雅黑;color:#595757">&nbsp;</span><o:p></o:p></p>
 <p class="MsoNormal">&nbsp;<o:p></o:p></p>
</div>
</body>
</html>



[prev in list] [next in list] [prev in thread] [next in thread] 

Configure | About | News | Add a list | Sponsored by KoreLogic