RFR: 8281631: HashMap.putAll can cause redundant space waste [v3]

[XenoAmess]

On Fri, 11 Feb 2022 15:04:03 GMT, Roger Riggs <rriggs at 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