RFR (XS) 8233790: Forward output from heap dumper to jcmd/jmap

[Thomas Stüfe]

Hi Ralf,

On Fri, Nov 8, 2019 at 4:45 PM Schmelter, Ralf <ralf.schmelter at sap.com>
wrote:

> Hi Thomas,
>
> thanks for the review.
>
>
> > Can you please add a comment about the new parameter? E.g. "optional
> outputStream to which progress- and error messages will be written".
>
> Will do.
>
>
> > - in HeapDumper::dump(): while you are on it could you please initialize
> my_path to NULL at function start?
>
> Do you mean the HeapDumper::dump_heap() method? That seems unrelated to my
> change. And I would prefer to get a warning about a potentially
> uninitialized use.
>
>
Yes its unrelated but since you are in the area... But okay, I leave it up
to you.


>
> > You can actually get rid of HeapDumper::error_as_C_string() altogher now.
>
> I thought about it, but the message is more than just the error text (you
> would get the "Dumping heap to <file> .." part) and is multiple lines. This
> seems not fitting for an exception message.
>
>
Okay.


>
> > Did you test that no jtreg tests fall over the changed output? e.g.
> test/jdk/sun/tools/jmap/BasicJMapTest.java, or whatever is under
> hotspot/jtreg/serviceability/jcmd
>
> I've checked them. They work because they just check that "Heap dump file
> created" is contained in the output, which is still the case or don't check
> the output at all (the dcmd tests).
>
>
Okay.


> Best regards,
> Ralf
>

Okay. If you only add the comment I do not need to see a new webrev.

Cheers, Thomas
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.java.net/pipermail/serviceability-dev/attachments/20191108/79e12a6f/attachment.html>