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

[Peter Levart]

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.
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.org/pipermail/hotspot-gc-dev/attachments/20150603/045b4b53/attachment.htm>