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

Serguei Spitsyn sspitsyn at openjdk.java.net
Tue Feb 9 04:30:44 UTC 2021


On Tue, 9 Feb 2021 02:38:44 GMT, Lin Zang <lzang at openjdk.org> wrote:

>> Marked as reviewed by cjplummer (Reviewer).
>
> Thanks @plummercj @sspitsyn @YaSuenag  a lot for helping review this PR again and again! 
> I have marked it as ready for push.

Hi Lin,

Sorry for confusing you but I've not noticed a duplication in your code:

1778                 if (cntTokens > 2) {
1779                     usage();
1780                 } else {
1781                     JMap jmap = new JMap();
1782                     String filename = "heap.bin";
1783                     int gzlevel = 0;
 . . . . 
1794                     if (cntTokens > 2) {
1795                         err.println("More than 2 options specified: " + cntTokens);
1796                         usage();
1797                         return;
1798                     }
1799                     if (cntTokens == 1) { // first argument could be filename or "gz="
                             . . . .

There are two checks for cntTokens > 2. The second check has to be removed.
Also, a return needs to be added after the line 1779 with "usage();" , so the "else" statement can be removed.
This has to be refactored to something like this:
1778                 if (cntTokens > 2) {
                             err.println("More than 2 options specified: " + cntTokens);
1779                     usage();
                             return;
1780                 }
1781                 JMap jmap = new JMap();
1782                 String filename = "heap.bin";
1783                 int gzlevel = 0;
1799                 if (cntTokens == 1) { // first argument could be filename or "gz="
                         . . . .

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

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


More information about the hotspot-runtime-dev mailing list