Re: RFR: 8257234 : Add gz option to SA jmap to write a gzipped heap dump [v20]

2021-02-25 Thread Lin Zang
On Sat, 20 Feb 2021 11:35:18 GMT, Lin Zang wrote: >> Marked as reviewed by cjplummer (Reviewer). > > Mark it as ready for pushing. Thanks @plummercj @sspitsyn and @YaSuenag for > help reviewing! Dear All, May I ask your help to push this PR as it is ready. Thanks! Lin - PR: https

Re: RFR: 8257234 : Add gz option to SA jmap to write a gzipped heap dump [v21]

2021-02-20 Thread Lin Zang
> 8257234 : Add gz option to SA jmap to write a gzipped heap dump Lin Zang has updated the pull request incrementally with one additional commit since the last revision: refine the argument name - Changes: - all: https://git.openjdk.java.net/jdk/pull/1712/files - new: https:/

Re: RFR: 8257234 : Add gz option to SA jmap to write a gzipped heap dump [v20]

2021-02-20 Thread Lin Zang
On Fri, 19 Feb 2021 20:01:08 GMT, Chris Plummer wrote: >> Lin Zang has updated the pull request incrementally with two additional >> commits since the last revision: >> >> - fix indentation isue >> - fix help message issue and Use ByteBuffer for integer writing > > Marked as reviewed by cjplu

Re: RFR: 8257234 : Add gz option to SA jmap to write a gzipped heap dump [v20]

2021-02-20 Thread Lin Zang
On Sat, 20 Feb 2021 10:48:22 GMT, Serguei Spitsyn wrote: >> Lin Zang has updated the pull request incrementally with two additional >> commits since the last revision: >> >> - fix indentation isue >> - fix help message issue and Use ByteBuffer for integer writing > > Hi Lin, > I'm sorry for t

Re: RFR: 8257234 : Add gz option to SA jmap to write a gzipped heap dump [v20]

2021-02-20 Thread Serguei Spitsyn
On Wed, 10 Feb 2021 04:04:56 GMT, Lin Zang wrote: >> 8257234 : Add gz option to SA jmap to write a gzipped heap dump > > Lin Zang has updated the pull request incrementally with two additional > commits since the last revision: > > - fix indentation isue > - fix help message issue and Use Byt

Re: RFR: 8257234 : Add gz option to SA jmap to write a gzipped heap dump [v20]

2021-02-19 Thread Chris Plummer
On Wed, 10 Feb 2021 04:04:56 GMT, Lin Zang wrote: >> 8257234 : Add gz option to SA jmap to write a gzipped heap dump > > Lin Zang has updated the pull request incrementally with two additional > commits since the last revision: > > - fix indentation isue > - fix help message issue and Use Byt

Re: RFR: 8257234 : Add gz option to SA jmap to write a gzipped heap dump [v20]

2021-02-19 Thread Lin Zang
On Wed, 10 Feb 2021 04:23:43 GMT, Yasumasa Suenaga wrote: >> Lin Zang has updated the pull request incrementally with two additional >> commits since the last revision: >> >> - fix indentation isue >> - fix help message issue and Use ByteBuffer for integer writing > > Marked as reviewed by ys

Re: RFR: 8257234 : Add gz option to SA jmap to write a gzipped heap dump [v19]

2021-02-09 Thread Yasumasa Suenaga
On Wed, 10 Feb 2021 02:42:26 GMT, Lin Zang wrote: > Do you think it is ok to also change these codes? If my understanding is > correct, the fileOutputStream does not have the process of big/little endian. Yes, I agree with your latest change. It looks good to me. - PR: https://git

Re: RFR: 8257234 : Add gz option to SA jmap to write a gzipped heap dump [v20]

2021-02-09 Thread Yasumasa Suenaga
On Wed, 10 Feb 2021 04:04:56 GMT, Lin Zang wrote: >> 8257234 : Add gz option to SA jmap to write a gzipped heap dump > > Lin Zang has updated the pull request incrementally with two additional > commits since the last revision: > > - fix indentation isue > - fix help message issue and Use Byt

Re: RFR: 8257234 : Add gz option to SA jmap to write a gzipped heap dump [v20]

2021-02-09 Thread Lin Zang
> 8257234 : Add gz option to SA jmap to write a gzipped heap dump Lin Zang has updated the pull request incrementally with two additional commits since the last revision: - fix indentation isue - fix help message issue and Use ByteBuffer for integer writing - Changes: - all: ht

Re: RFR: 8257234 : Add gz option to SA jmap to write a gzipped heap dump [v19]

2021-02-09 Thread Lin Zang
On Wed, 10 Feb 2021 01:53:38 GMT, Yasumasa Suenaga wrote: >> Lin Zang has updated the pull request incrementally with one additional >> commit since the last revision: >> >> refine code in test. > > src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/utilities/HeapHprofBinWriter.java > line

Re: RFR: 8257234 : Add gz option to SA jmap to write a gzipped heap dump [v19]

