RFR: 7150256: Add back Diagnostic Command JMX API

[Kelly O'Hair]

makefile changes look fine.

-kto

On Dec 20, 2012, at 3:40 PM, Mandy Chung wrote:

> Hi Frederic,
> 
> It's good to see the management interface for diagnostic commands is back.
>  
> On 12/17/12 6:58 AM, Frederic Parain wrote:
>> 
>> 
>> http://cr.openjdk.java.net/~fparain/7150256/webrev.01/ 
>> 
> Please send your makefiles change to build-dev and build-infra for review to ensure necessary change is made in the new build-infra work.
> 
> I didn't have time to a detailed review and I think it looks okay.  Some comments:
> 
> DiagnosticCommandImpl.Wrapper class
>    If there is any issue initializing the diagnostic command, it ignores the exception.  That makes it very hard to diagnose when things go wrong.  This exports diagnostic commands supported by the running JVM and so I would think any error would be a bug.  
> 
> DiagnosticCommandArgumentInfo is non-public class but its constructor and methods are still public.
> 
> VMManagementImpl.c
>    Is there any part of the native implementation moved to Java.    getDiagnosticCommandArgumentInfoArray() first constructs an array of DiagnosticCommandArgumentInfo and then calls Arrays.asList via JNI to return a List<DiagnosticCommandArgumentInfo>.  The returned List is used by getDiagnosticCommandInfo that returns DiagnosticCommandInfo[] since the DiagnosticCommandInfo constructor takes a List.  I would do as much code in Java if possible.  This is a bit hard to maintain.  This is not critical for the first push and can be saved for future cleanup.
> 
> Nit: there is no space between 'if' or 'for' and '(' in some source files and DcmdMBeanTest.  In VMManagementImpl.c - the jdk native code uses 4-space indentation which is different than the hotspot coding convention.
> 
> The new tests hardcode the port number that is unreliable and may fail in nightly/jprt testing (e.g. the port is occupied by another test run).   Some management tests have the same reliability issue and I'm not sure what the state is right now.  It'd be good if the new tests can avoid using hardcode port number.
> 
> Does the test throw any exception in case of error?  They catch various exceptions that should be thrown as a fatal error. 
> 
> Mandy

-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.openjdk.java.net/pipermail/serviceability-dev/attachments/20121220/72c612d5/attachment.html