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

Serguei Spitsyn sspitsyn at openjdk.java.net
Fri Jan 29 04:30:47 UTC 2021


On Thu, 28 Jan 2021 22:33:53 GMT, Chris Plummer <cjplummer at openjdk.org> 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 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;
                        }
                    }

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

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


More information about the serviceability-dev mailing list