2021-02-09 Thread Lin Zang
On Wed, 10 Feb 2021 01:53:38 GMT, Yasumasa Suenaga wrote: >> Lin Zang has updated the pull request incrementally with one additional >> commit since the last revision: >> >> refine code in test. > > src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/utilities/HeapHprofBinWriter.java > line

Re: RFR: 8257234 : Add gz option to SA jmap to write a gzipped heap dump [v19]

2021-02-09 Thread Yasumasa Suenaga
On Tue, 9 Feb 2021 10:04:55 GMT, Lin Zang wrote: >> 8257234 : Add gz option to SA jmap to write a gzipped heap dump > > Lin Zang has updated the pull request incrementally with one additional > commit since the last revision: > > refine code in test. src/jdk.hotspot.agent/share/classes/sun/j

Re: RFR: 8257234 : Add gz option to SA jmap to write a gzipped heap dump [v19]

2021-02-09 Thread Lin Zang
> 8257234 : Add gz option to SA jmap to write a gzipped heap dump Lin Zang has updated the pull request incrementally with one additional commit since the last revision: refine code in test. - Changes: - all: https://git.openjdk.java.net/jdk/pull/1712/files - new: https://git

Re: RFR: 8257234 : Add gz option to SA jmap to write a gzipped heap dump [v17]

2021-02-09 Thread Lin Zang
On Tue, 9 Feb 2021 08:28:57 GMT, Serguei Spitsyn wrote: >> Dear Serguei, >> >>> There are two checks for cntTokens > 2. The second check has to be removed. >>> Also, a return needs to be added after the line 1779 with "usage();" , so >>> the "else" statement can be removed. >>> This has to be r

Re: RFR: 8257234 : Add gz option to SA jmap to write a gzipped heap dump [v17]

2021-02-09 Thread Lin Zang
On Tue, 9 Feb 2021 04:27:44 GMT, Serguei Spitsyn wrote: >> Thanks @plummercj @sspitsyn @YaSuenag a lot for helping review this PR >> again and again! >> I have marked it as ready for push. > > Hi Lin, > > Sorry for confusing you but I've not noticed a duplication in your code: > > 1778

Re: RFR: 8257234 : Add gz option to SA jmap to write a gzipped heap dump [v17]

2021-02-09 Thread Serguei Spitsyn
On Tue, 9 Feb 2021 06:37:21 GMT, Lin Zang wrote: >> Hi Lin, >> >> Sorry for confusing you but I've not noticed a duplication in your code: >> >> 1778 if (cntTokens > 2) { >> 1779 usage(); >> 1780 } else { >> 1781 JMap jmap

Re: RFR: 8257234 : Add gz option to SA jmap to write a gzipped heap dump [v18]

2021-02-08 Thread Lin Zang
> 8257234 : Add gz option to SA jmap to write a gzipped heap dump Lin Zang has updated the pull request incrementally with one additional commit since the last revision: remove redundant check - Changes: - all: https://git.openjdk.java.net/jdk/pull/1712/files - new: https://g

Re: RFR: 8257234 : Add gz option to SA jmap to write a gzipped heap dump [v17]

2021-02-08 Thread Serguei Spitsyn
On Tue, 9 Feb 2021 02:38:44 GMT, Lin Zang wrote: >> Marked as reviewed by cjplummer (Reviewer). > > Thanks @plummercj @sspitsyn @YaSuenag a lot for helping review this PR again > and again! > I have marked it as ready for push. Hi Lin, Sorry for confusing you but I've not noticed a duplicati

Re: RFR: 8257234 : Add gz option to SA jmap to write a gzipped heap dump [v17]

2021-02-08 Thread Lin Zang
On Mon, 8 Feb 2021 22:03:18 GMT, Chris Plummer wrote: >> Lin Zang has updated the pull request incrementally with one additional >> commit since the last revision: >> >> refine code for argument parsing > > Marked as reviewed by cjplummer (Reviewer). Thanks @plummercj @sspitsyn @YaSuenag a

Re: RFR: 8257234 : Add gz option to SA jmap to write a gzipped heap dump [v17]

2021-02-08 Thread Chris Plummer
On Mon, 8 Feb 2021 08:05:09 GMT, Lin Zang wrote: >> 8257234 : Add gz option to SA jmap to write a gzipped heap dump > > Lin Zang has updated the pull request incrementally with one additional > commit since the last revision: > > refine code for argument parsing Marked as reviewed by cjplumm

Re: RFR: 8257234 : Add gz option to SA jmap to write a gzipped heap dump [v17]

2021-02-08 Thread Lin Zang
> 8257234 : Add gz option to SA jmap to write a gzipped heap dump Lin Zang has updated the pull request incrementally with one additional commit since the last revision: refine code for argument parsing - Changes: - all: https://git.openjdk.java.net/jdk/pull/1712/files - new:

Re: RFR: 8257234 : Add gz option to SA jmap to write a gzipped heap dump [v16]

2021-02-07 Thread Chris Plummer
On Sun, 7 Feb 2021 08:20:10 GMT, Lin Zang wrote: >> 8257234 : Add gz option to SA jmap to write a gzipped heap dump > > Lin Zang has updated the pull request incrementally with two additional > commits since the last revision: > > - add test in HeapDumpTest and fix in argument parsing > - rev

