On Fri, 11 Feb 2022 15:04:03 GMT, Roger Riggs <rri...@openjdk.org> wrote:

>> XenoAmess has refreshed the contents of this pull request, and previous 
>> commits have been removed. The incremental views will show differences 
>> compared to the previous content of the PR. The pull request contains one 
>> new commit since the last revision:
>> 
>>   9072610: HashMap.putAll can cause redundant space waste
>
> 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!

-------------

PR: https://git.openjdk.java.net/jdk/pull/7431

Reply via email to