RFR (XXS) 8080535: (ch) Expected size of Character.UnicodeBlock.map is not optimal (original) (raw)

Ivan Gerasimov ivan.gerasimov at oracle.com
Mon May 18 15:16:57 UTC 2015

Thanks Chris for your review!

This looks much better. Trivially I’d calculate the initial size like: 510 / 0.75 + 1.0f ( plus 1 ) I'll all +1 for extra accuracy.

… 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. Sure, I'll a comment too.

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.

I'd rather leave the named constant, as it seems just a bit less error-prone to me.

I guess, this new test should not bring attention too often.

Sincerely yours, Ivan

-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> <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/> <http://cr.openjdk.java.net/%7Eigerasim/8080535/00/webrev/> Sincerely yours, Ivan

More information about the core-libs-dev mailing list