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