On Fri, 11 Feb 2022 15:48:44 GMT, XenoAmess <d...@openjdk.java.net> wrote:
>> src/java.base/share/classes/java/util/WeakHashMap.java line 247:
>>
>>> 245: */
>>> 246: private static int calculateHashMapCapacity(int size){
>>> 247: return size + (size + 2) / 3;
>>
>> Consider integer overflow; if size were Integer.MAX_VALUE / 2; the
>> computation would overflow.
>> The negative result would eventually throw an exception. Using Long for the
>> computation would avoid that and keep the expression simple.
>
> @RogerRiggs
>
> Hi. I added a overflow check.
>
> The check makes sure it cannot overflow now.
>
> I wrote a test to show this overflow check be correct.
>
>
> public class A {
>
> /**
> * use for calculate HashMap capacity, using default load factor 0.75
> *
> * @param size size
> * @return HashMap capacity under default load factor
> */
> private static int calculateHashMapCapacity(int size) {
> if (size > (int) (Integer.MAX_VALUE * 0.75)) {
> return Integer.MAX_VALUE;
> }
> return size + (size + 2) / 3;
> }
>
> public static void main(String[] args) {
> for (int i = 1; i < Integer.MAX_VALUE ; ++i) {
> if (calculateHashMapCapacity(i) <= 0) {
> System.err.println(i);
> return;
> }
> }
> }
> }
>
>
>
> I also see the compiled result to make sure the `(int) (Integer.MAX_VALUE *
> 0.75)` calculation is made at compiling but not runtime, which will not make
> this function too much slower.
>
> please review again, thanks!
Performance is a lesser issue. Given all of the other computation that occurs
to populate the map, this computation is in the noise. It is also likely that
with more instructions to fetch and execute and a possible branch, the integer
check is slower.
With current hardware, the long divide dominates the cost. Addition is almost
free.
Code maintainability is more important; keep the source code simple and concise.
-------------
PR: https://git.openjdk.java.net/jdk/pull/7431
