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

List:       openjdk-serviceability-dev
Subject:    RE: [RFR]8215623: Add incremental dump for jmap histo
From:       臧琳 <zanglin5 () jd ! com>
Date:       2019-05-20 15:14:34
Message-ID: 5c6c2f4df6844d9ba118262d55990638 () jd ! com
[Download RAW message or body]

[Attachment #2 (text/plain)]

Dear Serguei,
The main reason of using separate file for incremental dump is due to the \
consideration of parallel incremental dump implementation, so that every \
heap-iteration thread could dump its own data in  separate file, to avoid using file \
lock. I originally made the design of whole file-support, incremental, parallel dump \
together, and then divided them into three sub-tasks, trying to make each task easy \
to discuss and review.  In that design, the file=<file> option is for final results, \
"incrementalHisto.dump.<thread_id>" is for thread "local" incremental dumped data. \
For incremental dump, I agree it is better if we can get rid of incremental-file \
options and using file=<file>, but I think we need to discuss how to make it \
adaptable for parallel dump. What do you think?

Thanks,
Lin


From: serguei.spitsyn@oracle.com <serguei.spitsyn@oracle.com>
Sent: Saturday, May 18, 2019 2:42 AM
To: 臧琳 <zanglin5@jd.com>
Cc: Hohensee, Paul <hohensee@amazon.com>; JC Beyler <jcbeyler@google.com>; \
                serviceability-dev@openjdk.java.net
Subject: Re: [RFR]8215623: Add incremental dump for jmap histo

Dear Lin,

Before I go to the details below could you, please, explain why do we need a separate \
file for incremental dump. Should we just record the full dump into file=<file> \
incrementally? Recording into the full dump file just has to be always incremental.
It can be done by chunks if necessary, so I'm open to consider introducing new \
chunksize option.

Do I miss anything here?

Thanks,
Serguei

On 5/17/19 03:18, 臧琳 wrote:
Dear Serguei,


在 2019年5月16日,下午1:39,serguei.spitsyn@oracle.com<mailto:serguei.spitsyn@oracle.com> \
写道:

On 5/13/19 23:46, 臧琳 wrote:

Dear Serguei,

     Thanks for your comments.



 > > - incremental[:<file_name>], enable the incremental dump of heap, dumped

 > >   data will be saved to, by default it is "IncrementalHisto.dump"

 >

 >  Q1: Should the <file_name> be full path or short name?

 >      Is there any default path? What is the path of the

 > "IncrementalHisto.dump" file?



The original design doesn't have the <file_name> option so the file is hardcoded \
named "IncrementalHisto.dump" and save to the same path as "file=" specified. Or \
print to whatever output stream is if "file=" is not set.

> The file option is described as: file=<file> dump data to <file>
> It does not tell anything about the path.

Yes, do you agree that we add a comment in the help info like:
file=<file> dump data to <file>, file can be specified with full path.

With the new design, I suggest firstly parse <file_name>, if the value contains \
folder path, use the specified path, if not, use same path as "file=" value,  and if \
"file=" is not set, use output stream. (The reason I prefer to use same path as \
"file=" is I assume that users prefer to save all data file under the same folder.)

> It needs to be clearly specified.
> What statements do you suggest?
> 
> One idea of simplification is to get rid of the default <file_name>
> and to require it to be always specified (non-optional).
> 
> Then we could replace this:

> file=<file> dump data to <file>

> incremental dump support:

> incremental[:<file_name>]  enable incremental dump, data will be dumped

> to <file_name> (default is "IncrementalHisto.dump")
> 
> with this:

> file=<file> dump data to <file>

> incremental=<inc_file>  dump incremental data to <inc_file>
I think having a default IncrementalHisto.dump file saved at the same path of the \
<file> is a way to make incremental easy to use. IMHO, when user use jmap -histo with \
"file=<file>", and want to enable inremental histo, the easiest way is just use \
"-incremental" flag and all data files will be  saved under the same folder of \
<file>. They don't have to consider  the specific filename for incremental \
additionally. This is the reason I set default value of IncrementalHisto.dump.

But I also want the user to have freedom to use different filename and path for \
incremental results, so I  make it optional for incremental file_name.

If we can make it non-optional, does it mean that user may have following command:
       Jmap -histo,file=<absoult_path/a/b/c/histo.dump>,incremental:<absoult_path/a/b/c/incrementalHisto.dump> \
pid It seems a little bit complicated to me, what do you think?





 > > - chunksize=<N>, size of objects (in KB) will be dumped in one chunk.

 >

 > Q2: Should it be chunk of dump, not chunk of objects?



The purpose of "chunksize" is to decide how many objects' info are dumped at once. \
for example use "chunksize=1" on a "Xmx1m", there will be at max 1MB/1KB = 1000 \
chunks, which indicates that there will be 1000 times of file writing when do "jmap \
-histo".

> I hardly understand the point to know max of objects that can be dumped at once.
> It is more important to know how much memory in the file it is going to take.
> How much of dump memory will take one object?
> Does it vary (does it depend on object types)?

Yes, the dump memory for one object varies from size and types.
The option"chunksize" is for user to control the proportion of heap that the \
incremental dump can process at a time.  IMO the use scenario is as following:

   when the JVM have an 180GB max heap size, and jmap histo used with chunksize=1g, \
it means the incremental dump happens when every 1GB heap is scaned, so it does't has \
too much incremenal dump, because the incremental dump takes time and may cause jmap \
-histo work slower.

PS, I think we should support "g","m" and "k" instead of using "KB" , do you agree?



 > > - maxfilesize=<N>, size of the incremental data dump file (in KB), when data \
size

 > >   is larger than maxfilesize, the file is erased and latest data will be \
written.



 > Q3: What is a relation and limitations between chunksize and maxfilesize?

 >    Should the maxfilesize be multiple of the chunksize?

> The question Q3 above was not unanswered.
> But never mind. Please, see the suggestion below.

If the chunksize it large, and there are too much objects of different classes in \
heap, the actual filesize can be larger than the maxfilesize. But I believe this is \
raraly happened because the size of one class's histo info only takes several bytes \
in the final result,  and from the implementation of jmap, it can find out all loaded \
classes before doing heap iteration, so the different of result only happens on the \
object quantity.

 > Q4: The sentence "the file is erased and latest data will be written"

is not clear enough.

 >    Why the whole file needs to be erased

 >    Should the incremental file behave like a cyclic buffer?

 >    If so, then only next chunk needs to be erased.

 >    Then the chunks need to be numbered in order, so the earliest one can be found.



The "maxfilesize" controls the file size not to be too large, so when the dumped data \
is larger than "maxfilesize", the file is erased and latest data are written.The \
reason I erase whole file is that chunk data is accumulative, so the latest data \
includes the previous statistical ones. And this way may make the file easy to read.



I agree that we can add ordered number in chunks, I think it more or less help user \
to get to know how object distributed in heap.



I think maybe it is reasonable to have the incremental file behave like gclog, when \
maxfilesize is reached, the file is renamed with numbered suffix, and new file is \
created to use. so there can be IncrementalHisto.dump.0 and IncrementalHisto.dump.1 \
etc for large heap.



what do you think?

> I think, it is not a bad idea.
> In general, new incremental feature design does not look simple and clear enough.
> It feels like another step of simplification is needed.

> What about to get rid of the maxfilesize option?
> Then each chunk can be recorded to a separate file \
> IncrementalHisto.dump.<chunk_number>. A couple of questions to clarify:
      > - Do want all chunks or just the latest chunk to be saved?

I think usually it is not required to save all chunks. The chunk is incremental, so \
the new one contains all info that old ones have.  But having old chunks may help \
user to know how object is distributed , because one chunk is a fixed proportion of \
heap, so the different between to a chunk and it's predecessor can tell the object \
                distribution of the  newly scanned portion of heap.
        The question is do you think these info is necessary? If not, I agree we can \
get rid of the maxfilesize.

      > - If we save all chunks then what is the point to have the full dump recorded \
as well?

         IMO, the incremental histo solves two problems:  <1> The jmap histo may \
stuck if heap is large, so it is useful if we can get intermediate result. <2> \
                incremental info may help user know object distribution of some \
                portion of the heap.
         And I agree that if full dump is successfully gotten, the chunks became less \
useful.



> The advantages of this approach is that there is no need to describe:
      > - relationship between chunksize and maxfilesize
      > - recording behavior for multiple chunks in the incremental file
      > - what chunks have been recorded into the incremental

I agree that maxfilesize may not be useful because the histo data of chunks are \
usually not large.  And it sounds good to me that we save chunk data in seperate \
files named IncrementalHisto.dump.<chunk_number>. So the problem is how much chunks \
do you think we need to save? I think the latest chunk is a must, and maybe the \
previous 3-5 ones?


> But again, this still needs to be clearly specified.
> It would be nice to reach a consensus on a design first.

Totally agree :)

