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

[serguei.spit...@oracle.com]

Hi Ralf, The fix looks Okay in general. A couple of questions. The dumper.dump() returns int value. Returned value is not used anymore in the attachListener.cpp and diagnisticCommand.cpp. Is it still used somewhere else or we can replace it with void? Could you explain a little bit why the following fragments were removed? Is it because this information is not that useful or there is some other motivation? attachListener.cpp: - int res = dumper.dump(op->arg(0)); - if (res == 0) { - out->print_cr("Heap dump file created"); - } else { - // heap dump failed - ResourceMark rm; - char* error = dumper.error_as_C_string(); - if (error == NULL) { - out->print_cr("Dump failed - reason unknown"); - } else { - out->print_cr("%s", error); - } - } diagnisticCommand.cpp - if (res == 0) { - output()->print_cr("Heap dump file created"); - } else { - // heap dump failed - ResourceMark rm; - char* error = dumper.error_as_C_string(); - if (error == NULL) { - output()->print_cr("Dump failed - reason unknown"); - } else { - output()->print_cr("%s", error); - } - } Thanks, Serguei On 11/8/19 03:56, Schmelter, Ralf 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