On Tue, 26 Jan 2021 12:24:16 GMT, Lin Zang <lz...@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

Reply via email to