RFR (XXS) 8080535: (ch) Expected size of Character.UnicodeBlock.map is not optimal (original) (raw)
Chris Hegarty chris.hegarty at oracle.com
Sat May 16 19:52:26 UTC 2015
- Previous message: RFR (XXS) 8080535: (ch) Expected size of Character.UnicodeBlock.map is not optimal
- Next message: RFR (XXS) 8080535: (ch) Expected size of Character.UnicodeBlock.map is not optimal
- Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]
On 16 May 2015, at 01:59, Ivan Gerasimov <ivan.gerasimov at oracle.com> wrote:
On 16.05.2015 2:18, Martin Buchholz wrote: I don't think you're taking the load factor into account. Hm. You're right, HashMap's constructor expects the capacity as the argument. I was confused by IdentityHashMap, whose constructor expects the maximum number of elements to be inserted.
https://code.google.com/p/guava-libraries/source/browse/guava/src/com/google/common/collect/Maps.java?r=f8144b4df7eec5f1c9e6942d8d5ef734a8767fd9#110 You want to check (via reflection) that the size of the backing array is unchanged if you copy the map with new HashMap(map). Sorry, I didn't get it. How could we detect that the initial capacity wasn't big enough, if we hadn't stored it? I wouldn't bother defining the constant. I only need it in the regression test, to check whether it was sufficient. Here's the updated webrev: http://cr.openjdk.java.net/~igerasim/8080535/01/webrev/ <http://cr.openjdk.java.net/~igerasim/8080535/01/webrev/>
This looks much better. Trivially Iād calculate the initial size like: 510 / 0.75 + 1.0f ( plus 1 )
⦠but your regression test should prove that the plus one is not needed.
Maybe a comment that the 0.75 is the default load factor from HashMap.
The constant could be removed and the ā510ā be used directly in the test. Since the test is whitebox.
The test is fine, but could be a little less obscure if it looked at the table size, rather than the equality. But what you have is fine.
-Chris.
Comments, suggestions are welcome! Sincerely yours, Ivan
On Fri, May 15, 2015 at 4:01 PM, Ivan Gerasimov <ivan.gerasimov at oracle.com <mailto:ivan.gerasimov at oracle.com>> wrote:
Hello! When constructing the map, the expected size is specified to be 256, but then 510 elements are inserted. A new whitebox test is provided, so the next time the number of entries grows, the expected size will not be forgotten. Would you please help review this fix? BUGURL: https://bugs.openjdk.java.net/browse/JDK-8080535 WEBREV: http://cr.openjdk.java.net/~igerasim/8080535/00/webrev/ <http://cr.openjdk.java.net/%7Eigerasim/8080535/00/webrev/> Sincerely yours, Ivan
- Previous message: RFR (XXS) 8080535: (ch) Expected size of Character.UnicodeBlock.map is not optimal
- Next message: RFR (XXS) 8080535: (ch) Expected size of Character.UnicodeBlock.map is not optimal
- Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]