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

[Serguei Spitsyn]

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

>> One implementation is in the JVM itself, to be used when the JVM is still running well, and not just from command line tools. Heap dumping can also be triggered by the JVM itslef by setting flags like -XX:+HeapDumpBeforeFullGC. The other implementation is in SA, to be used on a core file or a hung JVM process, although it can also be used on a JVM that is still running well. BTW, there used to be a 3rd implementation. The old hprof profiler (a JVMTI agent) also was used to generate hprof files.
>
> 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) {
       int gzl = 0;
       String[] keyValue = option.split("=");
       assert keyValue[0].equals("gz") : "expected option to start from gz=";
       if (keyValue.length != 2) {
           err.println("Exactly one argument is expected for option "gz"");
           return 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;
   }

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

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