On Fri, 19 Mar 2021 19:40:13 GMT, Chris Plummer <cjplum...@openjdk.org> wrote:
>> Koichi Sakata has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Fix the sort bug for huge bytes in jhisto > > I see another cast-to-int related bug, and it's visible in your jhisto output > (both before and after): > > 304: 1 16 jdk.internal.perf.Perf > 305: 1 16 java.util.jar.JavaUtilJarAccessImpl > 306: 254 2863336944 int[] > Total : 22803 -1430650672 > > The large `int[]` array should be at the top of the list, not the bottom. The > issue is in ObjectHistogramElement.java: > > public int compare(ObjectHistogramElement other) { > return (int) (other.size - size); > } > > So this will result in the returned value having the wrong sign if the > difference between `other.size` and `size` is too large. In > ObjectHistogram.java, just above the code you fixed, we have: > > public List<ObjectHistogramElement> getElements() { > List<ObjectHistogramElement> list = new ArrayList<>(); > list.addAll(map.values()); > Collections.sort(list, new Comparator<>() { > public int compare(ObjectHistogramElement o1, ObjectHistogramElement > o2) { > return o1.compare(o2); > } > }); > return list; > } > So it looks like this is calling the buggy `compare()` method. I think the > fix is to have `compare()` return -1, 0, or 1 depending on the `long` value > of `(other.size - size)`, rather than just trying to return `(other.size - > size)`. @plummercj That is right! I fixed that bug and pasted the latest output of jhisto to the body. Of course I re-ran the jtreg test. ------------- PR: https://git.openjdk.java.net/jdk/pull/3087