Thanks, will add doc.
http://gwt-code-reviews.appspot.com/1384804/diff/1/dev/core/src/com/google/gwt/dev/util/collect/HashMap.java File dev/core/src/com/google/gwt/dev/util/collect/HashMap.java (right): http://gwt-code-reviews.appspot.com/1384804/diff/1/dev/core/src/com/google/gwt/dev/util/collect/HashMap.java#newcode54 dev/core/src/com/google/gwt/dev/util/collect/HashMap.java:54: throw new ConcurrentModificationException(); Hehe, good question. :) All our tests pass, at least. http://gwt-code-reviews.appspot.com/1384804/diff/1/dev/core/src/com/google/gwt/dev/util/collect/HashMap.java#newcode111 dev/core/src/com/google/gwt/dev/util/collect/HashMap.java:111: HashMap.this.ensureSizeFor(Math.max(size(), c.size())); On 2011/03/16 03:33:45, zundel wrote:
At first glance, this change doesn't make sense = couldn't we end up
with size()
+ c.size() items in the worst case? (c is all new entries). OK, so we
don't
know until we see if the entries are actually in the map already. But
do we
really need this here? Is addAll() going to end up calling
EntrySet.add()? We
don't worry about ensureSizeFor there - we could just let
HashMap.this.put()
worry about it. My guess is that this is a heuristic to make sure we
don't
resize the table multiple times in the middle of addAll(). Comment
please. Yes, the original reason for size() + c.size() was to prevent having to do a bunch of rehashes during an addAll(). However, I realized when debugging this that we had a map with 24 entries and 64 capacity. Since a big design goal for these classes is memory efficiency, it seemed better to err on the side of less memory. It's true we could end up with size() + c.size() in the worst case. But it works out not so bad, actually. In the worst case, where size() and c.size() are roughly equal and the sets are completely disjoint, you might do 1 initial rehash and then 1 additional rehash down the road. But even this is an edge case that requires you to get unlucky on both boundaries. Most of the time, you'll do either 1 initial rehash or 1 down the road. I'll add a comment, thanks. http://gwt-code-reviews.appspot.com/1384804/diff/1/dev/core/src/com/google/gwt/dev/util/collect/HashMap.java#newcode482 dev/core/src/com/google/gwt/dev/util/collect/HashMap.java:482: HashMap.this.ensureSizeFor(Math.max(size(), m.size())); I think I'll just factor out a method, and javadoc it. http://gwt-code-reviews.appspot.com/1384804/ -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
