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