> Thanks,
> Serguei

Again, Thanks for your comments

BRs,
Lin



Thanks,

Lin

________________________________________

From: serguei.spitsyn@oracle.com<mailto:serguei.spitsyn@oracle.com> \
<serguei.spitsyn@oracle.com><mailto:serguei.spitsyn@oracle.com>

Sent: Saturday, May 11, 2019 2:17:41 AM

To: 臧琳; Hohensee, Paul; JC Beyler

Cc: serviceability-dev@openjdk.java.net<mailto:serviceability-dev@openjdk.java.net>

Subject: Re: [RFR]8215623: Add incremental dump for jmap histo



Dear Lin,



Sorry for the late reply.

I've edited the CSR a little bit to fix some incorrect spots.

Now, a couple of spots are not clear to me.





 > - incremental[:<file_name>], enable the incremental dump of heap, dumped

 >   data will be saved to, by default it is "IncrementalHisto.dump"





  Q1: Should the <file_name> be full path or short name?

      Is there any default path? What is the path of the

"IncrementalHisto.dump" file?



 > - chunksize=<N>, size of objects (in KB) will be dumped in one chunk.



  Q2: Should it be chunk of dump, not chunk of objects?



 > - maxfilesize=<N>, size of the incremental data dump file (in KB),

when data size

 >   is larger than maxfilesize, the file is erased and latest data will

be written.



  Q3: What is a relation and limitations between chunksize and maxfilesize?

      Should the maxfilesize be multiple of the chunksize?



  Q4: The sentence "the file is erased and latest data will be written"

is not clear enough.

      Why the whole file needs to be erased

      Should the incremental file behave like a cyclic buffer?

      If so, then only next chunk needs to be erased.

      Then the chunks need to be numbered in order, so the earliest one

can be found.

      (I do not want you to accept my suggestions right away. It is just

a discussion point.

       You need to prove that your approach is good and clean enough.)



If we resolve the questions (or get into agreement) then I'll update the

CSR as needed.



Thanks,

Serguei





On 5/5/19 00:34, 臧琳 wrote:

Dear All,

      I have updated the CSR at https://bugs.openjdk.java.net/browse/JDK-8222319

      May I ask your help to review it?

      When it is finalized, I will refine the webrev.



BRs,

Lin



Dear Serguei,

          Thanks a lot for your reviewing.







   System.err.println("      incremental dump support:");

+        System.err.println("        chunkcount=<N>    object number counted (in \
Kilo) to trigger incremental dump");

+        System.err.println("        maxfilesize=<N>   size limit of incremental dump \
file (in KB)");





 From this description is not clear at all what does the chunkcount mean.

Is it to define how many heap objects are dumped in one chunk?

If so, would it better to name it chunksize instead where chunksize is measured in \
heap objects?

Then would it better to use the same units to define the maxfilesize as well?

