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

Langer, Christoph christoph.langer at sap.com
Tue Jun 9 20:22:54 UTC 2020


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

> -----Original Message-----
> From: Schmelter, Ralf <ralf.schmelter at sap.com>
> Sent: Montag, 8. Juni 2020 11:38
> To: Lindenmaier, Goetz <goetz.lindenmaier at sap.com>; Langer, Christoph
> <christoph.langer 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>
> Subject: RE: RFR(L) 8237354: Add option to jcmd to write a gzipped heap
> dump
> 
> Hi Goetz,
> 
> > What kind of tests did you run?
> 
> The jdk submit repo, the JCK tests (apart from API) and the jtreg tests on
> Windows x86/64, MacOS X, linux on x86/64, ppcle, ppcbe, zarch and aarch64
> and on AIX.
> 
> If there aren't any other concerns, I would like to commit this this change on
> Wednesday.
> 
> Best regards,
> Ralf
> 
> -----Original Message-----
> From: Lindenmaier, Goetz <goetz.lindenmaier at sap.com>
> Sent: Friday, 5 June 2020 18:02
> To: Schmelter, Ralf <ralf.schmelter at sap.com>; Langer, Christoph
> <christoph.langer 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>
> Subject: RE: RFR(L) 8237354: Add option to jcmd to write a gzipped heap
> dump
> 
> Hi Ralf,
> 
> Thanks for the quick reply and all the fixes.
> The changes to the workgroup are ok.
> Reviewed.  (An incremental webrev would have helped ��)
> 
> What kind of tests did you run?
> 
> > Yes, the buffer is now smaller (1M) versus the original (8M). You need
> > to be able to at least allocate one buffer or you get an error (this
> > is handled in the CompressionBackend ctor). You then allocate
> > additional buffers as needed (we want a new buffer, but there is no
> > free one), until we have a buffer for every worker thread or until
> > the allocation of the buffer failed. In this case some threads will
> > be idle, since we cannot have a buffer for each thread.
> Ok, that's what I thought. Thanks for the explanation.
> 
> > > Another question.
> > > The basic dumping is done sequential, right? The comression
> > > is parallel. Is there a tradeoff in #of threads where
> > > the compression is faster than writing?
> > Yes. The compression and writing is done parallel. Depeding on
> > the compression level and the speed of your harddrive, not all
> > threads will be active all the time. But since we reuse the GC threads
> > this should not matter. And the relative poor performance of
> > deflate() ensures that at least 5 to 10 threads will probably always
> > be active ;)
> Ok, thanks.
> 
> Best regards,
>   Goetz.


More information about the hotspot-runtime-dev mailing list