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.lev...@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.gerasi...@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 >> > > > >
