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.schmel...@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 >
