> On 16 May 2015, at 01:59, Ivan Gerasimov <ivan.gerasi...@oracle.com> wrote:
>
> On 16.05.2015 2:18, Martin Buchholz wrote:
>> I don't think you're taking the load factor into account.
> Hm. You're right, HashMap's constructor expects the capacity as the argument.
> I was confused by IdentityHashMap, whose constructor expects the maximum
> number of elements to be inserted.
>
>> https://code.google.com/p/guava-libraries/source/browse/guava/src/com/google/common/collect/Maps.java?r=f8144b4df7eec5f1c9e6942d8d5ef734a8767fd9#110
>> You want to check (via reflection) that the size of the backing array is
>> unchanged if you copy the map with new HashMap(map).
>>
> Sorry, I didn't get it. How could we detect that the initial capacity wasn't
> big enough, if we hadn't stored it?
>
>> I wouldn't bother defining the constant.
>>
> I only need it in the regression test, to check whether it was sufficient.
>
> Here's the updated webrev:
> http://cr.openjdk.java.net/~igerasim/8080535/01/webrev/
> <http://cr.openjdk.java.net/~igerasim/8080535/01/webrev/>
This looks much better. Trivially I’d calculate the initial size like:
510 / 0.75 + 1.0f ( plus 1 )
… but your regression test should prove that the plus one is not needed.
Maybe a comment that the 0.75 is the default load factor from HashMap.
The constant could be removed and the ‘510’ be used directly in the test. Since
the test is whitebox.
The test is fine, but could be a little less obscure if it looked at the table
size, rather than the equality. But what you have is fine.
-Chris.
>
> Comments, suggestions are welcome!
>
> Sincerely yours,
> Ivan
>
>> On Fri, May 15, 2015 at 4:01 PM, Ivan Gerasimov <ivan.gerasi...@oracle.com
>> <mailto:ivan.gerasi...@oracle.com>> wrote:
>>
>> Hello!
>>
>> When constructing the map, the expected size is specified to be
>> 256, but then 510 elements are inserted.
>> A new whitebox test is provided, so the next time the number of
>> entries grows, the expected size will not be forgotten.
>>
>> Would you please help review this fix?
>>
>> BUGURL: https://bugs.openjdk.java.net/browse/JDK-8080535
>> WEBREV: http://cr.openjdk.java.net/~igerasim/8080535/00/webrev/
>> <http://cr.openjdk.java.net/%7Eigerasim/8080535/00/webrev/>
>>
>> Sincerely yours,
>> Ivan
>>
>>
>
