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

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