On Apr 11 2013, at 06:03 , Alan Bateman wrote: > On 10/04/2013 19:12, Mike Duigou wrote: >> On Apr 9 2013, at 19:56 , Martin Buchholz wrote: >> >>> Mike, thanks. >>> >>> I don't see anything wrong in this version, although the ongoing >>> complexification and special-case-ification (with attendant risk of bugs) >>> of ArrayList and HashMap, the two most didactically important classes, >>> continue to bother me. This change continues to feel marginal. >> It was surprising to find just how many ArrayList and HashMap instances were >> empty and/or never used. Unfortunately it's harder to globally fix >> applications than it is to and special case code to ArrayList and HashMap. > Right and I wonder if the performance folks could share some of the data that > they have gathered on this.
I started this review cycle with high level numbers for a wide variety of Java applications that the performance team looked at. Across these applications 1.4%(±2.9%) of HashMaps were forever empty and a similar percentage spent a large portion of their lifetime empty. For ArrayList the numbers were 1.0%(±2.6%) are forever empty and a larger percentage spent much of their life empty. As a percentage of heap space the empty instances account for about 1%. For some applications the numbers are much worse. Of the empty instances 85% were created at default size. > In any case, I think this patch is looking good now, assuming this is the > near-to-final version: > > http://cr.openjdk.java.net/~mduigou/JDK-8011200/4/webrev/ Correct. I am running it through final pre-integration tests now. Some performance testing has been done on the second review version of this patch (the last before removing initialCapacity fields unfortunately) which shows no performance regressions across our reference workloads and a few significant improvements (a GC benchmark). > > One remaining point that I'm a bit uneasy about is ArrayList.writeObject > writing out the size rather than the array length. It is specified to be the > array backing the ArrayList. It's now ignored by readObject (which is good) > but reconstituting on an older/other JDK release will/may reconstitute to the > trimmed size. It's not harmful that it does so. This effectively forces the trimming behaviour in all deserialization cases. Mike
