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

[Thomas Stüfe]

Hi Ralf,

this makes sense. Some small remarks:

---

heapDumper.hpp

// dumps the heap to the specified file, returns 0 if success.
-  int dump(const char* path);
+  int dump(const char* path, outputStream* out = NULL);

Can you please add a comment about the new parameter? E.g. "optional
outputStream to which progress- and error messages will be written".

--

heapDumper.cpp

- in HeapDumper::dump(): while you are on it could you please initialize
my_path to NULL at function start?

---

You can actually get rid of HeapDumper::error_as_C_string() altogher now.
Last remaining caller is jmm_DumpHeap0(). You could use a stringStream to
catch the output of HeapDumper::dump() and use that string to build the
error message for the IOException in case of an error. I leave that up to
you.

---

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 ?

Thanks, Thomas






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

> This change forwards the output from the HeapDumper.dump() command to an
> optional output stream supplied by the caller.
> Until now the diagnositic command and the "dumpheap" command of the attach
> framework partly recreated the output by hand, but missing some information.
>
> Old output:
> Heap dump file created
>
> New output:
> Dumping heap to test.hprof ...
> Heap dump file created [9719330384 bytes in 27,759 secs]
>
> In addition to getting this improved information, it saves code too.
>
> Bugreport: https://bugs.openjdk.java.net/browse/JDK-8233790
> Webrev: http://cr.openjdk.java.net/~rschmelter/webrevs/8233790/webrev.0/
>
> Best regards,
> Ralf
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.java.net/pipermail/serviceability-dev/attachments/20191108/0422d642/attachment.html>