[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-01-25 1:01:25
Message-ID: 74eaa1f868b74257beffff465f60edb2 () jd ! com
[Download RAW message or body]

Dear All,
         May I get more review about this webrev?
         Webrev: http://cr.openjdk.java.net/~xiaofeya/8215623/webrev.01/
         Bug: https://bugs.openjdk.java.net/browse/JDK-8215623


Thanks!
Lin
________________________________________
From: ê°ÁÕ
Sent: Wednesday, January 9, 2019 9:46:55 AM
To: Hohensee, Paul; serviceability-dev@openjdk.java.net
Subject: RE: [RFR]8215623: Add incremental dump for jmap histo

Hi Paul and All,
     Thanks a lot for your review and comments,
I have updated the webrev to

Webrev: http://cr.openjdk.java.net/~xiaofeya/8215623/webrev.01/

Bug: https://bugs.openjdk.java.net/browse/JDK-8215623
     Would you like to help review it. Thanks.

BRs
Lin

From: ê°ÁÕ
Sent: Monday, January 7, 2019 7:14 PM
To: Hohensee, Paul <hohensee@amazon.com>; serviceability-dev@openjdk.java.net; ê°ÁÕ \
                <zanglin5@jd.com>
Subject: RE: [RFR]8215623: Add incremental dump for jmap histo

And another way is to add a argument ¡°IncrementalFile=<path>¡±,which specifies the \
location for saving the intermediate data file. And if it is not specified, \
incremental data will dump to ¡°out¡±.

What do you think?

Thanks,
Lin
From: ê°ÁÕ
Sent: Monday, January 7, 2019 7:02 PM
To: Hohensee, Paul <hohensee@amazon.com<mailto:hohensee@amazon.com>>; \
                serviceability-dev@openjdk.java.net<mailto:serviceability-dev@openjdk.java.net>
                
Subject: ´ð¸´: [RFR]8215623: Add incremental dump for jmap histo


Dear Paul,

        Thanks for your review, I agree your suggestion to make incremental general. \
and

        I think the incremental file is better to be different with the file or the \
"out", because

in future the parallel histo will have each thread individually dump their data, and \
I want

it to be lock-free, so each thread need to dump to their own file. The final data

will be sum up and dump to the file that "filename=<file>" specified.  Moreover, the \
incremental

file contains intermediate data, if it is dumped to "<file>", it more or less \
"pollute" the final file (final file that contains lots intermediate data).

       so the logic is that incremental data will be dumped to a file named \
"Histo_Dump_Temp.dump",

if the "filename" is assigned, the "Histo_Dump_Temp.dump" will be generated under the \
same folder,

if "filename" not specified, it will dump to current dir. And if "chunkcount" is 0 or \
max_int, or "maxfilesize" is 0, the incremental dump will be disabled.

________________________________
·¢¼þÈË: Hohensee, Paul <hohensee@amazon.com<mailto:hohensee@amazon.com>>
·¢ËÍʱ¼ä: 2019Äê1Ô 1ÈÕ 4:56
ÊÕ¼þÈË: ê°ÁÕ; serviceability-dev@openjdk.java.net<mailto:serviceability-dev@openjdk.java.net>
 Ö÷Ìâ: Re: [RFR]8215623: Add incremental dump for jmap histo


As for 8215622, update the copyright dates to 2019 please, since this won¡¯t get \
pushed until then.



You might generalize the implementation so that all inspections are done \
incrementally, and parameterize RecordInstanceClosure with the incremental threshold. \
¡°incremental¡± could become ¡°chunkcount=<n>¡±, where <n> defaults to infinity (max \
value of size_t).



I¡¯d not use a default file name when ¡°chunkcount¡± is specified, I¡¯d just write to \
whatever the output stream is. ¡°chunkcount¡± is then independent of ¡°file¡±.



I¡¯d add another histo argument for the maximum file size, call it ¡°maxfilesize¡±, \
and parameterize RecordInstanceClosure with it. Default would be infinity (max value \
of size_t).



If you want to make it easy to use your 8k and 5mb chunkcount and maxfilesize \
combination, you could redefine your original ¡°incremental¡± argument as syntactic \
sugar for ¡°chunkcount=8k,maxfilesize=5m¡±.



INCREMENTAL_THRESHOLD and MAX_INCREMENTAL_FILESIZE become DEFAULT_CHUNKSIZE and \
MAX_FILE_SIZE, are both set to max size_t, and should be defined as ¡°const int¡± \
within RecordInstanceClosure.



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:13 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]8215623: Add incremental dump for jmap histo



Hi All,

       May I ask your help to review this patch for enhance jmap ¨Chisto.

It adds the ¡°incremental¡± arguments that allow jmap ¨Chisto to incrementally save \
the intermediate data into a temp file.

The intermediate data is dumped incrementally and write to a rolling file, which \
limit the size of the temp file to be small.

This is useful for user to get intermediate results when jmap/jvm process is killed \
accidentally. Especially when the heap is large.



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/8215623/webrev.00/

Bug: https://bugs.openjdk.java.net/browse/JDK-8215623



Thanks.



BRs,

Lin


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