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>; serviceability- > 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 �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 > > > >