RFR(M, v13): JDK-8059036 : Implement Diagnostic Commands for heap and finalizerinfo

[Mandy Chung]

On 06/03/2015 08:29 AM, Dmitry Samersoff wrote:
> Updated webrev:
>
> http://cr.openjdk.java.net/~dsamersoff/JDK-8059036/webrev.13

I reviewed the jdk change.  The version looks okay and some comments:

ReferenceQueue.java - you should eliminate the use of rawtype:

84             Reference rn = r.next;

Change Reference to Reference<? extends T>

The forEach method - eliminate the use of raw type and
move @SuppressWarning to the field variable in line 182:

       @SuppressWarnings("unchecked")
       Reference<? extends T> rn = r.next;

FinalizerHistogram.java
   Copyright year needs update.

I personally think the VM code would be much simpler if you stay with 
alternative entry of String and int than dealing with a 
FinalizerHistogramEntry private class.  It's okay as FinalizerHistogram 
class is separated.

The comment in line 35 is suitable to move to the class description and 
this is the suggestion.

     // This FinalizerHistogram class is for GC.finalizer_info 
diagnostic command support.
     // It is invoked by the VM.

Should getFinalizerHistogram() return FinalizerHistogramEntry[]? The VM 
knows the returned type anyway.   It's an inner class of 
FinalizerHistogram and it can simply be named as "Entry".   I suggest to 
have Entry::increment method to replace ".instanceCount++".

The tests are in hotspot/test.    Can you add a unit test in jdk/test?  
Perhaps you can test FinalizerHistogram.getFinalizerHistogram() via 
reflection - just a sanity test.

A minor naming comment - now you have a FinalizerHistogram class. 
Perhaps the getFinalizerHistogram method should be renamed e.g. "dump"?

Mandy