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