Re: RFR: 8257234 : Add gz option to SA jmap to write a gzipped heap dump [v14]

2021-02-07 Thread Lin Zang
On Thu, 4 Feb 2021 03:54:26 GMT, Chris Plummer wrote: >> Lin Zang has updated the pull request incrementally with two additional >> commits since the last revision: >> >> - fix an issue of double printing error message. >> - fix jcmd jmap issue and add test in BascJMapTest and code refine > >

Re: RFR: 8257234 : Add gz option to SA jmap to write a gzipped heap dump [v16]

2021-02-07 Thread Lin Zang
> 8257234 : Add gz option to SA jmap to write a gzipped heap dump Lin Zang has updated the pull request incrementally with two additional commits since the last revision: - add test in HeapDumpTest and fix in argument parsing - revert changes in jcmd jmap Another PR has been created for

Re: RFR: 8257234 : Add gz option to SA jmap to write a gzipped heap dump [v15]

2021-02-05 Thread Lin Zang
On Sat, 6 Feb 2021 04:42:32 GMT, Chris Plummer wrote: >> Lin Zang has updated the pull request incrementally with one additional >> commit since the last revision: >> >> refine help message and also refactor the logic in CommandProcessor > > src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot

Re: RFR: 8257234 : Add gz option to SA jmap to write a gzipped heap dump [v15]

2021-02-05 Thread Chris Plummer
On Fri, 5 Feb 2021 15:36:00 GMT, Lin Zang wrote: >> 8257234 : Add gz option to SA jmap to write a gzipped heap dump > > Lin Zang has updated the pull request incrementally with one additional > commit since the last revision: > > refine help message and also refactor the logic in CommandProce

Re: RFR: 8257234 : Add gz option to SA jmap to write a gzipped heap dump [v14]

2021-02-05 Thread Chris Plummer
On Fri, 5 Feb 2021 19:58:42 GMT, Serguei Spitsyn wrote: >> Dear @plummercj, >> >>> `--dumpfile` and `--gz` can only be used with `--binaryheap`. That should >>> be made clear in the help text. >> >> I added indentation for these two sub-options. >> >>> * SA's `jhsdb jmap --binaryheap` (which

Re: RFR: 8257234 : Add gz option to SA jmap to write a gzipped heap dump [v14]

2021-02-05 Thread Lin Zang
On Thu, 4 Feb 2021 23:23:41 GMT, Chris Plummer wrote: >> Thanks for the thorough list, I have tested most of them but not all. I will >> cover them and update here later. >> >>> So really there are two implementations of heap dumping, one in the VM and >>> one in SA, but a total of 5 ways to g

Re: RFR: 8257234 : Add gz option to SA jmap to write a gzipped heap dump [v14]

2021-02-05 Thread Serguei Spitsyn
On Fri, 5 Feb 2021 15:32:26 GMT, Lin Zang wrote: >> One implementation is in the JVM itself, to be used when the JVM is still >> running well, and not just from command line tools. Heap dumping can also be >> triggered by the JVM itslef by setting flags like -XX:+HeapDumpBeforeFullGC. >> The o

Re: RFR: 8257234 : Add gz option to SA jmap to write a gzipped heap dump [v15]

2021-02-05 Thread Lin Zang
> 8257234 : Add gz option to SA jmap to write a gzipped heap dump Lin Zang has updated the pull request incrementally with one additional commit since the last revision: refine help message and also refactor the logic in CommandProcessor - Changes: - all: https://git.openjdk.ja

Re: RFR: 8257234 : Add gz option to SA jmap to write a gzipped heap dump [v14]

