[12] RFR: 8215194: Initial size of UnicodeBlock map is incorrect

[Ivan Gerasimov]

Hi Naoto!

The fix looks good, thank you!

I wonder if the test can be updated to also verify the optimal capacity 
of HashMap<String, Character.UnicodeScript> aliases;


On 12/11/18 10:33 AM, Naoto Sato wrote:
> Hi Roger,
>
> On 12/11/18 10:12 AM, Roger Riggs wrote:
>> Hi Naoto,
>>
>> Looks ok,
>>
>> I intended only the number of elements to be defined as a constant.
>> The other factors can be hard coded.
>
> OK, I revised it again:
>
> http://cr.openjdk.java.net/~naoto/8215194/webrev.02/
>
>>
>> In the test, you will still have to edit the test when the number 
>> changes.
>> I meant to avoid that edit.  Though then may be there is not need for 
>> the test at all.
>
> Yes, if we just reflectively use the constant in 
> Character.UnicodeBlock in the test, it is guaranteed to succeed no 
> matter what.
OptimalCapacity.ofHashMap uses the initialCapacity to verify that a 
HashMap created with that initialCapacity will not reallocate its 
internal array after inserting the factual number of elements.

So, if one day the number of entities is increased, but the constant 
NUM_ENTITIES is not updated, then the test will catch it.

With kind regards,
Ivan

> Thus I added the assertion. Still, the test ofHashMap() succeeds till 
> the block additions surpasses that 1024 capacity (thus this was 
> overlooked on Unicode 11 upgrade).
>
> Naoto
>
>>
>> Roger
>>
>>
>> On 12/11/2018 12:59 PM, Naoto Sato wrote:
>>> Hi Roger,
>>>
>>> Thanks. I updated it as suggested (incl. test using reflection):
>>>
>>> http://cr.openjdk.java.net/~naoto/8215194/webrev.01/
>>>
>>> Naoto
>>>
>>> On 12/11/18 7:57 AM, Roger Riggs wrote:
>>>> Hi Naoto,
>>>>
>>>> Since the value changes from time to time, it would give it some 
>>>> visibility
>>>> if it were defined using a private final int  (or float)
>>>>       private final int MAP_CAPACITY = 667;
>>>>
>>>> Though I suppose the test can't use the value without using 
>>>> reflection.
>>>> But it would lower the maintenance in the long term.
>>>>
>>>> $.02, Roger
>>>>
>>>> On 12/11/2018 09:51 AM, Naoto Sato wrote:
>>>>> Hi,
>>>>>
>>>>> Please review the fix for the following issue:
>>>>>
>>>>> https://bugs.openjdk.java.net/browse/JDK-8215194
>>>>>
>>>>> The proposed fix is located at:
>>>>>
>>>>> http://cr.openjdk.java.net/~naoto/8215194/webrev.00/
>>>>>
>>>>> This one line fix is for the correctness of the initial map size 
>>>>> of Character.UnicodeBlock.
>>>>>
>>>>> Naoto
>>>>
>>
>

-- 
With kind regards,
Ivan Gerasimov