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

[Chris Plummer]

On Tue, 19 Jan 2021 12:39:12 GMT, Lin Zang <lzang at 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 one additional commit since the last revision:
> 
>   fix issue of setting gz option with no value

I think you need to add some additional testing. You can use ClhsdbDumpheap.java and also BasicJMapTest.java. Besides testing with a compressed hprof file, also add a test for "gz=" with no compression value and with invalid compression value.

src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/CommandProcessor.java line 1792:

> 1790:                         return;
> 1791:                     }
> 1792:                     if (keyValue[0].equals("gz")) {

This code now assumes there will be a `gz` option specified. If there isn't, you will still execute this section of code, and get an NPE at this point.

src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/utilities/HeapHprofBinWriter.java line 403:

> 401:         VM vm = VM.getVM();
> 402: 
> 403:         // Check weather we should dump the heap as segments

Should be "whether"

src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/utilities/HeapHprofBinWriter.java line 475:

> 473:         if (!useSegmentedHeapDump) {
> 474:             // Fill in final length
> 475:             fillInHeapRecordLength();

It's unclear to me why this code is now conditional on not using a segmented heap dump.

src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/utilities/HeapHprofBinWriter.java line 1307:

> 1305: 
> 1306:     /**
> 1307:      * The class implements a buffered output stream for segment data dump.

Should be "segmented data dump"

src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/utilities/HeapHprofBinWriter.java line 1309:

> 1307:      * The class implements a buffered output stream for segment data dump.
> 1308:      * It is used inside HeapHprofBinWritter only for heap dump.
> 1309:      * Because the current implementation of segment heap dump needs to update

"segmented"

src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/utilities/HeapHprofBinWriter.java line 1316:

> 1314:      * If the data to be written are larger than internal buffer, or the internal buffer
> 1315:      * is full, the internal buffer will be extend to a larger one.
> 1316:      * This class defines a switch to turn on/off the segment mode. if turned off,

"segmented". Also, uppercase "If".

src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/utilities/HeapHprofBinWriter.java line 1317:

> 1315:      * is full, the internal buffer will be extend to a larger one.
> 1316:      * This class defines a switch to turn on/off the segment mode. if turned off,
> 1317:      * it behaves same as BufferedOutputStream.

"the same as"

src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/utilities/HeapHprofBinWriter.java line 517:

> 515: 
> 516:     private void fillInHeapRecordLength() throws IOException {
> 517:         assert !useSegmentedHeapDump : "fillInHeapRecordLength is not supported for segment heap dump";

"segmented heap dump"

src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/utilities/HeapHprofBinWriter.java line 1321:

> 1319:     private class SegmentedOutputStream extends BufferedOutputStream {
> 1320:         /**
> 1321:          * Creates a new buffered output stream to support segment heap dump data.

"segmented"

src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/utilities/HeapHprofBinWriter.java line 1335:

> 1333: 
> 1334:         /**
> 1335:          * Creates a new buffered output stream to support segment heap dump data.

"segmented"

src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/utilities/HeapHprofBinWriter.java line 1493:

> 1491:         // Segment support.
> 1492:         private boolean segmentMode;
> 1493:         private boolean allowSegmental;

"allowSegmented" would be a better name.

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

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