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

[Lin Zang]

On Fri, 29 Jan 2021 04:27:32 GMT, Serguei Spitsyn <sspitsyn at openjdk.org> 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/hotspot/CommandProcessor.java
> 
> Thank you for the update.
> 
> The list of possible commands must include one more variant with no arguments:
> +                     * Possible command:
> +                     *     dumpheap gz=1 file;
> +                     *     dumpheap gz=1;
> +                     *     dumpheap file;
> +                     *     dumpheap
> 
> The argument processing is still not right.
> Why are there no checks for gzlevel < 0 and gzlevel > 9 ?
> 
> I'd suggest the following refactoring below:
> 
>                    /*
>                     * Possible commands:
>                     *     dumpheap gz=1 file
>                     *     dumpheap gz=1
>                     *     dumpheap file
>                     *     dumpheap
>                     *
>                     * Use default filename if cntTokens == 0.
>                     * Handle cases with cntTokens == 1 or 2.
>                     */
> 
>                     if (cntTokens > 2) {
>                         err.println("Too big number of options: " + cntTokens);
>                         usage();
>                         return;
>                     }
>                     if (cntTokens >= 1) { // parse first argument which is "gz=" option
>                         String option  = t.nextToken();
>                         gzlevel = parseHeapDumpCompressionLevel(option);
>                         if (gzlevel <= 0 || gzlevel > 9) {
>                             err.println("Invalid "gz=" option: " + option);
>                             usage();
>                             return;
>                         }
>                         filename = "heap.bin.gz";
>                     }
>                     if (cntTokens == 2) { // parse second argument which is filename
>                         filename = t.nextToken();
>                         if (filename.startsWith("gz=")) {
>                             err.println("Filename should not start with "gz=": " + filename);
>                             usage();
>                             return;
>                         }
>                     }

Hi @sspitsyn,
Thanks for your suggestion, the parseHeapDumpCompresslevel() inside tested the gzlevel, but I think your code is much reasonable. I will make it in next update. 
BRs,
Lin

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

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