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

[Chris Plummer]

Hi Ralf,

Also looks good to me.

Serguei, the removed code is consolidated into HeapDumper::dump().

thanks,

Chris

On 11/8/19 4:25 PM, serguei.spitsyn at oracle.com wrote:
> 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
>