On Sun, 7 Feb 2021 08:20:10 GMT, Lin Zang <lz...@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
