[RFR]8215623: Add incremental dump for jmap histo

Tue Feb 26 02:50:51 UTC 2019

Dear All, 
         I have revised the webrev at http://cr.openjdk.java.net/~xiaofeya/8215623/webrev.02/. 
         May I ask your help for reviewing? Thanks 


BRs,
Lin
> -----Original Message-----
> From: ê°ÁÕ
> Sent: Friday, January 25, 2019 9:01 AM
> To: Hohensee, Paul <hohensee at amazon.com>; serviceability-
> dev at openjdk.java.net
> Subject: Re: [RFR]8215623: Add incremental dump for jmap histo
> 
> 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 at 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 at amazon.com>; serviceability-
> dev at openjdk.java.net; ê°ÁÕ <zanglin5 at 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 at amazon.com<mailto:hohensee at amazon.com>>; serviceability-
> dev at openjdk.java.net<mailto:serviceability-dev at 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 at amazon.com<mailto:hohensee at amazon.com>>
> ·¢ËÍÊ±¼ä: 2019Äê1ÔÂ1ÈÕ 4:56
> ÊÕ¼þÈË: ê°ÁÕ; serviceability-dev at openjdk.java.net<mailto:serviceability-
> dev at 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 at openjdk.java.net<mailto:serviceability-dev-
> bounces at openjdk.java.net>> on behalf of ê°ÁÕ
> <zanglin5 at jd.com<mailto:zanglin5 at jd.com>>
> Date: Thursday, December 20, 2018 at 11:13 PM
> To: "serviceability-dev at openjdk.java.net<mailto:serviceability-
> dev at openjdk.java.net>" <serviceability-
> dev at openjdk.java.net<mailto:serviceability-dev at 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
> 
> 
> 
>