RFR: 8281631: HashMap.putAll can cause redundant space waste [v3]
XenoAmess
duke at openjdk.java.net
Fri Feb 11 17:10:44 UTC 2022
On Fri, 11 Feb 2022 16:32:29 GMT, Roger Riggs <rriggs at openjdk.org> wrote:
>> @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.
@RogerRiggs
so you recommend `(int) Math.min(((m.size() * 4L + 2L) / 3L), Integer.MAX_VALUE)`? OK then, changed it.
-------------
PR: https://git.openjdk.java.net/jdk/pull/7431
More information about the core-libs-dev
mailing list