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

[Ivan Gerasimov]

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