Dear All, Here I updated the latest changeset which did the following refine: * fixed the compatibility issues so that old version of jmap can work normally with latest changeset. * revised the code for parsing the jmap histo arguments. * revised the logic for incremental dumping of jmap histo. The main change of this webrev is making all arguments passed to virtual machine as one line. So there is no need to care about the max argument count limitation. And hence resolved the compatibility issues as discussed in http://mail.openjdk.java.net/pipermail/serviceability-dev/2019-March/027338.html
May I ask your help to review? http://cr.openjdk.java.net/~lzang/jmap-8214535/8215623/webrev.04/ BRs, Lin 在 2019年2月26日,上午10:50,臧琳 <zangl...@jd.com<mailto:zangl...@jd.com>> 写道: 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 <hohen...@amazon.com<mailto:hohen...@amazon.com>>; serviceability- d...@openjdk.java.net<mailto:d...@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@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 <hohen...@amazon.com>; serviceability- d...@openjdk.java.net; 臧琳 <zangl...@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 <hohen...@amazon.com<mailto:hohen...@amazon.com>>; serviceability- d...@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 <hohen...@amazon.com<mailto:hohen...@amazon.com>> 发送时间: 2019年1月1日 4:56 收件人: 臧琳; serviceability-dev@openjdk.java.net<mailto:serviceability- d...@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- boun...@openjdk.java.net<mailto:serviceability-dev- boun...@openjdk.java.net>> on behalf of 臧琳 <zangl...@jd.com<mailto:zangl...@jd.com>> Date: Thursday, December 20, 2018 at 11:13 PM To: "serviceability-dev@openjdk.java.net<mailto:serviceability- d...@openjdk.java.net>" <serviceability- d...@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 –histo. It adds the “incremental” arguments that allow jmap –histo 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