Thank you Martin for the enhancement!

It's a good idea to get rid of threshold variable!
When the table grows large, it will start to try to resize the table on every single put(). Though it shouldn't matter much, as in such situation everything is already slow.


On 08.07.2014 4:44, Martin Buchholz wrote:

On Mon, Jul 7, 2014 at 9:41 AM, Martin Buchholz <marti...@google.com <mailto:marti...@google.com>> wrote:

    I'd like to offer an alternative version of this change.  webrev
    coming soon.


Here's the promised webrev.
http://cr.openjdk.java.net/~martin/webrevs/openjdk9/IdentityHashMap-capacity/ <http://cr.openjdk.java.net/%7Emartin/webrevs/openjdk9/IdentityHashMap-capacity/>

- Fixes a typo in the javadoc.
- removes the "threshold" field (WAT, a cache to avoid multiplying by 3?)
- optimizes 3*x into x + x << 1

Unfortunately, x + x << 1 causes the same overflow bug as 3*x:
(int)(1431655766 + 1431655766 << 1) == 2

this can happen in capacity() and, theoretically, in putAll().

I propose to leave the check
         if (expectedMaxSize > MAXIMUM_CAPACITY / 3)
             return MAXIMUM_CAPACITY;
which is overflow resistant.

In putAll, I guess, something like this can be used:
int len = table.length;
if (n > len || n > len - n || n > len - (n << 1))
    resize(capacity(n));


- adds more test assertions
- removes the unusual 4/3 slack space for expansion on deserialization.
TODO: the test should be testng-ified, I think.



Reply via email to