Request for Review: 7120511: Add diagnostic commands

[Frederic Parain]

Thanks for the review, my comments are inlined below.

On 01/ 5/12 05:19 PM, Karen Kinnear wrote:
> Frederic,
>
> Code looks good.
>
> A couple of minor comments/questions:
> 1) diagnosticCommand.cpp HeapDumpDCmd::execute
>     - There is a comment in attachListener under dump_heap that would
>        be helpful to have here as well: // Request a full GC before heap dump if live_objects ...

A comment has been added in diagnosticCommand.cpp and
the help message of the HeapDumpDCmd has been updated too.

> 2) I wonder if it would be helpful for the  new jcmds that share functionality with existing mechanisms
>      if it is worth a comment in the code noting where else this logic is called - might
>      help future bug fixers do thorough testing.

I've added cross-references in comments between the attachListener
file and the diagnosticCommand file.

> 3) attachListener jcmd
>     You've added an out->cr() after throwing the exception here, but I wonder if
>     you actually want it whether or not there was an exception thrown here -
>     e.g. inside execute I see a number of places where there are print calls for
>     exceptions and raw_print called - do those want the<cr>  as well?
>     Or are there cases where there is nothing printed so you would not want that? e.g. runFinalization?

The out->cr() in attachListener was correct, but other locations were
an exception was printed were lacking an out->cr() call. It's fixed now.

New webrev: http://cr.openjdk.java.net/~fparain/7120511/webrev.01/

Thank you,

Fred

-- 
Frederic Parain - Oracle
Grenoble Engineering Center - France
Phone: +33 4 76 18 81 17
Email: Frederic.Parain at Oracle.com