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
