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

[Chris Plummer]

He also added the outputStream argument to HeapDumper::dump(). Both of 
the sections of code below already called dump(), and now they do so 
with the added outputStream argument. HeapDumper::dump() has been 
modified to print on the outputStream rather than to the tty.

Chris

On 11/8/19 5:02 PM, serguei.spitsyn at oracle.com wrote:
> Hi Chris,
>
> I'm a little bit confused.
> The Ralf's change in the HeapDumper::dump() is just replacement of 
> 'tty' occurrences with 'out',
> so the change has not moved the deleted code in these files into the 
> HeapDumper::dump().
>
> Probably, you wanted to say that the pre-existed error messages 
> printed in the HeapDumper::dump() are enough.
> This would explain why the code is deleted.
> Just wanted a bit of clarification from Ralf to make sure it is the case.
>
> Thanks,
> Serguei
>
>
> On 11/8/19 16:42, Chris Plummer wrote:
>> 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
>>>
>>
>