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

[Chris Plummer]

On Tue, 26 Jan 2021 12:24:16 GMT, Lin Zang <lzang at openjdk.org> wrote:

>> As for any testing requirement, maybe you could pass in a flag to Reader indicating whether or not the testing requires the "HPROF BLOCKSIZE"  comment.
>
> Dear @plummercj,
> I have made  investigation on extending GZIPOutputStream,  since it is not possible to override/overwrite private method writeHeader(), the only way I could figure out is to create a class named HProfGZIPOutputStream that extends DeflaterOutputStream,  copies most of GZIPOutputStream's code and rewrite a writeHeader() file. 
> 
> I am not sure whether it is nice since most of the code are same with GZIPOutputStream.  IMO, it may be nice if we can add another writeHeader() method to GZIPOutputStream that support adding file comments, but I am also not sure whether it is acceptable to add a new method in this general used class. 
> 
> BRs,
> Lin

I don't think we want to essentially clone GZIPOutputStream into a new HProfGZIPOutputStream in order to solve this problem. I think adding a GZIPOutputStream.writeHeader() method that supports adding a comment make sense, but probably not something that should be done with this CR. I new CR can be filed for it, and maybe take care of it sometime in the future, but for this we should focus on a solution that doesn't require it and isn't too complicated. 

Looking back at my original suggestion to put the gzip reading support in Reader.getStack(), I'm not clear on the issue you ran into. You said that GzipRandomAccess requires the "HPROF BLOCKSIZE=" comment, but I'm not so sure you need to use GzipRandomAccess. You can use the same gzip detection mechanism as Reader.readFile() does to see if it can read the file without unzipping it, and if not just do what your test currently does now, and try to read it with GZIPInputStream. You don't need to use GzipRandomAccess here.

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

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