From 3f96fa8ce9741e500a7bd2435741d37eb5b9ecbf Mon Sep 17 00:00:00 2001 From: Tagir Valeev Date: Wed, 17 May 2017 11:04:39 +0700 Subject: [PATCH] KeyFMap improvements (IDEA-CR-21117) 1. JavaDoc 2. toString() unified 3. equalsByReference, identityHashCode, size 4. implementations are package-private, getters removed 5. plus() can return self if new value is the same as existing one --- .../vfs/newvfs/impl/UserDataInterner.java | 57 +--------------- .../util/keyFMap/ArrayBackedFMap.java | 59 +++++++++++----- .../com/intellij/util/keyFMap/EmptyFMap.java | 19 +++++- .../com/intellij/util/keyFMap/KeyFMap.java | 67 +++++++++++++++++-- .../intellij/util/keyFMap/MapBackedFMap.java | 26 +++++++ .../intellij/util/keyFMap/OneElementFMap.java | 35 +++++++--- .../util/keyFMap/PairElementsFMap.java | 54 ++++++++------- .../intellij/util/keyFMap/KeyFMapTest.java | 18 +++++ 8 files changed, 222 insertions(+), 113 deletions(-) diff --git a/platform/platform-impl/src/com/intellij/openapi/vfs/newvfs/impl/UserDataInterner.java b/platform/platform-impl/src/com/intellij/openapi/vfs/newvfs/impl/UserDataInterner.java index ebeed31cfc62..0f12184740ad 100644 --- a/platform/platform-impl/src/com/intellij/openapi/vfs/newvfs/impl/UserDataInterner.java +++ b/platform/platform-impl/src/com/intellij/openapi/vfs/newvfs/impl/UserDataInterner.java @@ -17,14 +17,10 @@ package com.intellij.openapi.vfs.newvfs.impl; import com.intellij.reference.SoftReference; import com.intellij.util.containers.hash.LinkedHashMap; -import com.intellij.util.keyFMap.ArrayBackedFMap; import com.intellij.util.keyFMap.KeyFMap; -import com.intellij.util.keyFMap.OneElementFMap; -import com.intellij.util.keyFMap.PairElementsFMap; import org.jetbrains.annotations.NotNull; import java.lang.ref.WeakReference; -import java.util.Arrays; import java.util.Map; /** @@ -53,9 +49,7 @@ class UserDataInterner { } private static boolean shouldIntern(@NotNull KeyFMap map) { - return map instanceof OneElementFMap || - map instanceof PairElementsFMap || - map instanceof ArrayBackedFMap && ((ArrayBackedFMap)map).getKeyIds().length <= 5; + return map.size() <= 5; } } @@ -64,26 +58,7 @@ class MapReference extends WeakReference { MapReference(KeyFMap referent) { super(referent); - myHash = computeHashCode(referent); - } - - private static int computeHashCode(KeyFMap object) { - if (object instanceof OneElementFMap) { - return ((OneElementFMap)object).getKey().hashCode() * 31 + System.identityHashCode(((OneElementFMap)object).getValue()); - } - if (object instanceof PairElementsFMap) { - PairElementsFMap map = (PairElementsFMap)object; - return (map.getKey1().hashCode() * 31 + map.getKey2().hashCode()) * 31 + - System.identityHashCode(map.getValue1()) + System.identityHashCode(map.getValue2()); - } - if (object instanceof ArrayBackedFMap) { - int hc = Arrays.hashCode(((ArrayBackedFMap)object).getKeyIds()); - for (Object o : ((ArrayBackedFMap)object).getValues()) { - hc = hc * 31 + System.identityHashCode(o); - } - return hc; - } - return 0; + myHash = referent.identityHashCode(); } @Override @@ -99,32 +74,6 @@ class MapReference extends WeakReference { KeyFMap o1 = get(); KeyFMap o2 = ((MapReference)obj).get(); if (o1 == null || o2 == null) return false; - - if (o1 instanceof OneElementFMap && o2 instanceof OneElementFMap) { - OneElementFMap m1 = (OneElementFMap)o1; - OneElementFMap m2 = (OneElementFMap)o2; - return m1.getKey() == m2.getKey() && m1.getValue() == m2.getValue(); - } - if (o1 instanceof PairElementsFMap && o2 instanceof PairElementsFMap) { - PairElementsFMap m1 = (PairElementsFMap)o1; - PairElementsFMap m2 = (PairElementsFMap)o2; - return m1.getKey1() == m2.getKey1() && m1.getKey2() == m2.getKey2() && - m1.getValue1() == m2.getValue1() && m1.getValue2() == m2.getValue2(); - } - if (o1 instanceof ArrayBackedFMap && o2 instanceof ArrayBackedFMap) { - ArrayBackedFMap m1 = (ArrayBackedFMap)o1; - ArrayBackedFMap m2 = (ArrayBackedFMap)o2; - return Arrays.equals(m1.getKeyIds(), m2.getKeyIds()) && containSameElements(m1.getValues(), m2.getValues()); - } - return false; - } - - private static boolean containSameElements(Object[] v1, Object[] v2) { - if (v1.length != v2.length) return false; - - for (int i = 0; i < v1.length; i++) { - if (v1[i] != v2[i]) return false; - } - return true; + return o1.equalsByReference(o2); } } diff --git a/platform/util/src/com/intellij/util/keyFMap/ArrayBackedFMap.java b/platform/util/src/com/intellij/util/keyFMap/ArrayBackedFMap.java index a17a6bb1d9f0..fb43ac6a25aa 100644 --- a/platform/util/src/com/intellij/util/keyFMap/ArrayBackedFMap.java +++ b/platform/util/src/com/intellij/util/keyFMap/ArrayBackedFMap.java @@ -39,6 +39,9 @@ public class ArrayBackedFMap implements KeyFMap { int keyCode = key.hashCode(); int keyPos = Arrays.binarySearch(keys, keyCode); if (keyPos >= 0) { + if (values[keyPos] == value) { + return this; + } Object[] newValues = values.clone(); newValues[keyPos] = value; // Can reuse keys as it is never mutated @@ -52,7 +55,7 @@ public class ArrayBackedFMap implements KeyFMap { return new MapBackedFMap(keys, keyCode, values, value); } - private int size() { + public int size() { return keys.length; } @@ -69,9 +72,12 @@ public class ArrayBackedFMap implements KeyFMap { int i2 = 3 - (i+2)/2; Key key1 = Key.getKeyByIndex(keys[i1]); Key key2 = Key.getKeyByIndex(keys[i2]); - if (key1 == null && key2 == null) return EMPTY_MAP; - if (key1 == null) return new OneElementFMap(key2, values[i2]); - if (key2 == null) return new OneElementFMap(key1, values[i1]); + if (key1 == null) { + throw new IllegalStateException("Key not found: #" + keys[i1]); + } + if (key2 == null) { + throw new IllegalStateException("Key not found: #" + keys[i2]); + } return new PairElementsFMap(key1, values[i1], key2, values[i2]); } int[] newKeys = ArrayUtil.remove(keys, i); @@ -98,13 +104,13 @@ public class ArrayBackedFMap implements KeyFMap { @Override public String toString() { - StringBuilder s = new StringBuilder("("); + StringBuilder s = new StringBuilder("{"); for (int i = 0; i < keys.length; i++) { int key = keys[i]; Object value = values[i]; - s.append((s.length() == 1) ? "" : ", ").append(Key.getKeyByIndex(key)).append(" -> ").append(value); + s.append((s.length() == 1) ? "" : ", ").append(Key.getKeyByIndex(key)).append("=").append(value); } - return s.append(")").toString(); + return s.append("}").toString(); } @Override @@ -112,9 +118,14 @@ public class ArrayBackedFMap implements KeyFMap { return false; } - @NotNull - public int[] getKeyIds() { - return keys; + @Override + public int identityHashCode() { + int hash = 0; + for (int i = 0; i < keys.length; i++) { + hash = hash * 31 + keys[i]; + hash = hash * 31 + System.identityHashCode(values[i]); + } + return hash; } @NotNull @@ -123,17 +134,16 @@ public class ArrayBackedFMap implements KeyFMap { return getKeysByIndices(keys); } - @NotNull - public Object[] getValues() { - return values; - } - @NotNull static Key[] getKeysByIndices(int[] indexes) { Key[] result = new Key[indexes.length]; - for (int i =0; i < indexes.length; i++) { - result[i] = Key.getKeyByIndex(indexes[i]); + for (int i = 0; i < indexes.length; i++) { + Key key = Key.getKeyByIndex(indexes[i]); + if (key == null) { + throw new IllegalStateException("Key not found: #" + indexes[i]); + } + result[i] = key; } return result; @@ -164,4 +174,19 @@ public class ArrayBackedFMap implements KeyFMap { } return true; } + + @Override + public boolean equalsByReference(KeyFMap 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] != 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 b8d712d091c0..384988287ff4 100644 --- a/platform/util/src/com/intellij/util/keyFMap/EmptyFMap.java +++ b/platform/util/src/com/intellij/util/keyFMap/EmptyFMap.java @@ -27,7 +27,7 @@ class EmptyFMap implements KeyFMap { @NotNull @Override public KeyFMap plus(@NotNull Key key, @NotNull V value) { - return new OneElementFMap(key, value); + return new OneElementFMap(key, value); } @NotNull @@ -41,6 +41,11 @@ class EmptyFMap implements KeyFMap { return null; } + @Override + public int size() { + return 0; + } + @NotNull @Override public Key[] getKeys() { @@ -49,7 +54,7 @@ class EmptyFMap implements KeyFMap { @Override public String toString() { - return ""; + return "{}"; } @Override @@ -57,6 +62,16 @@ class EmptyFMap implements KeyFMap { return true; } + @Override + public int identityHashCode() { + return 0; + } + + @Override + public boolean equalsByReference(KeyFMap other) { + return other == this; + } + @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 d64f7784c5d4..0bdd9c50ceba 100644 --- a/platform/util/src/com/intellij/util/keyFMap/KeyFMap.java +++ b/platform/util/src/com/intellij/util/keyFMap/KeyFMap.java @@ -20,31 +20,88 @@ import org.jetbrains.annotations.NotNull; 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} + * 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() + * it's the sum of hash codes of its entries, which in turn is calculated as {@code key.hashCode() ^ value.hashCode()} + *

+ * + *

+ * Note that keys are not always strongly referenced, but if key is used in the map, it must exist + * during the map lifecycle. A good practice is to use keys declared in static final fields. *

* * @author peter */ public interface KeyFMap { + /** + * An empty {@code KeyFMap}. No additional instances of empty {@code KeyFMap} should be created as they are indistinguishable. + */ KeyFMap EMPTY_MAP = new EmptyFMap(); + /** + * Returns a {@code KeyFMap} which consists of the same elements as this {@code KeyFMap}, but + * the key {@code key} is associated with the supplied {@code value}. May return itself if the {@code key} + * is already associated with the supplied {@code value}. + * + * @param key a key to add or replace + * @param value a value to be associated with the key + * @param a type of the value + * @return an updated {@code KeyFMap} (this or newly created) + */ @NotNull KeyFMap plus(@NotNull Key key, @NotNull V value); + + /** + * Returns a KeyFMap which consists of the same elements as this KeyFMap, except + * the supplied key which is removed. May return itself if the supplied key is absent + * in this map. + * + * @param key a key to remove + * @return an updated KeyFMap + */ @NotNull KeyFMap minus(@NotNull Key key); + /** + * Returns a value associated with given key in this {@code KeyFMap}, or null if no value is associated. + * Note that unlike {@link java.util.HashMap} {@code KeyFMap} cannot hold null values. + * + * @param key a key to get the value associated with + * @param a type of the value + * @return a value associated with given key or null if there's not such value + */ @Nullable V get(@NotNull Key key); + /** + * @return size (number of keys) of this {@code KeyFMap}. + */ + int size(); + + /** + * @return an array of all keys present in this {@code KeyFMap}, in no particular order. The length of the array equals to KeyFMap size. + */ @NotNull Key[] getKeys(); - String toString(); - + /** + * @return true if this {@code KeyFMap} is empty. + */ boolean isEmpty(); + + /** + * @return a hashCode function for this map which uses {@link System#identityHashCode(Object)} for values. + */ + int identityHashCode(); + + /** + * Checks if other {@code KeyFMap} equals to this, assuming reference equality for the values + * + * @param other {@code KeyFMap} to compare with + * @return true if other map equals to this map. + */ + boolean equalsByReference(KeyFMap other); } diff --git a/platform/util/src/com/intellij/util/keyFMap/MapBackedFMap.java b/platform/util/src/com/intellij/util/keyFMap/MapBackedFMap.java index b5cf54de13ac..21d6e0937d9f 100644 --- a/platform/util/src/com/intellij/util/keyFMap/MapBackedFMap.java +++ b/platform/util/src/com/intellij/util/keyFMap/MapBackedFMap.java @@ -98,6 +98,32 @@ final class MapBackedFMap extends TIntObjectHashMap implements KeyFMap { return getKeysByIndices(keys()); } + @Override + public int identityHashCode() { + final int[] hash = {0}; + forEachEntry(new TIntObjectProcedure() { + @Override + public boolean execute(int key, Object value) { + hash[0] = (hash[0] * 31 + key) * 31 + System.identityHashCode(value); + return true; + } + }); + return hash[0]; + } + + @Override + public boolean equalsByReference(KeyFMap other) { + if(other == this) return true; + if (!(other instanceof MapBackedFMap) || other.size() != size()) return false; + final MapBackedFMap map = (MapBackedFMap)other; + return forEachEntry(new TIntObjectProcedure() { + @Override + public boolean execute(int key, Object value) { + return map.get(key) == value; + } + }); + } + @Override public String toString() { final StringBuilder s = new StringBuilder(); diff --git a/platform/util/src/com/intellij/util/keyFMap/OneElementFMap.java b/platform/util/src/com/intellij/util/keyFMap/OneElementFMap.java index 53ce5ec90841..765e9efd2b69 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 final class OneElementFMap implements KeyFMap { +final class OneElementFMap implements KeyFMap { private final Key myKey; - private final VV myValue; + private final Object myValue; - public OneElementFMap(@NotNull Key key, @NotNull VV value) { + public OneElementFMap(@NotNull Key key, @NotNull V value) { myKey = key; myValue = value; } @@ -30,7 +30,9 @@ public final class OneElementFMap implements KeyFMap { @NotNull @Override public KeyFMap plus(@NotNull Key key, @NotNull V value) { - if (myKey == key) return new OneElementFMap(key, value); + if (myKey == key) { + return value == myValue ? this : new OneElementFMap(key, value); + } return new PairElementsFMap(myKey, myValue, key, value); } @@ -46,6 +48,11 @@ public final class OneElementFMap implements KeyFMap { return myKey == key ? (V)myValue : null; } + @Override + public int size() { + return 1; + } + @NotNull @Override public Key[] getKeys() { @@ -54,7 +61,7 @@ public final class OneElementFMap implements KeyFMap { @Override public String toString() { - return "<" + myKey + " -> " + myValue+">"; + return "{" + myKey + "=" + myValue + "}"; } @Override @@ -62,12 +69,9 @@ public final class OneElementFMap implements KeyFMap { return false; } - public Key getKey() { - return myKey; - } - - public VV getValue() { - return myValue; + @Override + public int identityHashCode() { + return myKey.hashCode() * 31 + System.identityHashCode(myValue); } @Override @@ -79,6 +83,15 @@ public final class OneElementFMap implements KeyFMap { return myKey == map.myKey && myValue.equals(map.myValue); } + @Override + public boolean equalsByReference(KeyFMap o) { + if (this == o) return true; + if (!(o instanceof OneElementFMap)) return false; + + OneElementFMap map = (OneElementFMap)o; + return myKey == map.myKey && myValue == map.myValue; + } + @Override public int hashCode() { 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 b918d9a2d4ba..19042fbf6a6f 100644 --- a/platform/util/src/com/intellij/util/keyFMap/PairElementsFMap.java +++ b/platform/util/src/com/intellij/util/keyFMap/PairElementsFMap.java @@ -18,7 +18,7 @@ package com.intellij.util.keyFMap; import com.intellij.openapi.util.Key; import org.jetbrains.annotations.NotNull; -public final class PairElementsFMap implements KeyFMap { +final class PairElementsFMap implements KeyFMap { // invariant: key1.hashCode() < key2.hashCode() private final @NotNull Key key1; private final @NotNull Key key2; @@ -44,8 +44,12 @@ public final class PairElementsFMap implements KeyFMap { @NotNull @Override 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 == key1) { + return value == value1 ? this : new PairElementsFMap(key, value, key2, value2); + } + if (key == key2) { + return value == value2 ? this : 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()) { @@ -57,8 +61,8 @@ public final class PairElementsFMap implements KeyFMap { @NotNull @Override public KeyFMap minus(@NotNull Key key) { - if (key == key1) return new OneElementFMap(key2, value2); - if (key == key2) return new OneElementFMap(key1, value1); + if (key == key1) return new OneElementFMap(key2, value2); + if (key == key2) return new OneElementFMap(key1, value1); return this; } @@ -68,6 +72,11 @@ public final class PairElementsFMap implements KeyFMap { return key == key1 ? (V)value1 : key == key2 ? (V)value2 : null; } + @Override + public int size() { + return 2; + } + @NotNull @Override public Key[] getKeys() { @@ -76,7 +85,7 @@ public final class PairElementsFMap implements KeyFMap { @Override public String toString() { - return "Pair: (" + key1 + " -> " + value1 + "; " + key2 + " -> " + value2 + ")"; + return "{" + key1 + "=" + value1 + ", " + key2 + "=" + value2 + "}"; } @Override @@ -84,24 +93,11 @@ public final 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 identityHashCode() { + int hash = key1.hashCode() * 31 + System.identityHashCode(value1); + hash = (hash * 31 + key2.hashCode()) * 31 + System.identityHashCode(value2); + return hash; } @Override @@ -118,4 +114,14 @@ public final class PairElementsFMap implements KeyFMap { return key1 == map.key1 && value1.equals(map.value1) && key2 == map.key2 && value2.equals(map.value2); } + + @Override + public boolean equalsByReference(KeyFMap o) { + if (this == o) return true; + if (!(o instanceof PairElementsFMap)) return false; + + PairElementsFMap map = (PairElementsFMap)o; + + return key1 == map.key1 && value1 == map.value1 && key2 == map.key2 && value2 == 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 3b7850a5daf5..fd59aa1a9da2 100644 --- a/platform/util/testSrc/com/intellij/util/keyFMap/KeyFMapTest.java +++ b/platform/util/testSrc/com/intellij/util/keyFMap/KeyFMapTest.java @@ -93,6 +93,24 @@ public class KeyFMapTest extends TestCase { } } + public void testIdentityHashCode() { + KeyFMap map = KeyFMap.EMPTY_MAP; + for(int i=0; i<15; i++) { + String val1 = "Value#"+i; + String val2 = "Value#"+i; + KeyFMap map1 = map.plus(KEYS.get(i), val1); + KeyFMap map2 = map.plus(KEYS.get(i), val2); + assertEquals(map1.hashCode(), map2.hashCode()); + assertEquals(map1, map2); + assertFalse(map1.identityHashCode() == map2.identityHashCode()); + assertFalse(map1.equalsByReference(map2)); + map2 = map.plus(KEYS.get(i), val1); + assertTrue(map1.identityHashCode() == map2.identityHashCode()); + assertTrue(map1.equalsByReference(map2)); + map = map1; + } + } + public void testGetKeysOnEmptyFMap() { doTestGetKeys(0); }