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

[Dmitry Samersoff]

Everyone,

Updated webrev:

http://cr.openjdk.java.net/~dsamersoff/JDK-8059036/webrev.12

getFinalizerHistogram and FinalizerHistogramEntry moved to separate
package-private class. Hotspot code changed accordingly.

getFinalizerHistogram updated to use Peter's code.

-Dmitry


On 2015-06-03 09:06, Peter Levart wrote:
> Hi Dmitry,
> 
> On 06/02/2015 01:12 PM, Dmitry Samersoff wrote:
>> Staffan,
>>
>>> Instead of hardcoding the field offsets, you can use
>>> InstanceKlass::find_field and fieldDescriptor::offset to find the
>>> offset at runtime.
>> Done. Please, see
>>
>> http://cr.openjdk.java.net/~dsamersoff/JDK-8059036/webrev.11
> 
> In the jdk part, now that you have a FinalizerHistogramEntry class, you
> can simplify the code even further:
> 
> 
>     private static final class FinalizerHistogramEntry {
>         int instanceCount;
>         final String className;
> 
>         int getInstanceCount() {
>             return instanceCount;
>         }
> 
>         FinalizerHistogramEntry(String className) {
>             this.className = className;
>         }
>     }
> 
>     static Object[] getFinalizerHistogram() {
>         Map<String, FinalizerHistogramEntry> countMap = new HashMap<>();
>         queue.forEach(r -> {
>             Object referent = r.get();
>             if (referent != null) {
>                 countMap.computeIfAbsent(
>                         referent.getClass().getName(),
>                         FinalizerHistogramEntry::new).instanceCount++;
>                 /* Clear stack slot containing this variable, to decrease
>                    the chances of false retention with a conservative GC */
>                 referent = null;
>             }
>         });
> 
>         FinalizerHistogramEntry fhe[] = countMap.values()
>                 .toArray(new FinalizerHistogramEntry[countMap.size()]);
>         Arrays.sort(fhe,
>                 Comparator.comparingInt(
>                        
> FinalizerHistogramEntry::getInstanceCount).reversed());
>         return fhe;
>     }
> 
> 
> ... see, no copying loop required. Also notice the access modifier used
> on the nested class and it's fields/method/constructor. This way the
> class is private and fields can be accessed from getFinalizerHistogram
> and lambda without compiler generating access bridges.
> 
> 
> Regards, Peter
> 
>>
>> I put the function int get_filed_offset_by_name(Symbol*,Symbol*) to
>> oop.inline.hpp leaving a room for further cleanup because I found couple
>> of places in hotspot that implements mostly similar functionality.
>>
>> -Dmitry
>>
>> On 2015-06-01 10:18, Staffan Larsen wrote:
>>> Dmitry,
>>>
>>> Instead of hardcoding the field offsets, you can use InstanceKlass::find_field and fieldDescriptor::offset to find the offset at runtime. 
>>>
>>> Thanks,
>>> /Staffan
>>>
>>>> On 31 maj 2015, at 13:43, Dmitry Samersoff <dmitry.samersoff at oracle.com> wrote:
>>>>
>>>> Everyone,
>>>>
>>>> Please take a look at new version of the fix.
>>>>
>>>> http://cr.openjdk.java.net/~dsamersoff/JDK-8059036/webrev.10/
>>>>
>>>> Changes (to previous version) are in
>>>> Finalizer.java and diagnosticCommand.cpp
>>>>
>>>> This version copy data from Map.Entry<> to array of
>>>> FinalizerHistogramEntry instances then,
>>>> VM prints content of this array.
>>>>
>>>> -Dmitry
>>>>
>>>>
>>>> On 2015-05-28 21:06, Mandy Chung wrote:
>>>>> On 05/28/2015 07:35 AM, Peter Levart wrote:
>>>>>> Hi Mandy,
>>>>>>
>>>>>> On 05/27/2015 03:32 PM, Mandy Chung wrote:
>>>>>>> Taking it further - is it simpler to return String[] of all
>>>>>>> classnames including the duplicated ones and have the VM do the
>>>>>>> count?  Are you concerned with the size of the String[]?
>>>>>> Yes, the histogram is much smaller than the list of all instances.
>>>>>> There can be millions of instances waiting in finalizer queue, but
>>>>>> only a few distinct classes.
>>>>> Do you happen to know what libraries are the major contributors to these
>>>>> millions of finalizers?
>>>>>
>>>>> It has been strongly recommended to avoid finalizers (see Effective Java
>>>>> Item 7).   I'm not surprised that existing code is still using
>>>>> finalizers while we should really encourage them to migrate away from it.
>>>>>
>>>>>> What could be done in Java to simplify things in native code but still
>>>>>> not format the output is to convert the array of Map.Entry(s) into an
>>>>>> Object[] array of alternating {String, int[], String, int[], .... }
>>>>>>
>>>>>> Would this simplify native code for the price of a little extra work
>>>>>> in Java? The sizes of old and new arrays are not big (# of distinct
>>>>>> classes of finalizable objects in the queue).
>>>>> I also prefer writing in Java and writing less native code (much
>>>>> simplified).  It's an interface that we have to agree upon and keep it
>>>>> simple.  Having the returned Object[] as alternate String and int[] is a
>>>>> reasonable compromise.
>>>>>
>>>>> ReferenceQueue.java - you can move @SuppressWarning from the method to
>>>>> just the field variable "rn"
>>>>>     @SuppressWarnings("unchecked")
>>>>>     Reference<? extends T> rn = r.next;
>>>>>
>>>>> Finalizer.java
>>>>>     It's better to rename printFinalizationQueue as it inspects the
>>>>> finalizer reference queue (maybe getFinalizerHistogram?).  Can you move
>>>>> this method to the end of this class and add the comment saying that
>>>>> this is invoked by VM for jcmd -finalizerinfo support and @return to
>>>>> describe the returned content.  I also think you can remove
>>>>> @SuppressWarnings for this method.
>>>>>
>>>>> Mandy
>>>>
>>>> -- 
>>>> Dmitry Samersoff
>>>> Oracle Java development team, Saint Petersburg, Russia
>>>> * I would love to change the world, but they won't give me the sources.
>>
> 


-- 
Dmitry Samersoff
Oracle Java development team, Saint Petersburg, Russia
* I would love to change the world, but they won't give me the sources.