RFR(L) 8237354: Add option to jcmd to write a gzipped heap dump
ralf.schmelter at sap.com
Wed Jun 10 06:55:21 UTC 2020
thanks for your review. I've incorporated your changes. I will run the relevant tests again and if no problems show up, I will submit the change later this day.
From: Langer, Christoph <christoph.langer at sap.com>
Sent: Tuesday, 9 June 2020 22:23
To: Schmelter, Ralf <ralf.schmelter at sap.com>
Cc: serviceability-dev at openjdk.java.net; hotspot-runtime-dev at openjdk.java.net runtime <hotspot-runtime-dev at openjdk.java.net>; David Holmes <david.holmes at oracle.com>; serguei.spitsyn at oracle.com; Ioi Lam <ioi.lam at oracle.com>; Lindenmaier, Goetz <goetz.lindenmaier at sap.com>
Subject: RE: RFR(L) 8237354: Add option to jcmd to write a gzipped heap dump
I finally managed to fully read through your change. Very nice piece of work.
I only found a few minor nits which would be nice if you could address them before pushing. But no need for further webrev. Here we go:
- update copyright year
L111: little spelling issue: forergound -> foreground
L509: spelling recommneded -> recommended
L510: Initialization of default value ("1") is not necessary as current implementation wouldn't allow the parameter -gz without value.
heapDumperCompression.hpp and heapDumperCompression.cpp:
License header says: Copyright (c) 2005, 2020, Oracle and/or its affiliates. All rights reserved.
However, it's a net new file, so it should just be 2020,
Also, since this is new code, coming from SAP, you should credit SAP in the copyright header (same way as you have done it in the test files).
L88: new ArrayList<> (diamond operator without type)
Thanks & Best regards
More information about the hotspot-runtime-dev