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

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<KeyFMap> {
 
   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> {
     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);
   }
 }

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<Object> key1 = Key.getKeyByIndex(keys[i1]);
           Key<Object> key2 = Key.getKeyByIndex(keys[i2]);
-          if (key1 == null && key2 == null) return EMPTY_MAP;
-          if (key1 == null) return new OneElementFMap<Object>(key2, values[i2]);
-          if (key2 == null) return new OneElementFMap<Object>(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;
+  }
 }

platform/util/src/com/intellij/util/keyFMap/EmptyFMap.java

@@ -27,7 +27,7 @@ class EmptyFMap implements KeyFMap {
   @NotNull
   @Override
   public <V> KeyFMap plus(@NotNull Key<V> key, @NotNull V value) {
-    return new OneElementFMap<V>(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 "<empty>";
+    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;

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}.
  *
  * <p>
  *   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()}
+ * </p>
+ *
+ * <p>
+ *   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.
  * </p>
  *
  * @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 <V> a type of the value
+   * @return an updated {@code KeyFMap} (this or newly created)
+   */
   @NotNull
   <V> KeyFMap plus(@NotNull Key<V> 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 <V> a type of the value
+   * @return a value associated with given key or null if there's not such value
+   */
   @Nullable
   <V> V get(@NotNull Key<V> 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);
 }

platform/util/src/com/intellij/util/keyFMap/MapBackedFMap.java

@@ -98,6 +98,32 @@ final class MapBackedFMap extends TIntObjectHashMap<Object> implements KeyFMap {
     return getKeysByIndices(keys());
   }
 
+  @Override
+  public int identityHashCode() {
+    final int[] hash = {0};
+    forEachEntry(new TIntObjectProcedure<Object>() {
+      @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<Object>() {
+      @Override
+      public boolean execute(int key, Object value) {
+        return map.get(key) == value;
+      }
+    });
+  }
+
   @Override
   public String toString() {
     final StringBuilder s = new StringBuilder();

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<VV> 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 <V> OneElementFMap(@NotNull Key<V> key, @NotNull V value) {
     myKey = key;
     myValue = value;
   }
@@ -30,7 +30,9 @@ public final class OneElementFMap<VV> implements KeyFMap {
   @NotNull
   @Override
   public <V> KeyFMap plus(@NotNull Key<V> key, @NotNull V value) {
-    if (myKey == key) return new OneElementFMap<V>(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<VV> 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<VV> implements KeyFMap {
 
   @Override
   public String toString() {
-    return "<" + myKey + " -> " + myValue+">";
+    return "{" + myKey + "=" + myValue + "}";
   }
 
   @Override
@@ -62,12 +69,9 @@ public final class OneElementFMap<VV> 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<VV> 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();

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 <V> KeyFMap plus(@NotNull Key<V> 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<Object>(key2, value2);
-    if (key == key2) return new OneElementFMap<Object>(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;
+  }
 }

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);
   }