RFR: 8166365: Small immutable collections should provide optimized implementations when possible

[Paul Sandoz]

Hi,

ImmutableCollections
—

Did you forget or intentionally omit overriding the hashCode for List2 and ListN?


 481         @Override
 482         public int hashCode() {
 483             int h = 0;
 484             for (E e : elements) {
 485                 if (e != null) {
 486                     h += e.hashCode();
 487                 }
 488             }
 489             return h;
 490         }

No need for the null check of e. If you override hashCode should you override equals as well?


 617         @Override
 618         public int hashCode() {
 619             return k0.hashCode() ^ v0.hashCode();
 620         }

That does not conform to the specification of Map.hashCode:

/**
 * Returns the hash code value for this map.  The hash code of a map is
 * defined to be the sum of the hash codes of each entry in the map's
 * {@code entrySet()} view.  This ensures that {@code m1.equals(m2)}

(Your tests might be lucky with the summing of hash codes with distinct ranges of bits set?)


 669         @Override
 670         public boolean containsValue(Object o) {
 671             for (int i = 1; i < table.length; i += 2) {
 672                 Object v = table[i];
 673                 if (v != null && o.equals(v)) {
 674                     return true;
 675                 }
 676             }
 677             return false;
 678         }

No need for the null check for v, can pass table[i] directly to o.equals

Paul.

> On 10 Jan 2017, at 03:50, Claes Redestad <claes.redestad at oracle.com> wrote:
> 
> Hi,
> 
> please review this change to improve startup/warmup characteristics
> and performance of the immutable collection factories:
> 
> Bug: https://bugs.openjdk.java.net/browse/JDK-8166365
> Webrev: http://cr.openjdk.java.net/~redestad/8166365/webrev.01/
> 
> While filed as an RFE, and a more thorough examination of the hierarchy
> of the ImmutableCollection classes has been discussed[1], this more
> limited patch resolves some immediate issues related to module system
> bootstrap and is therefore motivated to push to 9.
> 
> Thanks!
> 
> /Claes
> 
> [1] we should likely not inherit Iterator classes that check for
> co-modification etc,