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