From d15b92d4f9db65bc01ac434240ff456ecdc5cc64 Mon Sep 17 00:00:00 2001 From: Tagir Valeev Date: Tue, 16 May 2017 12:25:35 +0700 Subject: [PATCH] KeyFMap: equals & hashCode --- .../util/keyFMap/ArrayBackedFMap.java | 67 ++++++++++++------- .../com/intellij/util/keyFMap/EmptyFMap.java | 5 ++ .../com/intellij/util/keyFMap/KeyFMap.java | 5 ++ .../intellij/util/keyFMap/MapBackedFMap.java | 14 ++-- .../intellij/util/keyFMap/OneElementFMap.java | 18 ++--- .../util/keyFMap/PairElementsFMap.java | 51 +++++++++++--- .../intellij/util/keyFMap/KeyFMapTest.java | 53 +++++++++++++-- 7 files changed, 157 insertions(+), 56 deletions(-) diff --git a/platform/util/src/com/intellij/util/keyFMap/ArrayBackedFMap.java b/platform/util/src/com/intellij/util/keyFMap/ArrayBackedFMap.java index 02ba34463e32..a17a6bb1d9f0 100644 --- a/platform/util/src/com/intellij/util/keyFMap/ArrayBackedFMap.java +++ b/platform/util/src/com/intellij/util/keyFMap/ArrayBackedFMap.java @@ -19,8 +19,11 @@ import com.intellij.openapi.util.Key; import com.intellij.util.ArrayUtil; import org.jetbrains.annotations.NotNull; +import java.util.Arrays; + public class ArrayBackedFMap implements KeyFMap { static final int ARRAY_THRESHOLD = 8; + // Invariant: keys are always sorted private final int[] keys; private final Object[] values; @@ -34,29 +37,19 @@ public class ArrayBackedFMap implements KeyFMap { public KeyFMap plus(@NotNull Key key, @NotNull V value) { int oldSize = size(); int keyCode = key.hashCode(); - int[] newKeys = null; - Object[] newValues = null; - int i; - for (i = 0; i < oldSize; i++) { - int oldKey = keys[i]; - if (keyCode == oldKey) { - if (value == values[i]) return this; - newKeys = new int[oldSize]; - newValues = new Object[oldSize]; - System.arraycopy(keys, 0, newKeys, 0, oldSize); - System.arraycopy(values, 0, newValues, 0, oldSize); - newValues[i] = value; - break; - } + int keyPos = Arrays.binarySearch(keys, keyCode); + if (keyPos >= 0) { + Object[] newValues = values.clone(); + newValues[keyPos] = value; + // Can reuse keys as it is never mutated + return new ArrayBackedFMap(keys, newValues); } - if (i == oldSize) { - if (oldSize == ARRAY_THRESHOLD) { - return new MapBackedFMap(keys, keyCode, values, value); - } - newKeys = ArrayUtil.append(keys, keyCode); - newValues = ArrayUtil.append(values, value, ArrayUtil.OBJECT_ARRAY_FACTORY); + if (oldSize < ARRAY_THRESHOLD) { + int[] newKeys = ArrayUtil.insert(keys, -keyPos - 1, keyCode); + Object[] newValues = ArrayUtil.insert(values, -keyPos - 1, value); + return new ArrayBackedFMap(newKeys, newValues); } - return new ArrayBackedFMap(newKeys, newValues); + return new MapBackedFMap(keys, keyCode, values, value); } private int size() { @@ -105,13 +98,13 @@ public class ArrayBackedFMap implements KeyFMap { @Override public String toString() { - String s = ""; + StringBuilder s = new StringBuilder("("); for (int i = 0; i < keys.length; i++) { int key = keys[i]; Object value = values[i]; - s += (s.isEmpty() ? "" : ", ") + Key.getKeyByIndex(key) + " -> " + value; + s.append((s.length() == 1) ? "" : ", ").append(Key.getKeyByIndex(key)).append(" -> ").append(value); } - return "(" + s + ")"; + return s.append(")").toString(); } @Override @@ -145,4 +138,30 @@ public class ArrayBackedFMap implements KeyFMap { return result; } + + @Override + public int hashCode() { + int hash = 0; + int length = keys.length; + for (int i = 0; i < length; i++) { + // key index is its hashcode + hash += (keys[i] ^ values[i].hashCode()); + } + return hash; + } + + @Override + public boolean equals(Object o) { + if (this == o) return true; + if (!(o instanceof ArrayBackedFMap)) return false; + + ArrayBackedFMap map = (ArrayBackedFMap)o; + if (map.size() != size()) return false; + + int length = keys.length; + for (int i = 0; i < length; i++) { + if (keys[i] != map.keys[i] || !values[i].equals(map.values[i])) return false; + } + return true; + } } diff --git a/platform/util/src/com/intellij/util/keyFMap/EmptyFMap.java b/platform/util/src/com/intellij/util/keyFMap/EmptyFMap.java index b29c5cdc240f..b8d712d091c0 100644 --- a/platform/util/src/com/intellij/util/keyFMap/EmptyFMap.java +++ b/platform/util/src/com/intellij/util/keyFMap/EmptyFMap.java @@ -56,4 +56,9 @@ class EmptyFMap implements KeyFMap { public boolean isEmpty() { return true; } + + @Override + public int hashCode() { + return 0; + } } diff --git a/platform/util/src/com/intellij/util/keyFMap/KeyFMap.java b/platform/util/src/com/intellij/util/keyFMap/KeyFMap.java index 18d5b1069899..d64f7784c5d4 100644 --- a/platform/util/src/com/intellij/util/keyFMap/KeyFMap.java +++ b/platform/util/src/com/intellij/util/keyFMap/KeyFMap.java @@ -23,6 +23,11 @@ import org.jetbrains.annotations.Nullable; * An immutable map optimized for storing few {@link Key} entries with relatively rare updates * To construct a map, start with {@link KeyFMap#EMPTY_MAP} and call {@link #plus} and {@link #minus} * + *

+ * The hashCode() contract conforms to the hashCode() contract of the {@link java.util.Map} interface: + * it's the sum of hash codes of its entries, which in turn is calculated as key.hashCode() ^ value.hashCode() + *

+ * * @author peter */ public interface KeyFMap { diff --git a/platform/util/src/com/intellij/util/keyFMap/MapBackedFMap.java b/platform/util/src/com/intellij/util/keyFMap/MapBackedFMap.java index e2b4555ec928..b5cf54de13ac 100644 --- a/platform/util/src/com/intellij/util/keyFMap/MapBackedFMap.java +++ b/platform/util/src/com/intellij/util/keyFMap/MapBackedFMap.java @@ -21,9 +21,11 @@ import gnu.trove.TIntObjectHashMap; import gnu.trove.TIntObjectProcedure; import org.jetbrains.annotations.NotNull; +import java.util.Arrays; + import static com.intellij.util.keyFMap.ArrayBackedFMap.getKeysByIndices; -class MapBackedFMap extends TIntObjectHashMap implements KeyFMap { +final class MapBackedFMap extends TIntObjectHashMap implements KeyFMap { private MapBackedFMap(@NotNull MapBackedFMap oldMap, final int exclude) { super(oldMap.size()); oldMap.forEachEntry(new TIntObjectProcedure() { @@ -73,10 +75,12 @@ class MapBackedFMap extends TIntObjectHashMap implements KeyFMap { } if (oldSize == ArrayBackedFMap.ARRAY_THRESHOLD + 1) { int[] keys = keys(); - Object[] values = getValues(); - int i = ArrayUtil.indexOf(keys, keyCode); - keys = ArrayUtil.remove(keys, i); - values = ArrayUtil.remove(values, i); + keys = ArrayUtil.remove(keys, ArrayUtil.indexOf(keys, keyCode)); + Arrays.sort(keys); + Object[] values = new Object[keys.length]; + for (int i = 0; i < keys.length; i++) { + values[i] = get(keys[i]); + } return new ArrayBackedFMap(keys, values); } return new MapBackedFMap(this, keyCode); diff --git a/platform/util/src/com/intellij/util/keyFMap/OneElementFMap.java b/platform/util/src/com/intellij/util/keyFMap/OneElementFMap.java index a38c5f0ed97d..53ce5ec90841 100644 --- a/platform/util/src/com/intellij/util/keyFMap/OneElementFMap.java +++ b/platform/util/src/com/intellij/util/keyFMap/OneElementFMap.java @@ -18,11 +18,11 @@ package com.intellij.util.keyFMap; import com.intellij.openapi.util.Key; import org.jetbrains.annotations.NotNull; -public class OneElementFMap implements KeyFMap { +public final class OneElementFMap implements KeyFMap { private final Key myKey; - private final V myValue; + private final VV myValue; - public OneElementFMap(@NotNull Key key, @NotNull V value) { + public OneElementFMap(@NotNull Key key, @NotNull VV value) { myKey = key; myValue = value; } @@ -66,7 +66,7 @@ public class OneElementFMap implements KeyFMap { return myKey; } - public V getValue() { + public VV getValue() { return myValue; } @@ -76,17 +76,11 @@ public class OneElementFMap implements KeyFMap { if (!(o instanceof OneElementFMap)) return false; OneElementFMap map = (OneElementFMap)o; - - if (myKey != map.myKey) return false; - if (!myValue.equals(map.myValue)) return false; - - return true; + return myKey == map.myKey && myValue.equals(map.myValue); } @Override public int hashCode() { - int result = myKey.hashCode(); - result = 31 * result + myValue.hashCode(); - return result; + return myKey.hashCode() ^ myValue.hashCode(); } } diff --git a/platform/util/src/com/intellij/util/keyFMap/PairElementsFMap.java b/platform/util/src/com/intellij/util/keyFMap/PairElementsFMap.java index ec1da5b4a59f..b918d9a2d4ba 100644 --- a/platform/util/src/com/intellij/util/keyFMap/PairElementsFMap.java +++ b/platform/util/src/com/intellij/util/keyFMap/PairElementsFMap.java @@ -18,18 +18,27 @@ package com.intellij.util.keyFMap; import com.intellij.openapi.util.Key; import org.jetbrains.annotations.NotNull; -public class PairElementsFMap implements KeyFMap { - private final Key key1; - private final Key key2; - private final Object value1; - private final Object value2; +public final class PairElementsFMap implements KeyFMap { + // invariant: key1.hashCode() < key2.hashCode() + private final @NotNull Key key1; + private final @NotNull Key key2; + private final @NotNull Object value1; + private final @NotNull Object value2; PairElementsFMap(@NotNull Key key1, @NotNull Object value1, @NotNull Key key2, @NotNull Object value2) { - this.key1 = key1; - this.value1 = value1; - this.key2 = key2; - this.value2 = value2; assert key1 != key2; + // Key hashCodes are unique and ordered + if(key1.hashCode() < key2.hashCode()) { + this.key1 = key1; + this.value1 = value1; + this.key2 = key2; + this.value2 = value2; + } else { + this.key1 = key2; + this.value1 = value2; + this.key2 = key1; + this.value2 = value1; + } } @NotNull @@ -37,6 +46,11 @@ public class PairElementsFMap implements KeyFMap { public KeyFMap plus(@NotNull Key key, @NotNull V value) { if (key == key1) return new PairElementsFMap(key, value, key2, value2); if (key == key2) return new PairElementsFMap(key, value, key1, value1); + if(key.hashCode() < key1.hashCode()) { + return new ArrayBackedFMap(new int[]{key.hashCode(), key1.hashCode(), key2.hashCode()}, new Object[]{value, value1, value2}); + } else if(key.hashCode() < key2.hashCode()) { + return new ArrayBackedFMap(new int[]{key1.hashCode(), key.hashCode(), key2.hashCode()}, new Object[]{value1, value, value2}); + } return new ArrayBackedFMap(new int[]{key1.hashCode(), key2.hashCode(), key.hashCode()}, new Object[]{value1, value2, value}); } @@ -70,19 +84,38 @@ public class PairElementsFMap implements KeyFMap { return false; } + @NotNull public Key getKey1() { return key1; } + @NotNull public Key getKey2() { return key2; } + @NotNull public Object getValue1() { return value1; } + @NotNull public Object getValue2() { return value2; } + + @Override + public int hashCode() { + return (key1.hashCode() ^ value1.hashCode()) + (key2.hashCode() ^ value2.hashCode()); + } + + @Override + public boolean equals(Object o) { + if (this == o) return true; + if (!(o instanceof PairElementsFMap)) return false; + + PairElementsFMap map = (PairElementsFMap)o; + + return key1 == map.key1 && value1.equals(map.value1) && key2 == map.key2 && value2.equals(map.value2); + } } diff --git a/platform/util/testSrc/com/intellij/util/keyFMap/KeyFMapTest.java b/platform/util/testSrc/com/intellij/util/keyFMap/KeyFMapTest.java index 232c223af9ba..3b7850a5daf5 100644 --- a/platform/util/testSrc/com/intellij/util/keyFMap/KeyFMapTest.java +++ b/platform/util/testSrc/com/intellij/util/keyFMap/KeyFMapTest.java @@ -20,10 +20,15 @@ import com.intellij.util.ArrayUtil; import com.intellij.util.containers.ContainerUtil; import junit.framework.TestCase; -import java.util.List; +import java.util.*; +import java.util.stream.Collectors; +import java.util.stream.IntStream; public class KeyFMapTest extends TestCase { - private static KeyFMap createKeyFMap(List keys, List values) { + private static final List> KEYS = + IntStream.range(0, 20).mapToObj(i -> Key.create("Key#"+i)).collect(Collectors.toList()); + + private static KeyFMap createKeyFMap(List> keys, List values) { KeyFMap map = KeyFMap.EMPTY_MAP; for (int i = 0; i < keys.size(); i++) { @@ -34,24 +39,60 @@ public class KeyFMapTest extends TestCase { } private static void doTestGetKeys(int size) { - List keys = ContainerUtil.newArrayList(); List values = ContainerUtil.newArrayList(); for (int i = 0; i < size; i++) { - keys.add(Key.create("Key#" + i)); values.add("Value#" + i); } - KeyFMap map = createKeyFMap(keys, values); + KeyFMap map = createKeyFMap(KEYS.subList(0, size), values); Key[] actualKeys = map.getKeys(); assertEquals(size, actualKeys.length); - for (Key key : keys) { + for (Key key : KEYS.subList(0, size)) { assertTrue("Key not found: " + key, ArrayUtil.contains(key, actualKeys)); } } + public void testHashCodeEquals() { + Random r = new Random(1); + for(int n=0; n<15; n++) { + Map, Object> hashMap = new HashMap<>(); + KeyFMap fMap = KeyFMap.EMPTY_MAP; + + for(int i=0; i key; + while (true) { + key = KEYS.get(r.nextInt(KEYS.size())); + if (hashMap.putIfAbsent(key, value) == null) break; + } + KeyFMap newFMap = fMap.plus(key, value); + assertNotSame(fMap, newFMap); // new key is added: must be not same + fMap = newFMap; + } + assertEquals(hashMap + ":" + fMap, hashMap.hashCode(), fMap.hashCode()); + KeyFMap fMap2 = KeyFMap.EMPTY_MAP; + for (Map.Entry, Object> entry : hashMap.entrySet()) { + fMap2 = fMap2.plus(entry.getKey(), entry.getValue()); + } + assertEquals(fMap, fMap2); + Iterator> iterator = hashMap.keySet().iterator(); + while(iterator.hasNext()) { + Key key = iterator.next(); + iterator.remove(); + assertEquals(fMap, fMap2); + fMap = fMap.minus(key); + assertFalse(fMap.equals(fMap2)); + fMap2 = fMap2.minus(key); + assertEquals(fMap, fMap2); + assertEquals(fMap.hashCode(), fMap2.hashCode()); + } + assertTrue(fMap.isEmpty()); + } + } + public void testGetKeysOnEmptyFMap() { doTestGetKeys(0); }