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

[Mike Duigou]

Looks pretty good. I like the additional comments as well.

Could you document the return conditions of resize()?

A "since we're there already" suggestion for readObject:

if (size < 0)
  throw new InvalidObjectException("Illegal mappings count: " + size);

Mike

On Jul 8 2014, at 13:07 , Martin Buchholz <martinrb at google.com> wrote:

> I updated my webrev and it is again "feature-complete".
> http://cr.openjdk.java.net/~martin/webrevs/openjdk9/IdentityHashMap-capacity/
> (old webrev at
> http://cr.openjdk.java.net/~martin/webrevs/openjdk9/IdentityHashMap-capacity.0/
> )
> 
> This incorporates Peter's idea of making resize return a boolean, keeps the
> map unchanged if resize throws, moves the check for capacity exceeded into
> resize, and minimizes bytecode in put().  I'm happy with this (except for
> degraded behavior near MAX_CAPACITY).
> 
> 
> 
> 
> On Tue, Jul 8, 2014 at 8:06 AM, Peter Levart <peter.levart at gmail.com> wrote:
> 
>> On 07/08/2014 03:00 PM, Ivan Gerasimov wrote:
>> 
>>> I took your latest version of the patch and modified it a little:
>>>> 
>>>> http://cr.openjdk.java.net/~plevart/jdk9-dev/IdentityHashMap/webrev.01/
>>>> 
>>>> 
>>> But isn't it post-insert-resize vs pre-insert-resize problem Doug
>>> mentioned above?
>>> I've tested a similar fix and it showed slow down of the put() operation.
>>> 
>> Hi Ivan,
>> 
>> Might be that it has to do with # of bytecodes in the method and in-lining
>> threshold. I modified it once more, to make put() method as short as
>> possible:
>> 
>> http://cr.openjdk.java.net/~plevart/jdk9-dev/IdentityHashMap/webrev.05/
>> 
>> With this, I ran the following JMH benchmark:
>> 
>> @State(Scope.Thread)
>> public class IHMBench {
>> 
>>    Map<Object, Object> map = new IdentityHashMap<Object, Object>();
>> 
>>    @Benchmark
>>    public void putNewObject(Blackhole bh) {
>>        Object o = new Object();
>>        bh.consume(map.put(o, o));
>>        if (map.size() > 100000) {
>>            map = new IdentityHashMap<Object, Object>();
>>        }
>>    }
>> }
>> 
>> I get the following results on my i7/Linux using:
>> 
>> java -Xmx4G -Xms4G -XX:+UseParallelGC -jar benchmarks.jar -f 0 -i 10 -wi 8
>> -gc 1 -t 1
>> 
>> Original:
>> 
>> Benchmark                     Mode   Samples        Score  Score error
>> Units
>> j.t.IHMBench.putNewObject    thrpt        10 13088296.198 403446.449
>> ops/s
>> 
>> Patched:
>> 
>> Benchmark                     Mode   Samples        Score  Score error
>> Units
>> j.t.IHMBench.putNewObject    thrpt        10 13180594.537 282047.154
>> ops/s
>> 
>> 
>> Can you run your test with webrev.05 and see what you get ?
>> 
>> Regards, Peter
>> 
>>