RFR: [6904367]: (coll) IdentityHashMap is resized before exceeding the expected maximum size

[Martin Buchholz]

OK, I think we're down to the smallest possible bug:

if size == MAX_CAPACITY - 1 and we putAll a concurrent map with greater
size, but the concurrent map shrinks while we're adding it, and no actual
elements get added to the IHM (but each element put takes ~  2**28 probes),
then an ISE is thrown when it was not strictly necessary.

Meanwhile, I'm actually wishing we could throw at something like 80% full...

Relatedly: It occurs to me that it's traditional in java.util to throw OOME
instead of ISE for capacity exceeded.


On Wed, Jul 9, 2014 at 12:33 AM, Peter Levart <peter.levart at gmail.com>
wrote:

>
> On 07/09/2014 09:23 AM, Peter Levart wrote:
>
>
> On 07/09/2014 02:46 AM, Martin Buchholz wrote:
>
> Let me understand - you're worried that when size is MAX_CAPACITY - 1,
> then a call to putAll that does not actually add any elements might throw?
>
>
> This is not possible, because resize() is only called from putAll(map) if
> argument map.size() > this.size. At least one element will be added to the
> map and it's correct to throw if current size == MAX_CAPACITY - 1.
>
>
> ...at least if the argument map obeys the rules of Map contract. Even if
> it's a HashMap or another IdentityHashMap, it should not contain the same
> INSTANCE of the key in two or more mappings, should not "overshoot"
> reporting it's size() and should be stable during the whole putAll()
> operation... So calling IHM.addAll() with a live changing ConcurrentHashMap
> is not advisable. Even then, addAll() could only throw, because at some
> point the argument's size indicated that IHM could reach it's max. capacity.
>
> Peter
>
>
>
>   Well, I'm having a hard time considering that corner case a bug, given
> how unusable the map is at this point.
>
>
> It seems even this corner case is not present.
>
>
>  Your suggested fix of returning immediately in case of no resize would
> cause put to succeed and reach size == MAX_CAPACITY, which you were trying
> to prevent.
>
>
> That's not possible either, since resize() is always called from put()
> with current table.length, which makes newLength == 2 * oldLength,
> therefore (oldLength >= newLength) will never succeed in this case.
>
> Peter
>
>
>
> On Tue, Jul 8, 2014 at 5:25 PM, Ivan Gerasimov <ivan.gerasimov at oracle.com>
> wrote:
>
>>
>> On 09.07.2014 3:20, Martin Buchholz wrote:
>>
>> I agree with Peter that we do not need to increment modCount on resize.
>>
>>  My latest webrev is again "done".
>>
>>  Ivan, feel free to take over.
>>
>>
>>  Yes, thanks! The fix is much much better now.
>>
>> I think I see yet another very minor glitch, though.
>> If the table is already of size 2 * MAXIMUM_CAPACITY, and we're merging
>> in another map, which has more elements with putAll(), resize() will be
>> called and it will throw, even though all the elements could fit into the
>> map due to the key duplicates.
>>
>> To avoid this the following check should be done first in resize():
>>
>>  469         if (oldLength >= newLength) 470             return false;
>>
>>  Sincerely yours,
>> Ivan
>>
>
>
>
>