2021-02-05 Thread Lin Zang
On Fri, 5 Feb 2021 15:01:26 GMT, Lin Zang wrote: >> Also, this method can be refactored to something like this (the check for >> exactly one option argument is needed): >>private int parseHeapDumpCompressionLevel(String option) { >>String[] keyValue = option.split("="); >>if

Re: RFR: 8257234 : Add gz option to SA jmap to write a gzipped heap dump [v14]

2021-02-05 Thread Lin Zang
On Fri, 5 Feb 2021 10:36:25 GMT, Serguei Spitsyn wrote: >> Hi Lin, >> >> A week ago you replied that you are accepting the following suggested >> refactoring: >> >>/* >> * Possible commands: >> * dumpheap gz=1 file >>

Re: RFR: 8257234 : Add gz option to SA jmap to write a gzipped heap dump [v14]

2021-02-05 Thread Serguei Spitsyn
On Fri, 5 Feb 2021 10:03:09 GMT, Serguei Spitsyn wrote: >> One implementation is in the JVM itself, to be used when the JVM is still >> running well, and not just from command line tools. Heap dumping can also be >> triggered by the JVM itslef by setting flags like -XX:+HeapDumpBeforeFullGC. >

Re: RFR: 8257234 : Add gz option to SA jmap to write a gzipped heap dump [v14]

2021-02-05 Thread Serguei Spitsyn
On Thu, 4 Feb 2021 23:23:41 GMT, Chris Plummer wrote: >> Thanks for the thorough list, I have tested most of them but not all. I will >> cover them and update here later. >> >>> So really there are two implementations of heap dumping, one in the VM and >>> one in SA, but a total of 5 ways to g

Re: RFR: 8257234 : Add gz option to SA jmap to write a gzipped heap dump [v14]

2021-02-04 Thread Chris Plummer
On Thu, 4 Feb 2021 08:10:00 GMT, Lin Zang wrote: >> src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/SALauncher.java line 130: >> >>> 128: System.out.println("--binaryheapTo dump java >>> heap in hprof binary format."); >>> 129: System.out.println("--dumpf

Re: RFR: 8257234 : Add gz option to SA jmap to write a gzipped heap dump [v14]

2021-02-04 Thread Lin Zang
On Fri, 29 Jan 2021 22:54:57 GMT, Chris Plummer wrote: >> Lin Zang has updated the pull request incrementally with two additional >> commits since the last revision: >> >> - fix an issue of double printing error message. >> - fix jcmd jmap issue and add test in BascJMapTest and code refine >

Re: RFR: 8257234 : Add gz option to SA jmap to write a gzipped heap dump [v14]

2021-02-04 Thread Lin Zang
On Fri, 29 Jan 2021 22:51:23 GMT, Chris Plummer wrote: >> Lin Zang has updated the pull request incrementally with two additional >> commits since the last revision: >> >> - fix an issue of double printing error message. >> - fix jcmd jmap issue and add test in BascJMapTest and code refine >

Re: RFR: 8257234 : Add gz option to SA jmap to write a gzipped heap dump [v14]

2021-02-03 Thread Chris Plummer
On Fri, 29 Jan 2021 05:39:59 GMT, Lin Zang wrote: >> 8257234 : Add gz option to SA jmap to write a gzipped heap dump > > Lin Zang has updated the pull request incrementally with two additional > commits since the last revision: > > - fix an issue of double printing error message. > - fix jcmd

Re: RFR: 8257234 : Add gz option to SA jmap to write a gzipped heap dump [v13]

2021-02-03 Thread Lin Zang
On Thu, 28 Jan 2021 22:33:53 GMT, Chris Plummer wrote: >> Lin Zang has updated the pull request incrementally with one additional >> commit since the last revision: >> >> fix the logic of using gz= as file name > > You added clhsdb testing to ClhsdbDumpheap.java, but I had also suggested > a

Re: RFR: 8257234 : Add gz option to SA jmap to write a gzipped heap dump [v13]

2021-01-28 Thread Serguei Spitsyn
On Fri, 29 Jan 2021 05:14:31 GMT, Lin Zang wrote: >> Hi Lin, >> >> src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/CommandProcessor.java >> >> Thank you for the update. >> >> The list of possible commands must include one more variant with no >> arguments: >> + * Possi

Re: RFR: 8257234 : Add gz option to SA jmap to write a gzipped heap dump [v13]

2021-01-28 Thread Lin Zang
On Fri, 29 Jan 2021 05:33:19 GMT, Serguei Spitsyn wrote: > You are right. > So, in my suggestion just replace this block: > > ``` > if (gzlevel <= 0 || gzlevel > 9) { > err.println("Invalid "gz=" option: " + option); >

Re: RFR: 8257234 : Add gz option to SA jmap to write a gzipped heap dump [v14]

2021-01-28 Thread Lin Zang
> 8257234 : Add gz option to SA jmap to write a gzipped heap dump Lin Zang has updated the pull request incrementally with two additional commits since the last revision: - fix an issue of double printing error message. - fix jcmd jmap issue and add test in BascJMapTest and code refine --

Re: RFR: 8257234 : Add gz option to SA jmap to write a gzipped heap dump [v13]

2021-01-28 Thread Lin Zang
On Fri, 29 Jan 2021 04:27:32 GMT, Serguei Spitsyn wrote: >> You added clhsdb testing to ClhsdbDumpheap.java, but I had also suggested >> adding testing to BasicJMapTest.java to test pmap. Can you please add some >> testing there also? > > Hi Lin, > > src/jdk.hotspot.agent/share/classes/sun/jvm

Re: RFR: 8257234 : Add gz option to SA jmap to write a gzipped heap dump [v13]

2021-01-28 Thread Lin Zang
On Thu, 28 Jan 2021 22:33:53 GMT, Chris Plummer wrote: >> Lin Zang has updated the pull request incrementally with one additional >> commit since the last revision: >> >> fix the logic of using gz= as file name > > You added clhsdb testing to ClhsdbDumpheap.java, but I had also suggested > a

Re: RFR: 8257234 : Add gz option to SA jmap to write a gzipped heap dump [v13]

2021-01-28 Thread Serguei Spitsyn
On Thu, 28 Jan 2021 22:33:53 GMT, Chris Plummer wrote: >> Lin Zang has updated the pull request incrementally with one additional >> commit since the last revision: >> >> fix the logic of using gz= as file name > > You added clhsdb testing to ClhsdbDumpheap.java, but I had also suggested > a

Re: RFR: 8257234 : Add gz option to SA jmap to write a gzipped heap dump [v13]

2021-01-28 Thread Chris Plummer
On Wed, 27 Jan 2021 23:54:58 GMT, Lin Zang wrote: >> 8257234 : Add gz option to SA jmap to write a gzipped heap dump > > Lin Zang has updated the pull request incrementally with one additional > commit since the last revision: > > fix the logic of using gz= as file name src/jdk.hotspot.agent

Re: RFR: 8257234 : Add gz option to SA jmap to write a gzipped heap dump [v13]

2021-01-28 Thread Chris Plummer
On Wed, 27 Jan 2021 23:54:58 GMT, Lin Zang wrote: >> 8257234 : Add gz option to SA jmap to write a gzipped heap dump > > Lin Zang has updated the pull request incrementally with one additional > commit since the last revision: > > fix the logic of using gz= as file name You added clhsdb test

Re: RFR: 8257234 : Add gz option to SA jmap to write a gzipped heap dump [v13]

2021-01-27 Thread Lin Zang
> 8257234 : Add gz option to SA jmap to write a gzipped heap dump Lin Zang has updated the pull request incrementally with one additional commit since the last revision: fix the logic of using gz= as file name - Changes: - all: https://git.openjdk.java.net/jdk/pull/1712/files

Re: RFR: 8257234 : Add gz option to SA jmap to write a gzipped heap dump [v9]

2021-01-27 Thread Lin Zang
On Wed, 27 Jan 2021 20:22:00 GMT, Serguei Spitsyn wrote: >>> Do you think it is reasonable to treat "gz=[number]/[non-number]" >>> differently in this case? or should it just exit with error for all "gz=" >>> options that is not in the range of compression level? BTW, I think your >>> suggeste

Re: RFR: 8257234 : Add gz option to SA jmap to write a gzipped heap dump [v9]

2021-01-27 Thread Serguei Spitsyn
On Wed, 27 Jan 2021 18:58:01 GMT, Chris Plummer wrote: > I think we should produce an error for something like gz=abc rather than use > that as the filename, because I think it is likely user error. Agreed. My suggestion was exactly this. We have to return an error in any attempt to start filen

Re: RFR: 8257234 : Add gz option to SA jmap to write a gzipped heap dump [v9]

2021-01-27 Thread Chris Plummer
On Wed, 27 Jan 2021 10:36:54 GMT, Lin Zang wrote: > Do you think it is reasonable to treat "gz=[number]/[non-number]" differently > in this case? or should it just exit with error for all "gz=" options that is > not in the range of compression level? BTW, I think your suggested code is > bette

Re: RFR: 8257234 : Add gz option to SA jmap to write a gzipped heap dump [v12]

2021-01-27 Thread Lin Zang
> 8257234 : Add gz option to SA jmap to write a gzipped heap dump Lin Zang has updated the pull request incrementally with one additional commit since the last revision: fix code in CommandProcessor - Changes: - all: https://git.openjdk.java.net/jdk/pull/1712/files - new: htt

Re: RFR: 8257234 : Add gz option to SA jmap to write a gzipped heap dump [v9]

2021-01-27 Thread Lin Zang
On Tue, 26 Jan 2021 10:03:13 GMT, Serguei Spitsyn wrote: >> Copyrights need updating. > > Minor suggestion for > `src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/tools/JMap.java`: > +} else if (keyValue[0].equals("gz")) { > +if (keyValue.le

Re: RFR: 8257234 : Add gz option to SA jmap to write a gzipped heap dump [v9]

2021-01-27 Thread Serguei Spitsyn
On Tue, 26 Jan 2021 12:42:41 GMT, Lin Zang wrote: > This is used to guard the case cntTokens == 0, otherwise that t.nextToken() > will throw ArrayIndexOutOfBoundsException. The line `String option = t.nextToken();` can be moved to each of two if statements: if (cntTokens == 2) {

Re: RFR: 8257234 : Add gz option to SA jmap to write a gzipped heap dump [v11]

2021-01-27 Thread Lin Zang
> 8257234 : Add gz option to SA jmap to write a gzipped heap dump Lin Zang has updated the pull request incrementally with one additional commit since the last revision: fix issue cause JMapHProfLargeHeapTest fail and code refine - Changes: - all: https://git.openjdk.java.net/j

Re: RFR: 8257234 : Add gz option to SA jmap to write a gzipped heap dump [v10]

2021-01-26 Thread Chris Plummer
On Wed, 27 Jan 2021 06:38:02 GMT, Lin Zang wrote: >> 8257234 : Add gz option to SA jmap to write a gzipped heap dump > > Lin Zang has updated the pull request incrementally with two additional > commits since the last revision: > > - update copyright info > - fix commandline argument issue an

Re: RFR: 8257234 : Add gz option to SA jmap to write a gzipped heap dump [v9]

2021-01-26 Thread Lin Zang
On Tue, 26 Jan 2021 19:44:55 GMT, Chris Plummer wrote: >> Dear @plummercj, >> I have made investigation on extending GZIPOutputStream, since it is not >> possible to override/overwrite private method writeHeader(), the only way I >> could figure out is to create a class named HProfGZIPOutputS

Re: RFR: 8257234 : Add gz option to SA jmap to write a gzipped heap dump [v10]

2021-01-26 Thread Lin Zang
> 8257234 : Add gz option to SA jmap to write a gzipped heap dump Lin Zang has updated the pull request incrementally with two additional commits since the last revision: - update copyright info - fix commandline argument issue and refine test cases - Changes: - all: https://gi

Re: RFR: 8257234 : Add gz option to SA jmap to write a gzipped heap dump [v9]

2021-01-26 Thread Chris Plummer
On Tue, 26 Jan 2021 12:24:16 GMT, Lin Zang wrote: >> As for any testing requirement, maybe you could pass in a flag to Reader >> indicating whether or not the testing requires the "HPROF BLOCKSIZE" >> comment. > > Dear @plummercj, > I have made investigation on extending GZIPOutputStream, si

Re: RFR: 8257234 : Add gz option to SA jmap to write a gzipped heap dump [v9]

2021-01-26 Thread Lin Zang
On Tue, 26 Jan 2021 08:50:06 GMT, Serguei Spitsyn wrote: >> src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/CommandProcessor.java >> line 2119: >> >>> 2117: } >>> 2118: } else { >>> 2119: err.println("Unknow option \"" + option + "\""); >> >> "Unknown" > > -

Re: RFR: 8257234 : Add gz option to SA jmap to write a gzipped heap dump [v9]

2021-01-26 Thread Lin Zang
On Tue, 26 Jan 2021 04:41:40 GMT, Chris Plummer wrote: >> So the problem is that the code in GzipRandomAccess.getAccess() returns NULL >> if it doesn't find the "HPROF BLOCKSIZE" comment, but you are also saying >> that tools like heaphero.io work fine without the comment. So maybe you just >>

Re: RFR: 8257234 : Add gz option to SA jmap to write a gzipped heap dump [v9]

2021-01-26 Thread Lin Zang
On Tue, 26 Jan 2021 08:50:06 GMT, Serguei Spitsyn wrote: >> src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/CommandProcessor.java >> line 2119: >> >>> 2117: } >>> 2118: } else { >>> 2119: err.println("Unknow option \"" + option + "\""); >> >> "Unknown" > > -

Re: RFR: 8257234 : Add gz option to SA jmap to write a gzipped heap dump [v9]

2021-01-26 Thread Lin Zang
On Tue, 26 Jan 2021 04:41:40 GMT, Chris Plummer wrote: >> So the problem is that the code in GzipRandomAccess.getAccess() returns NULL >> if it doesn't find the "HPROF BLOCKSIZE" comment, but you are also saying >> that tools like heaphero.io work fine without the comment. So maybe you just >>

Re: RFR: 8257234 : Add gz option to SA jmap to write a gzipped heap dump [v9]

2021-01-26 Thread Serguei Spitsyn
On Mon, 25 Jan 2021 21:34:54 GMT, Chris Plummer wrote: >> Lin Zang has updated the pull request incrementally with one additional >> commit since the last revision: >> >> fix coding style issue > > Copyrights need updating. Minor suggestion for `src/jdk.hotspot.agent/share/classes/sun/jvm/h

Re: RFR: 8257234 : Add gz option to SA jmap to write a gzipped heap dump [v9]

2021-01-26 Thread Serguei Spitsyn
On Mon, 25 Jan 2021 20:53:19 GMT, Chris Plummer wrote: >> Lin Zang has updated the pull request incrementally with one additional >> commit since the last revision: >> >> fix coding style issue > > src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/CommandProcessor.java > line 2119: > >> 2

Re: RFR: 8257234 : Add gz option to SA jmap to write a gzipped heap dump [v9]

2021-01-25 Thread Chris Plummer
On Tue, 26 Jan 2021 04:40:19 GMT, Chris Plummer wrote: >> Sorry for typo. the GzipAccess should be GzipRandomAccess. > > So the problem is that the code in GzipRandomAccess.getAccess() returns NULL > if it doesn't find the "HPROF BLOCKSIZE" comment, but you are also saying > that tools like hea

Re: RFR: 8257234 : Add gz option to SA jmap to write a gzipped heap dump [v9]

2021-01-25 Thread Chris Plummer
On Tue, 26 Jan 2021 04:02:01 GMT, Lin Zang wrote: >> Hi Chris, >> Thanks for review and nice catch on this. >> >> I have considered using Reader.readFile() but it can not parse the gziped >> heap dump successfully, and I investigated the reason is that the underlying >> GzipAccess class requi

Re: RFR: 8257234 : Add gz option to SA jmap to write a gzipped heap dump [v9]

2021-01-25 Thread Lin Zang
On Tue, 26 Jan 2021 03:53:17 GMT, Lin Zang wrote: >> test/hotspot/jtreg/serviceability/sa/ClhsdbDumpheap.java line 73: >> >>> 71: File out = new File(deCompressedFile); >>> 72: try { >>> 73: GZIPInputStream gis = new GZIPInputStream(new >>> FileInputStrea

Re: RFR: 8257234 : Add gz option to SA jmap to write a gzipped heap dump [v9]

2021-01-25 Thread Lin Zang
On Mon, 25 Jan 2021 21:25:54 GMT, Chris Plummer wrote: >> Lin Zang has updated the pull request incrementally with one additional >> commit since the last revision: >> >> fix coding style issue > > test/hotspot/jtreg/serviceability/sa/ClhsdbDumpheap.java line 73: > >> 71: File ou

Re: RFR: 8257234 : Add gz option to SA jmap to write a gzipped heap dump [v9]

2021-01-25 Thread Chris Plummer
On Mon, 25 Jan 2021 13:02:56 GMT, Lin Zang wrote: >> 8257234 : Add gz option to SA jmap to write a gzipped heap dump > > Lin Zang has updated the pull request incrementally with one additional > commit since the last revision: > > fix coding style issue Copyrights need updating. src/jdk.hot

Re: RFR: 8257234 : Add gz option to SA jmap to write a gzipped heap dump [v7]

2021-01-25 Thread Lin Zang
On Tue, 19 Jan 2021 21:59:16 GMT, Chris Plummer wrote: >> Lin Zang has updated the pull request incrementally with one additional >> commit since the last revision: >> >> fix issue of setting gz option with no value > > I think you need to add some additional testing. You can use > ClhsdbDum

Re: RFR: 8257234 : Add gz option to SA jmap to write a gzipped heap dump [v9]

2021-01-25 Thread Lin Zang
> 8257234 : Add gz option to SA jmap to write a gzipped heap dump Lin Zang has updated the pull request incrementally with one additional commit since the last revision: fix coding style issue - Changes: - all: https://git.openjdk.java.net/jdk/pull/1712/files - new: https://g

Re: RFR: 8257234 : Add gz option to SA jmap to write a gzipped heap dump [v8]

2021-01-25 Thread Lin Zang
> 8257234 : Add gz option to SA jmap to write a gzipped heap dump Lin Zang has updated the pull request incrementally with one additional commit since the last revision: fix serveral issues and add test cases - Changes: - all: https://git.openjdk.java.net/jdk/pull/1712/files

Re: RFR: 8257234 : Add gz option to SA jmap to write a gzipped heap dump [v6]

2021-01-22 Thread Chris Plummer
On Fri, 22 Jan 2021 07:29:22 GMT, Lin Zang wrote: >> The check here is to control the segment size. It checks whether the current >> segment is too large, if yes, it fills the segment size slot in >> fillInHeapRecordLength() and set currentSegmentStart = 0, meaning to create >> a new segment

Re: RFR: 8257234 : Add gz option to SA jmap to write a gzipped heap dump [v7]

2021-01-22 Thread Chris Plummer
On Fri, 22 Jan 2021 07:04:47 GMT, Lin Zang wrote: >> src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/utilities/HeapHprofBinWriter.java >> line 475: >> >>> 473: if (!useSegmentedHeapDump) { >>> 474: // Fill in final length >>> 475: fillInHeapRecordLength(); >>

Re: RFR: 8257234 : Add gz option to SA jmap to write a gzipped heap dump [v6]

2021-01-21 Thread Lin Zang
On Fri, 22 Jan 2021 07:06:17 GMT, Lin Zang wrote: >> src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/utilities/HeapHprofBinWriter.java >> line 490: >> >>> 488: hprofBufferedOut = null; >>> 489: } >>> 490: >> >> Can you explain what this check is for and why it is no longer nee

Re: RFR: 8257234 : Add gz option to SA jmap to write a gzipped heap dump [v6]

2021-01-21 Thread Lin Zang
On Tue, 19 Jan 2021 21:38:28 GMT, Chris Plummer wrote: >> Lin Zang has updated the pull request incrementally with one additional >> commit since the last revision: >> >> code refine > > src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/utilities/HeapHprofBinWriter.java > line 490: > >> 4

Re: RFR: 8257234 : Add gz option to SA jmap to write a gzipped heap dump [v7]

2021-01-21 Thread Lin Zang
On Tue, 19 Jan 2021 21:35:16 GMT, Chris Plummer wrote: >> Lin Zang has updated the pull request incrementally with one additional >> commit since the last revision: >> >> fix issue of setting gz option with no value > > src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/utilities/HeapHprofBi

Re: RFR: 8257234 : Add gz option to SA jmap to write a gzipped heap dump [v6]

2021-01-19 Thread Chris Plummer
On Tue, 19 Jan 2021 05:43:12 GMT, Lin Zang wrote: >> 8257234 : Add gz option to SA jmap to write a gzipped heap dump > > Lin Zang has updated the pull request incrementally with one additional > commit since the last revision: > > code refine src/jdk.hotspot.agent/share/classes/sun/jvm/hotsp

Re: RFR: 8257234 : Add gz option to SA jmap to write a gzipped heap dump [v7]

2021-01-19 Thread Chris Plummer
On Tue, 19 Jan 2021 12:39:12 GMT, Lin Zang wrote: >> 8257234 : Add gz option to SA jmap to write a gzipped heap dump > > Lin Zang has updated the pull request incrementally with one additional > commit since the last revision: > > fix issue of setting gz option with no value I think you need

Re: RFR: 8257234 : Add gz option to SA jmap to write a gzipped heap dump [v7]

2021-01-19 Thread Lin Zang
> 8257234 : Add gz option to SA jmap to write a gzipped heap dump Lin Zang has updated the pull request incrementally with one additional commit since the last revision: fix issue of setting gz option with no value - Changes: - all: https://git.openjdk.java.net/jdk/pull/1712/fi

Re: RFR: 8257234 : Add gz option to SA jmap to write a gzipped heap dump [v6]

2021-01-19 Thread Yasumasa Suenaga
On Tue, 19 Jan 2021 05:43:12 GMT, Lin Zang wrote: >> 8257234 : Add gz option to SA jmap to write a gzipped heap dump > > Lin Zang has updated the pull request incrementally with one additional > commit since the last revision: > > code refine src/jdk.hotspot.agent/share/classes/sun/jvm/hotsp

Re: RFR: 8257234 : Add gz option to SA jmap to write a gzipped heap dump [v3]

2021-01-18 Thread Lin Zang
On Fri, 8 Jan 2021 13:21:21 GMT, Lin Zang wrote: >> Changes requested by cjplummer (Reviewer). > > Hi @plummercj , > Thanks very much for reviewing! > Sorry that I got stuck on other work and didn't read review message timely. > I will go through your suggestions carefully and try to fix all is

Re: RFR: 8257234 : Add gz option to SA jmap to write a gzipped heap dump [v6]

2021-01-18 Thread Lin Zang
> 8257234 : Add gz option to SA jmap to write a gzipped heap dump Lin Zang has updated the pull request incrementally with one additional commit since the last revision: code refine - Changes: - all: https://git.openjdk.java.net/jdk/pull/1712/files - new: https://git.openjdk.

Re: RFR: 8257234 : Add gz option to SA jmap to write a gzipped heap dump [v5]

2021-01-18 Thread Lin Zang
> 8257234 : Add gz option to SA jmap to write a gzipped heap dump Lin Zang has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains five additional commits since

Re: RFR: 8257234 : Add gz option to SA jmap to write a gzipped heap dump [v4]

2021-01-18 Thread Lin Zang
> 8257234 : Add gz option to SA jmap to write a gzipped heap dump Lin Zang has updated the pull request incrementally with one additional commit since the last revision: refine segment heap dump - Changes: - all: https://git.openjdk.java.net/jdk/pull/1712/files - new: https:/

Re: RFR: 8257234 : Add gz option to SA jmap to write a gzipped heap dump [v3]

2021-01-08 Thread Lin Zang
On Mon, 4 Jan 2021 22:35:24 GMT, Chris Plummer wrote: >> Lin Zang has updated the pull request incrementally with one additional >> commit since the last revision: >> >> delete unnecessary print > > Changes requested by cjplummer (Reviewer). Hi @plummercj , Thanks very much for reviewing! So

Re: RFR: 8257234 : Add gz option to SA jmap to write a gzipped heap dump [v3]

2021-01-04 Thread Chris Plummer
On Mon, 4 Jan 2021 22:00:34 GMT, Chris Plummer wrote: >> Lin Zang has updated the pull request incrementally with one additional >> commit since the last revision: >> >> delete unnecessary print > > src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/tools/JMap.java line 62: > >> 60:

Re: RFR: 8257234 : Add gz option to SA jmap to write a gzipped heap dump [v3]

2021-01-04 Thread Chris Plummer
On Thu, 10 Dec 2020 03:08:48 GMT, Lin Zang wrote: >> 8257234 : Add gz option to SA jmap to write a gzipped heap dump > > Lin Zang has updated the pull request incrementally with one additional > commit since the last revision: > > delete unnecessary print Changes requested by cjplummer (Revi

Re: RFR: 8257234 : Add gz option to SA jmap to write a gzipped heap dump [v3]

2020-12-09 Thread Lin Zang
> 8257234 : Add gz option to SA jmap to write a gzipped heap dump Lin Zang has updated the pull request incrementally with one additional commit since the last revision: delete unnecessary print - Changes: - all: https://git.openjdk.java.net/jdk/pull/1712/files - new: https:/

Re: RFR: 8257234 : Add gz option to SA jmap to write a gzipped heap dump [v2]

2020-12-09 Thread Lin Zang
> 8257234 : Add gz option to SA jmap to write a gzipped heap dump Lin Zang has updated the pull request incrementally with one additional commit since the last revision: refine comments - Changes: - all: https://git.openjdk.java.net/jdk/pull/1712/files - new: https://git.open

RFR: 8257234 : Add gz option to SA jmap to write a gzipped heap dump

2020-12-09 Thread Lin Zang
8257234 : Add gz option to SA jmap to write a gzipped heap dump - Commit messages: - 8257234 : Add gz option to SA jmap to write a gzipped heap dump Changes: https://git.openjdk.java.net/jdk/pull/1712/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=1712&range=00 Issue