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

Chris Plummer cjplummer at openjdk.java.net
Mon Feb 8 03:25:45 UTC 2021


On Sun, 7 Feb 2021 08:20:10 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 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 jcmd jmap.

src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/CommandProcessor.java line 1795:

> 1793:                      */
> 1794:                     if (cntTokens > 2) {
> 1795:                         err.println("Too big number of options: " + cntTokens);

"More than 2 options specified: "

src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/CommandProcessor.java line 1824:

> 1822:                             usage();
> 1823:                             return;
> 1824:                         }

I think having `if (cntTokens >= 1)` and `if (cntTokens == 2)` sections makes it harder to follow. Also, after calling parseHeapDumpCompressionLevel() you only need to check if 0 was returned. Otherwise you know it will be with the range of 1 to 9. I think the following might be easier to follow:
                    if (cntTokens == 1) { // first argument could be filename or "gz="
                        String option = t.nextToken();
                        if (!option.startsWith("gz=")) {
                            filename = option;
                        } else {
                            gzlevel = parseHeapDumpCompressionLevel(option);
                            if (gzlevel == 0) {
                                usage();
                                return;
                            }
                            filename = "heap.bin.gz";
                        }
                    }
                    if (cntTokens == 2) { // first argument is "gz=" followed by filename
                        gzlevel = parseHeapDumpCompressionLevel(option);
                        if (gzlevel == 0) {
                            usage();
                            return;
                        }
                        filename = t.nextToken();
                        if (filename.startsWith("gz=")) {
                            err.println("Filename should not start with "gz=": " + filename);
                            usage();
                            return;
                        }
                    }

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

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


More information about the hotspot-runtime-dev mailing list