Everyone, Updated webrev:
http://cr.openjdk.java.net/~dsamersoff/JDK-8059036/webrev.13 Changes to oop.inline.hpp is reverted and find_field used directly is diagnostic command. -Dmitry On 2015-06-03 11:48, Dmitry Samersoff wrote: > 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.