RFR 8081027: Create a common test to check adequacy of initial size of static HashMap/ArrayList fields (original) (raw)

Ivan Gerasimov ivan.gerasimov at oracle.com
Thu May 28 22:43:07 UTC 2015

Thank you Martin for looking at this!

On 27.05.2015 17:30, Martin Buchholz wrote:

Thanks.

Your methods like ofArrayList declare that they throw ReflectiveOperationException, but also have a try/catch to prevent that from ever happening. Fixed!

The assertions e.g. OptimalCapacity.ofArrayList(XClass.class, "theList", N) could look more like assertions, e.g. OptimalCapacity.assertOptimallySized(Class<?> clazz, String fieldName, int initialCapacity) Good naming is one of the most difficult part of coding IMO. The chosen names were meant to be read literally: "optimal capacity of ArrayList", etc. I agree, 'assert' would tell more about these methods' intention, but I'm not sure how to include it in the names. Maybe rename the class to AssertOptimalCapacity?

getStorage returns the actual internal array, but you only ever need the length. Maybe I would have a single method

static int internalArraySize(Object x) { ... } Yes. I was going to compare the references, but finished comparing only sizes. Changed the method to internalArraySize(), as you suggested.

"enteties" is misspelled. Fixed.

For ArrayLists, I would have been happy enough just testing that we have 100% utilization, i.e. size of array is the same as size of the List, without checking the initial capacity. But then the test wouldn't have caught the "bug" in src/java.base/share/classes/sun/security/ssl/ExtensionType.java The ArrayList was pre-sized to 9, and after reallocation the capacity happened to become (9 + 9/2) = 13, which by coincidence is the final size of the List.

Please have a look at the updated webrev: http://cr.openjdk.java.net/~igerasim/8081027/01/webrev/

Sincerely yours, Ivan

On Mon, May 25, 2015 at 4:07 PM, Ivan Gerasimov <ivan.gerasimov at oracle.com <mailto:ivan.gerasimov at oracle.com>> wrote:

Hello! This is in some way continuation of JDK-8080535 (Expected size of Character.UnicodeBlock.map is not optimal). A new utility class OptimalCapacity was added to jdk.testlibrary package. The test NonOptimalMapSize.java that had been added with JDK-8080535, was rewritten with use of this new class. A couple more tests were added, which utilize methods of OptimalCapacity for checking sizes of ArrayList and IdentityHashMap static variables. Optimization of initial sizes of two more variables saves us one reallocation during java start-time and a few more bytes of memory. Would you please help review this fix? BUGURL: https://bugs.openjdk.java.net/browse/JDK-8081027 WEBREV: http://cr.openjdk.java.net/~igerasim/8081027/00/webrev/ <http://cr.openjdk.java.net/%7Eigerasim/8081027/00/webrev/> Sincerely yours, Ivan

More information about the core-libs-dev mailing list