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

[Mikael Gerdin]

Hi Dmitry,

On 2015-06-02 13:12, 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
>
> 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.

Sorry for this sort of drive-by review, but I really don't think 
get_field_offset_by_name should be in the oop class. If anywhere it 
should be on InstanceKlass, and using Symbol* to identify a Klass* seems 
incorrect since the same symbol can refer to different classes in 
different class loader contexts.

/Mikael

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