RFR(L) 8237354: Add option to jcmd to write a gzipped heap dump

Schmelter, Ralf ralf.schmelter at sap.com
Wed Jun 10 06:55:21 UTC 2020


Hi Christoph,

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.

Best regards,
Ralf

-----Original Message-----
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

Hi Ralf,

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:

workgroup.cpp
- update copyright year
L111: little spelling issue: forergound -> foreground

diagnosticCommand.cpp
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).

test/lib/jdk/test/lib/hprof/parser/GzipRandomAccess.java: 
L88: new ArrayList<> (diamond operator without type)

Thanks & Best regards
Christoph


More information about the hotspot-runtime-dev mailing list