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

[Chris Plummer]

On Mon, 25 Jan 2021 13:02:56 GMT, Lin Zang <lzang at openjdk.org> 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.hotspot.agent/share/classes/sun/jvm/hotspot/CommandProcessor.java line 2119:

> 2117:             }
> 2118:         } else {
> 2119:             err.println("Unknow option \"" + option + "\"");

"Unknown"

test/hotspot/jtreg/serviceability/sa/ClhsdbDumpheap.java line 50:

> 48: public class ClhsdbDumpheap {
> 49:     private static final String kHeapDumpFileNameDefault = "heap.bin";
> 50:     private static final String kHeapDumpFileNameGzDefault = "heap.bin.gz";

I'm not sure about the naming here. Why the `k` prefix? Usually in java static final variables are all upper case with `_` separating the words. So maybe something like HEAPDUMP_FILENAME_DEFAULT

test/hotspot/jtreg/serviceability/sa/ClhsdbDumpheap.java line 73:

> 71:             File out = new File(deCompressedFile);
> 72:             try {
> 73:                 GZIPInputStream gis = new GZIPInputStream(new FileInputStream(dump));

`printStackTraces()` uses `Reader.getStack()`, which does not know about gzipped hprof files. However, `Reader.readFile()` was modified to automatically detect gzipped hprof files. I suggest you do the same for `getStack()` rather than making the test do all the work. Probably `getStack()` and `readFile()` can share the code that does this.

test/hotspot/jtreg/serviceability/sa/ClhsdbDumpheap.java line 95:

> 93:         boolean needVerify;
> 94: 
> 95:         public SubTest(String comm, String fName, String expected, boolean isComp, boolean verify) {

I think moving the `expected` argument to the last position would make it easier to read all the invocations of `SubTest()`

test/hotspot/jtreg/serviceability/sa/ClhsdbDumpheap.java line 190:

> 188:             for (int i = 0; i < subtests.length;i++) {
> 189:                 runTest(theApp.getPid(), subtests[i]);
> 190:             }

Nice job organizing all the test cases!

-------------

PR: https://git.openjdk.java.net/jdk/pull/1712