RFR: 8257234 : Add gz option to SA jmap to write a gzipped heap dump [v17]
Lin Zang
lzang at openjdk.java.net
Tue Feb 9 10:04:56 UTC 2021
On Tue, 9 Feb 2021 08:28:57 GMT, Serguei Spitsyn <sspitsyn at openjdk.org> wrote:
>> Dear Serguei,
>>
>>> 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:
>>
>> Nice catch and good suggestion. Thanks a lot for point it out!
>> I think It is no hurry to push this PR, and welcome for any comment.
>> Thanks!
>
> Hi Lin,
> Thank you for update.
>
> test/lib/jdk/test/lib/hprof/parser/Reader.java:
> + } else if ((i >>> 8) == 0x1f8b08) {
> + // Possible gziped file.
>
> Could you, please, define a named constant and use it instead of the literal value 0x1f8b08?
> Also, would it make sense to extend the comment to explain about it a little bit?
>
> I'm still looking at some files (e.g., HeapHprofBinWriter.java).
>
> Thanks,
> Serguei
Dear @sspitsyn,
Thanks for your suggestion.
> I'm still looking at some files (e.g., HeapHprofBinWriter.java).
No problem, please take your time. Appreciated for your effort helping review!
BRs,
Lin
-------------
PR: https://git.openjdk.java.net/jdk/pull/1712
More information about the serviceability-dev
mailing list