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

[Chris Plummer]

On Tue, 26 Jan 2021 04:02:01 GMT, Lin Zang <lzang at openjdk.org> wrote:

>> Hi Chris, 
>> Thanks for review and nice catch on this.
>> 
>> I have considered using Reader.readFile() but it can not parse the gziped heap dump successfully, and I investigated the reason is that the underlying GzipAccess class requires the gziped heap dump file to have "HPROF BLOCKSIZE=xxx" bytes in the gziped file header.
>> I also investigated that the implementation of jmap command in jdk.jcmd added these words as a "comment section" in gz file headers, by passing it to the "ZIP_GZip_Fully()" when it is first called. 
>> But I cannot find a way to do same thing in GZIPOutputStream(), it's writeHeader() does not support "comment" section. 
>> Also I have verified the gziped heap dump file generated with GZIPOutputStream() could be parsed by heaphero.io successfully. 
>> 
>> So it seems to me that "HPROF BLOCKSIZE" may not be a must for general gziped heap dump, but I am not sure whether it is required for testing the jmap command, so I didn't touch GzipAccess in test and do decompress manually in ClhsdbDumpheap.java. 
>> 
>> I am considering another way to modify GzipAccess for testing to accept heap dump file without "HPROF BLOCKSIZE" header, I will try to investigate whether it could work. 
>> What do you think? 
>> 
>> Thanks!
>> Lin
>
> Sorry for typo. the GzipAccess should be GzipRandomAccess.

So the problem is that the code in GzipRandomAccess.getAccess() returns NULL if it doesn't find the "HPROF BLOCKSIZE" comment, but you are also saying that tools like heaphero.io work fine without the comment. So maybe you just need to fix GzipRandomAccess.getAccess() to use a default blocksize if none is specified.

On the other hand, it seems it would be best if the SA hprof file was no different than the jcmd hprof file, and the SA file also included the comment. But as you said GZIPOutputStream() does not support adding a comment in the same manner as the zlib library does. I'm not sure how you would go about adding the support, so probably it is not worth it.

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

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