(I'm not insisting on this, just asking.)

The original meaning of  "chunkcount"  is how many objects are dumped in one chunk, \
and the "maxfilesize" is the limited size of the dump file.

For example, "chunkcount=1, maxfilesize=10" means that intermediated data will be \
written to the dump file for every 1000 objects, and

when the dump file is larger than 10k,erase the file and rewrite it with the latest \
dumped data.



The reason I didn't use object count to control the dump file size is that there can \
be humongous object, which may cause the file too large.

Do you think use object size instead of chunkcount is a good option? So the two \
options can be with same units.

BRs,

Lin


[Attachment #3 (text/html)]

<html xmlns:v="urn:schemas-microsoft-com:vml" \
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:宋体;
	panose-1:2 1 6 0 3 1 1 1 1 1;}
@font-face
	{font-family:"Cambria Math";
	panose-1:2 4 5 3 5 4 6 3 2 4;}
@font-face
	{font-family:等线;
	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:微软雅黑;
	panose-1:2 11 5 3 2 2 4 2 2 4;}
@font-face
	{font-family:"\@宋体";
	panose-1:2 1 6 0 3 1 1 1 1 1;}
@font-face
	{font-family:"\@微软雅黑";}
@font-face
	{font-family:"\@等线";
	panose-1:2 1 6 0 3 1 1 1 1 1;}
@font-face
	{font-family:Consolas;
	panose-1:2 11 6 9 2 2 4 3 2 4;}
/* Style Definitions */
p.MsoNormal, li.MsoNormal, div.MsoNormal
	{margin:0cm;
	margin-bottom:.0001pt;
	font-size:12.0pt;
	font-family:"Times New Roman",serif;
	color:black;}
a:link, span.MsoHyperlink
	{mso-style-priority:99;
	color:blue;
	text-decoration:underline;}
a:visited, span.MsoHyperlinkFollowed
	{mso-style-priority:99;
	color:purple;
	text-decoration:underline;}
code
	{mso-style-priority:99;
	font-family:"Courier New";}
pre
	{mso-style-priority:99;
	mso-style-link:"HTML 预设 式 字符";
	margin:0cm;
	margin-bottom:.0001pt;
	font-size:10.0pt;
	font-family:"Courier New";
	color:black;}
p.msonormal0, li.msonormal0, div.msonormal0
	{mso-style-name:msonormal;
	mso-margin-top-alt:auto;
	margin-right:0cm;
	mso-margin-bottom-alt:auto;
	margin-left:0cm;
	font-size:12.0pt;
	font-family:"Times New Roman",serif;
	color:black;}
span.HTML
	{mso-style-name:"HTML 预设 式 字符";
	mso-style-priority:99;
	mso-style-link:"HTML 预设 式";
	font-family:Consolas;
	color:black;}
span.apple-tab-span
	{mso-style-name:apple-tab-span;}
span.EmailStyle22
	{mso-style-type:personal-reply;
	font-family:等线;
	color:#1F497D;}
.MsoChpDefault
	{mso-style-type:export-only;
	font-size:10.0pt;}
@page WordSection1
	{size:612.0pt 792.0pt;
	margin:72.0pt 90.0pt 72.0pt 90.0pt;}
div.WordSection1
	{page:WordSection1;}
--></style><!--[if gte mso 9]><xml>
<o:shapedefaults v:ext="edit" spidmax="1026" />
</xml><![endif]--><!--[if gte mso 9]><xml>
<o:shapelayout v:ext="edit">
<o:idmap v:ext="edit" data="1" />
</o:shapelayout></xml><![endif]-->
</head>
<body bgcolor="white" lang="EN-US" link="blue" vlink="purple">
<div class="WordSection1">
<p class="MsoNormal"><span \
style="font-size:11.0pt;font-family:等线;color:#1F497D">Dear Serguei, \
<o:p></o:p></span></p> <p class="MsoNormal" style="text-indent:12.0pt"><span \
style="font-size:11.0pt;font-family:等线;color:#1F497D">The main reason of using \
separate file for incremental dump is due to the consideration of parallel \
incremental dump implementation, so that every heap-iteration  thread could dump its \
own data in&nbsp; separate file, to avoid using file lock.<o:p></o:p></span></p> <p \
class="MsoNormal" style="text-indent:12.0pt"><span \
style="font-size:11.0pt;font-family:等线;color:#1F497D">I originally made the \
design of whole file-support, incremental, parallel dump together, and then divided \
them into three sub-tasks, trying to make each  task easy to discuss and review. \
&nbsp;In that design, the file=&lt;file&gt; option is for final results, \
"incrementalHisto.dump.&lt;thread_id&gt;" is for thread "local" incremental dumped \
data.<o:p></o:p></span></p> <p class="MsoNormal" style="text-indent:12.0pt"><span \
style="font-size:11.0pt;font-family:等线;color:#1F497D">For incremental dump, I \
agree it is better if we can get rid of incremental-file options and using \
file=&lt;file&gt;, but I think we need to discuss how to  make it adaptable for \
parallel dump. What do you think?<o:p></o:p></span></p> <p class="MsoNormal"><span \
style="font-size:11.0pt;font-family:等线;color:#1F497D"><o:p>&nbsp;</o:p></span></p>
 <p class="MsoNormal"><span \
style="font-size:11.0pt;font-family:等线;color:#1F497D">Thanks,<o:p></o:p></span></p>
 <p class="MsoNormal"><span \
style="font-size:11.0pt;font-family:等线;color:#1F497D">Lin<o:p></o:p></span></p> \
<p class="MsoNormal" style="text-indent:12.0pt"><span \
style="font-size:11.0pt;font-family:等线;color:#1F497D"><o:p>&nbsp;</o:p></span></p>
 <p class="MsoNormal"><span \
style="font-size:11.0pt;font-family:等线;color:#1F497D"><o:p>&nbsp;</o:p></span></p>
 <div style="border:none;border-left:solid blue 1.5pt;padding:0cm 0cm 0cm 4.0pt">
<div>
<div style="border:none;border-top:solid #E1E1E1 1.0pt;padding:3.0pt 0cm 0cm 0cm">
<p class="MsoNormal"><b><span \
style="font-size:11.0pt;font-family:&quot;Calibri&quot;,sans-serif;color:windowtext">From:</span></b><span \
style="font-size:11.0pt;font-family:&quot;Calibri&quot;,sans-serif;color:windowtext"> \
serguei.spitsyn@oracle.com &lt;serguei.spitsyn@oracle.com&gt; <br>
<b>Sent:</b> Saturday, May 18, 2019 2:42 AM<br>
<b>To:</b> </span><span lang="ZH-CN" \
style="font-size:11.0pt;font-family:&quot;微软雅黑&quot;,sans-serif;color:windowtext">臧琳</span><span \
style="font-size:11.0pt;font-family:&quot;Calibri&quot;,sans-serif;color:windowtext"> \
&lt;zanglin5@jd.com&gt;<br> <b>Cc:</b> Hohensee, Paul &lt;hohensee@amazon.com&gt;; JC \
Beyler &lt;jcbeyler@google.com&gt;; serviceability-dev@openjdk.java.net<br> \
<b>Subject:</b> Re: [RFR]8215623: Add incremental dump for jmap \
histo<o:p></o:p></span></p> </div>
</div>
<p class="MsoNormal"><o:p>&nbsp;</o:p></p>
<div>
<p class="MsoNormal">Dear Lin,<br>
<br>
Before I go to the details below could you, please, explain why do we need a separate \
file for incremental dump.<br> Should we just record the full dump into \
file=&lt;file&gt; incrementally?<br> Recording into the full dump file just has to be \
always incremental.&nbsp; <br> It can be done by chunks if necessary, so I'm open to \
consider introducing new chunksize option.<br> <br>
Do I miss anything here?<br>
<br>
Thanks,<br>
Serguei<br>
<br>
On 5/17/19 03:18, <span lang="ZH-CN" style="font-family:宋体">臧琳</span> \
wrote:<o:p></o:p></p> </div>
<blockquote style="margin-top:5.0pt;margin-bottom:5.0pt">
<p class="MsoNormal">Dear Serguei,&nbsp;<o:p></o:p></p>
<div>
<p class="MsoNormal"><br>
<br>
<o:p></o:p></p>
<blockquote style="margin-top:5.0pt;margin-bottom:5.0pt">
<div>
<p class="MsoNormal"><span lang="ZH-CN" style="font-family:宋体">在</span> \
2019<span lang="ZH-CN" style="font-family:宋体">年</span>5<span lang="ZH-CN" \
style="font-family:宋体">月</span>16<span lang="ZH-CN" \
style="font-family:宋体">日,下午</span>1:39<span lang="ZH-CN" \
style="font-family:宋体">,</span><a \
href="mailto:serguei.spitsyn@oracle.com">serguei.spitsyn@oracle.com</a> <span \
lang="ZH-CN" style="font-family:宋体">写道:</span><o:p></o:p></p> </div>
<p class="MsoNormal"><o:p>&nbsp;</o:p></p>
<div>
<div>
<div>
<p class="MsoNormal">On 5/13/19 23:46, <span lang="ZH-CN" style="font-family:宋体">
臧琳</span> wrote:<o:p></o:p></p>
</div>
<blockquote style="margin-top:5.0pt;margin-bottom:5.0pt">
<pre>Dear Serguei,<o:p></o:p></pre>
<pre>&nbsp;&nbsp;&nbsp;&nbsp; Thanks for your comments.<o:p></o:p></pre>
<pre><o:p>&nbsp;</o:p></pre>
<pre> &gt; &gt; - incremental[:&lt;file_name&gt;], enable the incremental dump of \
heap, dumped<o:p></o:p></pre> <pre> &gt; &gt;&nbsp;&nbsp; data will be saved to, by \
default it is &quot;IncrementalHisto.dump&quot;<o:p></o:p></pre> <pre> \
&gt;<o:p></o:p></pre> <pre> &gt;&nbsp; Q1: Should the &lt;file_name&gt; be full path \
or short name?<o:p></o:p></pre> <pre> &gt;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp; Is there any \
default path? What is the path of the<o:p></o:p></pre> <pre> &gt; \
&quot;IncrementalHisto.dump&quot; file?<o:p></o:p></pre> <pre><o:p>&nbsp;</o:p></pre>
<pre>The original design doesn't have the &lt;file_name&gt; option so the file is \
hardcoded named &quot;IncrementalHisto.dump&quot; and save to the same path as \
&quot;file=&quot; specified. Or print to whatever output stream is if \
&quot;file=&quot; is not set.<o:p></o:p></pre> </blockquote>
<p class="MsoNormal"><br>
&gt; The file option is described as:<code><span style="font-size:10.0pt"> \
file=&lt;file&gt; dump data to &lt;file&gt;</span></code><span \
style="font-size:10.0pt;font-family:&quot;Courier New&quot;"><br> </span>&gt; It does \
not tell anything about the path.<o:p></o:p></p> </div>
</div>
</blockquote>
<div>
<p class="MsoNormal"><o:p>&nbsp;</o:p></p>
</div>
<p class="MsoNormal">Yes, do you agree that we add a comment in the help info \
like:<o:p></o:p></p> </div>
<div>
<p class="MsoNormal">file=&lt;file&gt; dump data to &lt;file&gt;, file can be \
specified with full path.<o:p></o:p></p> </div>
<div>
<blockquote style="margin-top:5.0pt;margin-bottom:5.0pt">
<div>
<div>
<blockquote style="margin-top:5.0pt;margin-bottom:5.0pt">
<pre>With the new design, I suggest firstly parse &lt;file_name&gt;, if the value \
contains folder path, use the specified path, if not, use same path as \
&quot;file=&quot; value,&nbsp; and if &quot;file=&quot; is not set, use output \
stream. (The reason I prefer to use same path as &quot;file=&quot; is I assume that \
users prefer to save all data file under the same folder.)<o:p></o:p></pre> \
</blockquote> <p class="MsoNormal"><br>
&gt; It needs to be clearly specified.<br>
&gt; What statements do you suggest?<br>
&gt;<br>
&gt; One idea of simplification is to get rid of the default &lt;file_name&gt;<br>
&gt; and to require it to be always specified (non-optional).<br>
&gt;<br>
&gt; Then we could replace this:<o:p></o:p></p>
<pre><code>&gt;&nbsp;&nbsp; file=&lt;file&gt; dump data to &lt;file&gt; \
<o:p></o:p></code></pre> <pre><code>&gt;&nbsp; &nbsp;incremental dump support: \
<o:p></o:p></code></pre> <pre><code>&gt;&nbsp;&nbsp;&nbsp;&nbsp; \
incremental[:&lt;file_name&gt;]&nbsp; enable incremental dump, data will be \
dumped<o:p></o:p></code></pre> \
<pre><code>&gt;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp \
;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp; \
to &lt;file_name&gt; (default is \
&quot;IncrementalHisto.dump&quot;)</code><o:p></o:p></pre> <p \
class="MsoNormal">&gt;&nbsp;<br> &gt; with this:<code><span style="font-size:10.0pt"> \
</span></code><o:p></o:p></p> <pre><code>&gt;&nbsp;&nbsp; file=&lt;file&gt; dump data \
to &lt;file&gt; <o:p></o:p></code></pre> <pre><code>&gt;&nbsp;&nbsp; \
incremental=&lt;inc_file&gt;&nbsp; dump incremental data to \
&lt;inc_file&gt;</code><o:p></o:p></pre> </div>
</div>
</blockquote>
<div>
<p class="MsoNormal">I think having a default IncrementalHisto.dump file saved at the \
same path of the &lt;file&gt; is a way to make incremental easy to \
use.&nbsp;<o:p></o:p></p> </div>
<div>
<p class="MsoNormal">IMHO, when user use jmap -histo with &quot;file=&lt;file&gt;", \
and want to enable inremental histo, the easiest way is just use \
&quot;-incremental&quot; flag and all data files will be &nbsp;saved under the same \
folder of &lt;file&gt;. They don't have to consider &nbsp;the  specific filename for \
incremental additionally. This is the reason I set default value of \
IncrementalHisto.dump.&nbsp;<o:p></o:p></p> </div>
<div>
<p class="MsoNormal"><o:p>&nbsp;</o:p></p>
</div>
<div>
<p class="MsoNormal">But I also want the user to have freedom to use different \
filename and path for incremental results, so I &nbsp;make it optional for \
incremental file_name.<o:p></o:p></p> </div>
<div>
<p class="MsoNormal"><o:p>&nbsp;</o:p></p>
</div>
<div>
<p class="MsoNormal">If we can make it non-optional, does it mean that user may have \
following command:<o:p></o:p></p> </div>
<div>
<p class="MsoNormal">&nbsp; &nbsp; &nbsp; &nbsp;Jmap \
-histo,file=&lt;absoult_path/a/b/c/histo.dump&gt;,incremental:&lt;absoult_path/a/b/c/incrementalHisto.dump&gt; \
pid&nbsp;<o:p></o:p></p> </div>
<div>
<p class="MsoNormal">It seems a little bit complicated to me, what do you \
think?&nbsp;<o:p></o:p></p> </div>
<p class="MsoNormal"><br>
<br>
<o:p></o:p></p>
<blockquote style="margin-top:5.0pt;margin-bottom:5.0pt">
<div>
<div>
<p class="MsoNormal"><br>
<br>
<o:p></o:p></p>
<blockquote style="margin-top:5.0pt;margin-bottom:5.0pt">
<pre> &gt; &gt; - chunksize=&lt;N&gt;, size of objects (in KB) will be dumped in one \
chunk.<o:p></o:p></pre> <pre> &gt;<o:p></o:p></pre>
<pre> &gt; Q2: Should it be chunk of dump, not chunk of objects?<o:p></o:p></pre>
<pre><o:p>&nbsp;</o:p></pre>
<pre>The purpose of &quot;chunksize&quot; is to decide how many objects' info are \
dumped at once. for example use &quot;chunksize=1&quot; on a &quot;Xmx1m&quot;, there \
will be at max 1MB/1KB = 1000 chunks, which indicates that there will be 1000 times \
of file writing when do &quot;jmap -histo&quot;.<o:p></o:p></pre> </blockquote>
<p class="MsoNormal"><br>
&gt; I hardly understand the point to know max of objects that can be dumped at \
once.<br> &gt; It is more important to know how much memory in the file it is going \
to take.<br> &gt; How much of dump memory will take one object?<br>
&gt; Does it vary (does it depend on object types)?<o:p></o:p></p>
</div>
</div>
</blockquote>
<div>
<p class="MsoNormal"><o:p>&nbsp;</o:p></p>
</div>
<p class="MsoNormal">Yes, the dump memory for one object varies from size and \
types.<o:p></o:p></p> </div>
<div>
<p class="MsoNormal">The option"chunksize" is for user to control the proportion of \
heap that the incremental dump can process at a time. &nbsp;IMO the use scenario is \
as following:<o:p></o:p></p> </div>
<div>
<p class="MsoNormal"><o:p>&nbsp;</o:p></p>
</div>
<div>
<p class="MsoNormal">&nbsp; &nbsp;when the JVM have an 180GB max heap size, and jmap \
histo used with chunksize=1g, it means the incremental dump happens when every 1GB \
heap is scaned, so it does't has too much incremenal dump, because the incremental \
dump takes time  and may cause jmap -histo work slower.<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal"><o:p>&nbsp;</o:p></p>
</div>
<div>
<p class="MsoNormal">PS, I think we should support "g","m" and "k" instead of using \
"KB" , do you agree?<br> &nbsp;<br>
<br>
<o:p></o:p></p>
<blockquote style="margin-top:5.0pt;margin-bottom:5.0pt">
<div>
<div>
<blockquote style="margin-top:5.0pt;margin-bottom:5.0pt">
<pre> &gt; &gt; - maxfilesize=&lt;N&gt;, size of the incremental data dump file (in \
KB), when data size<o:p></o:p></pre> <pre> &gt; &gt;&nbsp;&nbsp; is larger than \
maxfilesize, the file is erased and latest data will be written.<o:p></o:p></pre> \
<pre><o:p>&nbsp;</o:p></pre> <pre> &gt; Q3: What is a relation and limitations \
between chunksize and maxfilesize?<o:p></o:p></pre> <pre> &gt;&nbsp;&nbsp;&nbsp; \
Should the maxfilesize be multiple of the chunksize?<o:p></o:p></pre> </blockquote>
<p class="MsoNormal" style="margin-bottom:12.0pt"><br>
&gt; The question Q3 above was not unanswered.<br>
&gt; But never mind. Please, see the suggestion below.<o:p></o:p></p>
</div>
</div>
</blockquote>
<div>
<p class="MsoNormal"><o:p>&nbsp;</o:p></p>
</div>
<div>
<p class="MsoNormal">If the chunksize it large, and there are too much objects of \
different classes in heap, the actual filesize can be larger than the \
maxfilesize.&nbsp;<o:p></o:p></p> </div>
<div>
<p class="MsoNormal">But I believe this is raraly happened because the size of one \
class's histo info only takes several bytes in the final result,&nbsp;&nbsp;and from \
the implementation of jmap,&nbsp;<o:p></o:p></p> </div>
<div>
<p class="MsoNormal">it can find out all loaded classes before doing heap iteration, \
so the different of result only happens on the object quantity.<o:p></o:p></p> </div>
<blockquote style="margin-top:5.0pt;margin-bottom:5.0pt">
<div>
<div>
<blockquote style="margin-top:5.0pt;margin-bottom:5.0pt">
<pre> &gt; Q4: The sentence &quot;the file is erased and latest data will be \
written&quot;<o:p></o:p></pre> <pre>is not clear enough.<o:p></o:p></pre>
<pre> &gt;&nbsp;&nbsp;&nbsp; Why the whole file needs to be erased<o:p></o:p></pre>
<pre> &gt;&nbsp;&nbsp;&nbsp; Should the incremental file behave like a cyclic \
buffer?<o:p></o:p></pre> <pre> &gt;&nbsp;&nbsp;&nbsp; If so, then only next chunk \
needs to be erased.<o:p></o:p></pre> <pre> &gt;&nbsp;&nbsp;&nbsp; Then the chunks \
need to be numbered in order, so the earliest one can be found.<o:p></o:p></pre> \
<pre><o:p>&nbsp;</o:p></pre> <pre>The &quot;maxfilesize&quot; controls the file size \
not to be too large, so when the dumped data is larger than &quot;maxfilesize&quot;, \
the file is erased and latest data are written.The reason I erase whole file is that \
chunk data is accumulative, so the latest data includes the previous statistical \
ones. And this way may make the file easy to read.<o:p></o:p></pre> \
<pre><o:p>&nbsp;</o:p></pre> <pre>I agree that we can add ordered number in chunks, I \
think it more or less help user to get to know how object distributed in \
heap.<o:p></o:p></pre> <pre><o:p>&nbsp;</o:p></pre>
<pre>I think maybe it is reasonable to have the incremental file behave like gclog, \
when maxfilesize is reached, the file is renamed with numbered suffix, and new file \
is created to use. so there can be IncrementalHisto.dump.0 and \
IncrementalHisto.dump.1 etc for large heap.<o:p></o:p></pre> \
<pre><o:p>&nbsp;</o:p></pre> <pre>what do you think?<o:p></o:p></pre>
</blockquote>
<p class="MsoNormal"><br>
&gt; I think, it is not a bad idea.<br>
&gt; In general, new incremental feature design does not look simple and clear \
enough.<br> &gt; It feels like another step of simplification is needed.<br>
<br>
&gt; What about to get rid of the maxfilesize option?<br>
&gt; Then each chunk can be recorded to a separate file \
IncrementalHisto.dump.&lt;chunk_number&gt;.<br> &gt; A couple of questions to \
clarify:<br> &nbsp; &nbsp; &nbsp; &gt; - Do want all chunks or just the latest chunk \
to be saved?<o:p></o:p></p> </div>
</div>
</blockquote>
<p class="MsoNormal"><o:p>&nbsp;</o:p></p>
<div>
<p class="MsoNormal">I think usually it is not required to save all chunks. The chunk \
is incremental, so the new one contains all info that old ones have.<o:p></o:p></p> \
</div> <div>
<p class="MsoNormal">&nbsp; &nbsp; &nbsp; &nbsp; But having old chunks may help user \
to know how object is distributed , because one chunk is a fixed proportion of heap, \
so the different between to a chunk and it's predecessor can tell the object \
distribution of the &nbsp;newly scanned  portion of heap.<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal">&nbsp; &nbsp; &nbsp; &nbsp; The question is do you think these \
info is necessary? If not, I agree we can get rid of the \
maxfilesize.&nbsp;<o:p></o:p></p> </div>
<div>
<p class="MsoNormal"><o:p>&nbsp;</o:p></p>
</div>
<blockquote style="margin-top:5.0pt;margin-bottom:5.0pt">
<div>
<div>
<p class="MsoNormal">&nbsp; &nbsp; &nbsp; &gt; - If we save all chunks then what is \
the point to have the full dump recorded as well?<o:p></o:p></p> </div>
</div>
</blockquote>
<div>
<p class="MsoNormal"><o:p>&nbsp;</o:p></p>
</div>
<div>
<p class="MsoNormal">&nbsp; &nbsp; &nbsp; &nbsp; &nbsp;IMO, the incremental histo \
solves two problems: &nbsp;&lt;1&gt; The jmap histo may stuck if heap is large, so it \
is useful if we can get intermediate result. &lt;2&gt; incremental info may help user \
know object distribution of some portion of the  heap.<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal">&nbsp; &nbsp; &nbsp; &nbsp; &nbsp;And I agree that if full dump \
is successfully gotten, the chunks became less useful. &nbsp;<o:p></o:p></p> </div>
<p class="MsoNormal"><br>
<br>
<o:p></o:p></p>
<blockquote style="margin-top:5.0pt;margin-bottom:5.0pt">
<div>
<div>
<p class="MsoNormal" style="margin-bottom:12.0pt"><br>
&gt; The advantages of this approach is that there is no need to describe:<br>
&nbsp; &nbsp; &nbsp; &gt; - relationship between chunksize and maxfilesize<br>
&nbsp; &nbsp; &nbsp; &gt; - recording behavior for multiple chunks in the incremental \
file<br> &nbsp; &nbsp; &nbsp; &gt; - what chunks have been recorded into the \
incremental<o:p></o:p></p> </div>
</div>
</blockquote>
<div>
<p class="MsoNormal"><o:p>&nbsp;</o:p></p>
</div>
<div>
<p class="MsoNormal">I agree that maxfilesize may not be useful because the histo \
data of chunks are usually not large. &nbsp;And it sounds good to me that we save \
chunk data in seperate files named \
IncrementalHisto.dump.&lt;chunk_number&gt;.<o:p></o:p></p> </div>
<div>
<p class="MsoNormal">So the problem is how much chunks do you think we need to save? \
I think the latest chunk is a must, and maybe the previous 3-5 ones?<o:p></o:p></p> \
</div> <p class="MsoNormal"><br>
<br>
<o:p></o:p></p>
<blockquote style="margin-top:5.0pt;margin-bottom:5.0pt">
<div>
<div>
<p class="MsoNormal">&gt; But again, this still needs to be clearly specified.<br>
&gt; It would be nice to reach a consensus on a design first.<o:p></o:p></p>
</div>
</div>
</blockquote>
<div>
<p class="MsoNormal"><o:p>&nbsp;</o:p></p>
</div>
<p class="MsoNormal">Totally agree :)&nbsp;<o:p></o:p></p>
</div>
<div>
<blockquote style="margin-top:5.0pt;margin-bottom:5.0pt">
<div>
<div>
<p class="MsoNormal" style="margin-bottom:12.0pt"><br>
&gt; Thanks,<br>
&gt; Serguei<o:p></o:p></p>
</div>
</div>
</blockquote>
<div>
<p class="MsoNormal"><o:p>&nbsp;</o:p></p>
</div>
<div>
<p class="MsoNormal">Again, Thanks for your comments<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal"><o:p>&nbsp;</o:p></p>
</div>
<div>
<p class="MsoNormal">BRs,<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal">Lin<o:p></o:p></p>
</div>
<p class="MsoNormal"><br>
<br>
<o:p></o:p></p>
<blockquote style="margin-top:5.0pt;margin-bottom:5.0pt">
<div>
<div>
<blockquote style="margin-top:5.0pt;margin-bottom:5.0pt">
<pre>Thanks,<o:p></o:p></pre>
<pre>Lin<o:p></o:p></pre>
<pre>________________________________________<o:p></o:p></pre>
<pre>From: <a href="mailto:serguei.spitsyn@oracle.com">serguei.spitsyn@oracle.com</a> \
<a href="mailto:serguei.spitsyn@oracle.com">&lt;serguei.spitsyn@oracle.com&gt;</a><o:p></o:p></pre>
 <pre>Sent: Saturday, May 11, 2019 2:17:41 AM<o:p></o:p></pre>
<pre>To: <span lang="ZH-CN" style="font-family:等线">臧琳</span>; Hohensee, Paul; \
JC Beyler<o:p></o:p></pre> <pre>Cc: <a \
href="mailto:serviceability-dev@openjdk.java.net">serviceability-dev@openjdk.java.net</a><o:p></o:p></pre>
 <pre>Subject: Re: [RFR]8215623: Add incremental dump for jmap histo<o:p></o:p></pre>
<pre><o:p>&nbsp;</o:p></pre>
<pre>Dear Lin,<o:p></o:p></pre>
<pre><o:p>&nbsp;</o:p></pre>
<pre>Sorry for the late reply.<o:p></o:p></pre>
<pre>I've edited the CSR a little bit to fix some incorrect spots.<o:p></o:p></pre>
<pre>Now, a couple of spots are not clear to me.<o:p></o:p></pre>
<pre><o:p>&nbsp;</o:p></pre>
<pre><o:p>&nbsp;</o:p></pre>
<pre> &gt; - incremental[:&lt;file_name&gt;], enable the incremental dump of heap, \
dumped<o:p></o:p></pre> <pre> &gt;&nbsp;&nbsp; data will be saved to, by default it \
is &quot;IncrementalHisto.dump&quot;<o:p></o:p></pre> <pre><o:p>&nbsp;</o:p></pre>
<pre><o:p>&nbsp;</o:p></pre>
<pre>&nbsp; Q1: Should the &lt;file_name&gt; be full path or short \
name?<o:p></o:p></pre> <pre>&nbsp;&nbsp;&nbsp;&nbsp;&nbsp; Is there any default path? \
What is the path of the<o:p></o:p></pre> <pre>&quot;IncrementalHisto.dump&quot; \
file?<o:p></o:p></pre> <pre><o:p>&nbsp;</o:p></pre>
<pre> &gt; - chunksize=&lt;N&gt;, size of objects (in KB) will be dumped in one \
chunk.<o:p></o:p></pre> <pre><o:p>&nbsp;</o:p></pre>
<pre>&nbsp; Q2: Should it be chunk of dump, not chunk of objects?<o:p></o:p></pre>
<pre><o:p>&nbsp;</o:p></pre>
<pre> &gt; - maxfilesize=&lt;N&gt;, size of the incremental data dump file (in \
KB),<o:p></o:p></pre> <pre>when data size<o:p></o:p></pre>
<pre> &gt;&nbsp;&nbsp; is larger than maxfilesize, the file is erased and latest data \
will<o:p></o:p></pre> <pre>be written.<o:p></o:p></pre>
<pre><o:p>&nbsp;</o:p></pre>
<pre>&nbsp; Q3: What is a relation and limitations between chunksize and \
maxfilesize?<o:p></o:p></pre> <pre>&nbsp;&nbsp;&nbsp;&nbsp;&nbsp; Should the \
maxfilesize be multiple of the chunksize?<o:p></o:p></pre> \
<pre><o:p>&nbsp;</o:p></pre> <pre>&nbsp; Q4: The sentence &quot;the file is erased \
and latest data will be written&quot;<o:p></o:p></pre> <pre>is not clear \
enough.<o:p></o:p></pre> <pre>&nbsp;&nbsp;&nbsp;&nbsp;&nbsp; Why the whole file needs \
to be erased<o:p></o:p></pre> <pre>&nbsp;&nbsp;&nbsp;&nbsp;&nbsp; Should the \
incremental file behave like a cyclic buffer?<o:p></o:p></pre> \
<pre>&nbsp;&nbsp;&nbsp;&nbsp;&nbsp; If so, then only next chunk needs to be \
erased.<o:p></o:p></pre> <pre>&nbsp;&nbsp;&nbsp;&nbsp;&nbsp; Then the chunks need to \
be numbered in order, so the earliest one<o:p></o:p></pre> <pre>can be \
found.<o:p></o:p></pre> <pre>&nbsp;&nbsp;&nbsp;&nbsp;&nbsp; (I do not want you to \
accept my suggestions right away. It is just<o:p></o:p></pre> <pre>a discussion \
point.<o:p></o:p></pre> <pre>&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp; You need to prove \
that your approach is good and clean enough.)<o:p></o:p></pre> \
<pre><o:p>&nbsp;</o:p></pre> <pre>If we resolve the questions (or get into agreement) \
then I'll update the<o:p></o:p></pre> <pre>CSR as needed.<o:p></o:p></pre>
<pre><o:p>&nbsp;</o:p></pre>
<pre>Thanks,<o:p></o:p></pre>
<pre>Serguei<o:p></o:p></pre>
<pre><o:p>&nbsp;</o:p></pre>
<pre><o:p>&nbsp;</o:p></pre>
<pre>On 5/5/19 00:34, <span lang="ZH-CN" style="font-family:等线">臧琳</span> \
wrote:<o:p></o:p></pre> <blockquote style="margin-top:5.0pt;margin-bottom:5.0pt">
<pre>Dear All,<o:p></o:p></pre>
<pre>&nbsp;&nbsp;&nbsp;&nbsp;&nbsp; I have updated the CSR at <a \
href="https://bugs.openjdk.java.net/browse/JDK-8222319">https://bugs.openjdk.java.net/browse/JDK-8222319</a><o:p></o:p></pre>
 <pre>&nbsp;&nbsp;&nbsp;&nbsp;&nbsp; May I ask your help to review \
it?<o:p></o:p></pre> <pre>&nbsp;&nbsp;&nbsp;&nbsp;&nbsp; When it is finalized, I will \
refine the webrev.<o:p></o:p></pre> <pre><o:p>&nbsp;</o:p></pre>
<pre>BRs,<o:p></o:p></pre>
<pre>Lin<o:p></o:p></pre>
<pre><o:p>&nbsp;</o:p></pre>
<blockquote style="margin-top:5.0pt;margin-bottom:5.0pt">
<pre>Dear Serguei<span lang="ZH-CN" \
style="font-family:等线">,</span><o:p></o:p></pre> \
<pre>&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp; Thanks a lot for your \
reviewing.<o:p></o:p></pre> <pre><o:p>&nbsp;</o:p></pre>
<pre><o:p>&nbsp;</o:p></pre>
<pre><o:p>&nbsp;</o:p></pre>
<blockquote style="margin-top:5.0pt;margin-bottom:5.0pt">
<pre>&nbsp;&nbsp; System.err.println(&quot;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp; incremental \
dump support:&quot;);<o:p></o:p></pre> \
<pre>&#43;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp; \
System.err.println(&quot;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp; \
chunkcount=&lt;N&gt;&nbsp;&nbsp;&nbsp; object number counted (in Kilo) to trigger \
incremental dump&quot;);<o:p></o:p></pre> \
<pre>&#43;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp; \
System.err.println(&quot;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp; \
maxfilesize=&lt;N&gt;&nbsp;&nbsp; size limit of incremental dump file (in \
KB)&quot;);<o:p></o:p></pre> <pre><o:p>&nbsp;</o:p></pre>
<pre><o:p>&nbsp;</o:p></pre>
<pre> From this description is not clear at all what does the chunkcount \
mean.<o:p></o:p></pre> <pre>Is it to define how many heap objects are dumped in one \
chunk?<o:p></o:p></pre> <pre>If so, would it better to name it chunksize instead \
where chunksize is measured in heap objects?<o:p></o:p></pre> <pre>Then would it \
better to use the same units to define the maxfilesize as well?<o:p></o:p></pre> \
<pre>(I'm not insisting on this, just asking.)<o:p></o:p></pre> </blockquote>
<pre>The original meaning of&nbsp; "chunkcount&quot;&nbsp; is how many objects are \
dumped in one chunk, and the "maxfilesize" is the limited size of the dump \
file.<o:p></o:p></pre> <pre>For example, "chunkcount=1, maxfilesize=10" means that \
intermediated data will be written to the dump file for every 1000 objects, \
and<o:p></o:p></pre> <pre>when the dump file is larger than 10k<span lang="ZH-CN" \
style="font-family:等线">,</span>erase the file and rewrite it with the latest \
dumped data.<o:p></o:p></pre> <pre><o:p>&nbsp;</o:p></pre>
<pre>The reason I didn't use object count to control the dump file size is that there \
can be humongous object, which may cause the file too large.<o:p></o:p></pre> <pre>Do \
you think use object size instead of chunkcount is a good option? So the two options \
can be with same units.<o:p></o:p></pre> <pre>BRs,<o:p></o:p></pre>
<pre>Lin<o:p></o:p></pre>
</blockquote>
</blockquote>
</blockquote>
<p class="MsoNormal"><o:p>&nbsp;</o:p></p>
</div>
</div>
</blockquote>
</div>
<p class="MsoNormal"><o:p>&nbsp;</o:p></p>
</blockquote>
<p class="MsoNormal"><o:p>&nbsp;</o:p></p>
</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