RFR (XXS) 8080535: (ch) Expected size of Character.UnicodeBlock.map is not optimal
Ivan Gerasimov
ivan.gerasimov at oracle.com
Wed May 20 17:32:16 UTC 2015
On 20.05.2015 12:04, Paul Sandoz wrote:
> On May 19, 2015, at 9:40 PM, Ivan Gerasimov <ivan.gerasimov at oracle.com> wrote:
>
>> Hi everyone!
>>
>> What about this variant:
>> http://cr.openjdk.java.net/~igerasim/8080535/02/webrev/
>>
> 45 int mapSize = map.size();
>
> Not used.
>
>
> 49 int INITIAL_CAPACITY = 680; //(int)(510 / 0.75f + 1.0f);
>
> Change to lower case.
Thanks, will fix both.
>
> Just had an idea... I believe static intializers are executed in textual order. So you could have a static code block after all static UnicodeBlock instances have been defined that asserts the size == 510 (or is <= 1024 * 0.75). In that case i would argue a static final representing the expected size is justified.
>
> Then your test can derive the initial capacity from mapSize.
I've just checked www.unicode.org to see how many new entries we can
expect in the nearest future.
Unicode 7 will add 32 new blocks (96 with aliases):
http://www.unicode.org/versions/Unicode7.0.0/
Unicode 8 will add 10 more blocks (30 with aliases):
http://www.unicode.org/versions/beta-8.0.0.html
So, after implementing Unicode 7 and 8 in java we're going to have
510+96+30 = 636 entries in the map.
The internal array of size 1024 (as it will be, if (510/.75 + 1) is used
for the initial capacity), is going to be sufficient to hold all these.
So, I think we're going to be good for a few next years.
And once the number of entries exceeds 768, the resgression test will
warn us.
Here's the update with the corrections to the test as you suggested:
http://cr.openjdk.java.net/~igerasim/8080535/03/webrev/
Sincerely yours,
Ivan
> Paul.
>
>> No named constant.
>> A comment in the code about initial capacity.
>> In the test, we use the same constant to check if it were sufficient to hold the final number of entries.
>>
>> If someone evil shrinks the initial capacity in the code, the test will not be able to detect it, though it seems unlikely.
>>
>> // Please ignore the change to test/TEST.groups in the webrev; it is a leftover from a fix for JDK-8080330
>>
>> Sincerely yours,
>> Ivan
>>
>
>
More information about the core-libs-dev
mailing list