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.gerasi...@oracle.com <mailto:ivan.gerasi...@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
