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

Dmitry Samersoff dmitry.samersoff at oracle.com
Tue Jun 2 21:04:53 UTC 2015


Mikael,

The reason of placing get_filed_offset_by_name to the oop.inline is that
hotspot widely duplicate this code.

Symbol is used to identify a field within klass, not a klass them self
so I think it's OK to have it tied to the oopDesc.

-Dmitry

On 2015-06-02 15:06, Mikael Gerdin wrote:
> 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.
>>>
>>
>>


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


More information about the hotspot-gc-dev mailing list