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