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

[Lin Zang]

On Fri, 5 Feb 2021 10:36:25 GMT, Serguei Spitsyn <sspitsyn at openjdk.org> wrote:

>> Hi Lin,
>> 
>> A week ago you replied that you are accepting the following suggested refactoring:
>> 
>>                    /*
>>                     * 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) {
>>                             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;
>>                         }
>>                     }
>> 
>> But I still do not see it in the latest updates from you. It feels like there is some misunderstanding and confusion.
>> Could you, please, clear this up?
>> 
>> Thanks,
>> Serguei
>
> 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 (!keyValue[0].equals("gz")) {
>            err.println("Expected option is "gz="");
>            return 0;
>       }
>        if (keyValue.length != 2) {
>            err.println("Exactly one argument is expected for option "gz"");
>            return 0;
>        }
>        int gzl = 0;
>        String level = keyValue[1];
>        try {
>            gzl = Integer.parseInt(level);
>        } catch (NumberFormatException e) {
>            err.println("gz option value not an integer ("+level+")");
>            return 0;
>        }
>        if (gzl < 1 || gzl > 9) {
>            err.println("Compression level out of range (1-9): " + level);
>            return 0;
>        }
>        return gzl;
>    }

Hi Serguei,

> But I still do not see it in the latest updates from you. It feels like there is some misunderstanding and confusion.

Thanks a lot for remainder! Sorry that I forgot to submit the change.

> Also, this method can be refactored to something like this (the check for exactly one option argument is needed):

The change looks nice, I will made the change and submit it in a minute.

BRs,
Lin

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

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