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

2018-12-11 Thread Naoto Sato
Hi Claes, I don't see any particular reason for that. In fact, I was wondering the same thing, but just left it as it was. Since I've already pushed the changeset, I will fix it on the next occasion. Naoto On 12/11/18 2:31 PM, Claes Redestad wrote: Hi Naoto, while the here fix looks good,

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

2018-12-11 Thread Claes Redestad
Hi Naoto, while the here fix looks good, I can't help wondering why the map isn't final? /Claes On 2018-12-11 19:59, Naoto Sato wrote: Hi Ivan, Thank you for the review. On 12/11/18 10:49 AM, Ivan Gerasimov wrote: OptimalCapacity.ofHashMap uses the initialCapacity to verify that a HashMap

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

2018-12-11 Thread Roger Riggs
Looks fine. Thanks, Roger On 12/11/2018 01:33 PM, 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:

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

2018-12-11 Thread Naoto Sato
Hi Ivan, Thank you for the review. On 12/11/18 10:49 AM, Ivan Gerasimov wrote: 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

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

2018-12-11 Thread 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 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

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

2018-12-11 Thread Naoto Sato
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

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

2018-12-11 Thread Roger Riggs
Hi Naoto, Looks ok, I intended only the number of elements to be defined as a constant. The other factors can be hard coded. 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.

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

2018-12-11 Thread Naoto Sato
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

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

2018-12-11 Thread Rachna Goel
Hi Naoto, Thanks for fixing this. Your fix looks good to me. Thanks, Rachna On 12/11/18 8:21 PM, 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:

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

2018-12-11 Thread Roger Riggs
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

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

2018-12-11 Thread Naoto Sato
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