Allocate new keyes map lazily

Do not allocate the map for new mappings until it is actually needed.
This introduces some additional complexity via null checks, but that
is not something major.

Also refactor large equals() method into smaller chunks, so that it
can be inlined more easily.

JIRA: YANGTOOLS-919
Change-Id: Idb4b7a4cb7cde09af2eab0bcebfb71083e64ff2f
Signed-off-by: Robert Varga <robert.varga@pantheon.tech>
(cherry picked from commit 62876a059465a1c780eb88e9ef5f285f4a052ec2)

diff --git a/common/util/src/main/java/org/opendaylight/yangtools/util/MutableOffsetMap.java b/common/util/src/main/java/org/opendaylight/yangtools/util/MutableOffsetMap.java
index d06859438686a027d6886790e3f8f3124ce81954..e82b50acd7795645503ac587a4ea9bf43bfda781 100644 (file)
--- a/common/util/src/main/java/org/opendaylight/yangtools/util/MutableOffsetMap.java
+++ b/common/util/src/main/java/org/opendaylight/yangtools/util/MutableOffsetMap.java
@@ -7,7 +7,7 @@
  */
 package org.opendaylight.yangtools.util;
 
-import static com.google.common.base.Verify.verify;
+import static com.google.common.base.Preconditions.checkState;
 import static java.util.Objects.requireNonNull;
 
 import com.google.common.annotations.Beta;
@@ -18,6 +18,7 @@ import java.util.AbstractMap;
 import java.util.AbstractSet;
 import java.util.ArrayList;
 import java.util.Arrays;
+import java.util.Collections;
 import java.util.ConcurrentModificationException;
 import java.util.HashMap;
 import java.util.Iterator;
@@ -47,15 +48,14 @@ import org.eclipse.jdt.annotation.NonNull;
 public abstract class MutableOffsetMap<K, V> extends AbstractMap<K, V> implements Cloneable, ModifiableMapPhase<K, V> {
     static final class Ordered<K, V> extends MutableOffsetMap<K, V> {
         Ordered() {
-            super(new LinkedHashMap<>());
         }
 
         Ordered(final Map<K, V> source) {
-            super(OffsetMapCache.orderedOffsets(source.keySet()), source, new LinkedHashMap<>());
+            super(OffsetMapCache.orderedOffsets(source.keySet()), source);
         }
 
         Ordered(final ImmutableMap<K, Integer> offsets, final V[] objects) {
-            super(offsets, objects, new LinkedHashMap<>());
+            super(offsets, objects);
         }
 
         @Override
@@ -77,19 +77,23 @@ public abstract class MutableOffsetMap<K, V> extends AbstractMap<K, V> implement
         SharedSingletonMap<K, V> singletonMap() {
             return SharedSingletonMap.orderedCopyOf(this);
         }
+
+        @Override
+        HashMap<K, V> createNewKeys() {
+            return new LinkedHashMap<>();
+        }
     }
 
     static final class Unordered<K, V> extends MutableOffsetMap<K, V> {
         Unordered() {
-            super(new HashMap<>());
         }
 
         Unordered(final Map<K, V> source) {
-            super(OffsetMapCache.unorderedOffsets(source.keySet()), source, new HashMap<>());
+            super(OffsetMapCache.unorderedOffsets(source.keySet()), source);
         }
 
         Unordered(final ImmutableMap<K, Integer> offsets, final V[] objects) {
-            super(offsets, objects, new HashMap<>());
+            super(offsets, objects);
         }
 
         @Override
@@ -112,6 +116,11 @@ public abstract class MutableOffsetMap<K, V> extends AbstractMap<K, V> implement
         SharedSingletonMap<K, V> singletonMap() {
             return SharedSingletonMap.unorderedCopyOf(this);
         }
+
+        @Override
+        HashMap<K, V> createNewKeys() {
+            return new HashMap<>();
+        }
     }
 
     private static final Object[] EMPTY_ARRAY = new Object[0];
@@ -127,21 +136,17 @@ public abstract class MutableOffsetMap<K, V> extends AbstractMap<K, V> implement
     private transient volatile int modCount;
     private boolean needClone = true;
 
-    MutableOffsetMap(final ImmutableMap<K, Integer> offsets, final V[] objects, final HashMap<K, V> newKeys) {
-        verify(newKeys.isEmpty());
+    MutableOffsetMap(final ImmutableMap<K, Integer> offsets, final Object[] objects) {
         this.offsets = requireNonNull(offsets);
         this.objects = requireNonNull(objects);
-        this.newKeys = requireNonNull(newKeys);
     }
 
-    @SuppressWarnings("unchecked")
-    MutableOffsetMap(final HashMap<K, V> newKeys) {
-        this(ImmutableMap.of(), (V[]) EMPTY_ARRAY, newKeys);
+    MutableOffsetMap() {
+        this(ImmutableMap.of(), EMPTY_ARRAY);
     }
 
-    @SuppressWarnings("unchecked")
-    MutableOffsetMap(final ImmutableMap<K, Integer> offsets, final Map<K, V> source, final HashMap<K, V> newKeys) {
-        this(offsets, (V[]) new Object[offsets.size()], newKeys);
+    MutableOffsetMap(final ImmutableMap<K, Integer> offsets, final Map<K, V> source) {
+        this(offsets, new Object[offsets.size()]);
 
         for (Entry<K, V> e : source.entrySet()) {
             objects[offsets.get(e.getKey())] = requireNonNull(e.getValue());
@@ -216,7 +221,7 @@ public abstract class MutableOffsetMap<K, V> extends AbstractMap<K, V> implement
 
     @Override
     public final int size() {
-        return offsets.size() + newKeys.size() - removed;
+        return offsets.size() - removed + (newKeys == null ? 0 : newKeys.size());
     }
 
     @Override
@@ -234,7 +239,7 @@ public abstract class MutableOffsetMap<K, V> extends AbstractMap<K, V> implement
             }
         }
 
-        return newKeys.containsKey(key);
+        return newKeys != null && newKeys.containsKey(key);
     }
 
     @Override
@@ -255,7 +260,7 @@ public abstract class MutableOffsetMap<K, V> extends AbstractMap<K, V> implement
             }
         }
 
-        return newKeys.get(key);
+        return newKeys == null ? null : newKeys.get(key);
     }
 
     private void cloneArray() {
@@ -294,6 +299,9 @@ public abstract class MutableOffsetMap<K, V> extends AbstractMap<K, V> implement
             }
         }
 
+        if (newKeys == null) {
+            newKeys = createNewKeys();
+        }
         final V ret = newKeys.put(key, value);
         if (ret == null) {
             modCount++;
@@ -325,6 +333,9 @@ public abstract class MutableOffsetMap<K, V> extends AbstractMap<K, V> implement
             }
         }
 
+        if (newKeys == null) {
+            return null;
+        }
         final V ret = newKeys.remove(key);
         if (ret != null) {
             modCount++;
@@ -335,7 +346,9 @@ public abstract class MutableOffsetMap<K, V> extends AbstractMap<K, V> implement
     @Override
     public final void clear() {
         if (size() != 0) {
-            newKeys.clear();
+            if (newKeys != null) {
+                newKeys.clear();
+            }
             cloneArray();
             Arrays.fill(objects, removedObject());
             removed = objects.length;
@@ -349,8 +362,8 @@ public abstract class MutableOffsetMap<K, V> extends AbstractMap<K, V> implement
     }
 
     @Override
-    public Map<K, V> toUnmodifiableMap() {
-        if (removed == 0 && newKeys.isEmpty()) {
+    public @NonNull Map<K, V> toUnmodifiableMap() {
+        if (removed == 0 && noNewKeys()) {
             // Make sure next modification clones the array, as we leak it to the map we return.
             needClone = true;
 
@@ -388,7 +401,9 @@ public abstract class MutableOffsetMap<K, V> extends AbstractMap<K, V> implement
         } else {
             keyset.addAll(offsets.keySet());
         }
-        keyset.addAll(newKeys.keySet());
+        if (newKeys != null) {
+            keyset.addAll(newKeys.keySet());
+        }
 
         // Construct the values
         @SuppressWarnings("unchecked")
@@ -409,8 +424,10 @@ public abstract class MutableOffsetMap<K, V> extends AbstractMap<K, V> implement
             System.arraycopy(objects, 0, values, 0, offsets.size());
             offset = offsets.size();
         }
-        for (V v : newKeys.values()) {
-            values[offset++] = v;
+        if (newKeys != null) {
+            for (V v : newKeys.values()) {
+                values[offset++] = v;
+            }
         }
 
         return modifiedMap(keyset, values);
@@ -427,7 +444,7 @@ public abstract class MutableOffsetMap<K, V> extends AbstractMap<K, V> implement
             throw new IllegalStateException("Clone is expected to work", e);
         }
 
-        ret.newKeys = (HashMap<K, V>) newKeys.clone();
+        ret.newKeys = newKeys == null ? null : (HashMap<K, V>) newKeys.clone();
         ret.needClone = true;
         return ret;
     }
@@ -443,7 +460,7 @@ public abstract class MutableOffsetMap<K, V> extends AbstractMap<K, V> implement
             }
         }
 
-        return result + newKeys.hashCode();
+        return newKeys != null ? result + newKeys.hashCode() : result;
     }
 
     @Override
@@ -458,31 +475,38 @@ public abstract class MutableOffsetMap<K, V> extends AbstractMap<K, V> implement
         if (obj instanceof ImmutableOffsetMap) {
             final ImmutableOffsetMap<?, ?> om = (ImmutableOffsetMap<?, ?>) obj;
 
-            if (newKeys.isEmpty() && offsets.equals(om.offsets())) {
+            if (noNewKeys() && offsets.equals(om.offsets())) {
                 return Arrays.deepEquals(objects, om.objects());
             }
         } else if (obj instanceof MutableOffsetMap) {
             final MutableOffsetMap<?, ?> om = (MutableOffsetMap<?, ?>) obj;
 
             if (offsets.equals(om.offsets)) {
-                return Arrays.deepEquals(objects, om.objects) && newKeys.equals(om.newKeys);
+                return Arrays.deepEquals(objects, om.objects) && equalNewKeys(om);
             }
         }
 
         // Fall back to brute map compare
-        final Map<?, ?> other = (Map<?, ?>)obj;
+        return mapEquals((Map<?, ?>)obj);
+    }
 
+    private boolean equalNewKeys(final MutableOffsetMap<?, ?> other) {
+        return noNewKeys() ? other.noNewKeys() : newKeys.equals(other.newKeys());
+    }
+
+    private boolean mapEquals(final Map<?, ?> other) {
         // Size and key sets have to match
         if (size() != other.size() || !keySet().equals(other.keySet())) {
             return false;
         }
 
         try {
-            // Ensure all newKeys are present. Note newKeys is guaranteed to
-            // not contain null value.
-            for (Entry<K, V> e : newKeys.entrySet()) {
-                if (!e.getValue().equals(other.get(e.getKey()))) {
-                    return false;
+            if (newKeys != null) {
+                // Ensure all newKeys are present. Note newKeys is guaranteed to not contain a null value.
+                for (Entry<K, V> e : newKeys.entrySet()) {
+                    if (!e.getValue().equals(other.get(e.getKey()))) {
+                        return false;
+                    }
                 }
             }
 
@@ -518,7 +542,13 @@ public abstract class MutableOffsetMap<K, V> extends AbstractMap<K, V> implement
 
     @VisibleForTesting
     final Object newKeys() {
-        return newKeys;
+        return newKeys != null ? newKeys : ImmutableMap.of();
+    }
+
+    abstract HashMap<K, V> createNewKeys();
+
+    private boolean noNewKeys() {
+        return newKeys == null || newKeys.isEmpty();
     }
 
     private final class EntrySet extends AbstractSet<Entry<K, V>> {
@@ -608,7 +638,8 @@ public abstract class MutableOffsetMap<K, V> extends AbstractMap<K, V> implement
 
     private abstract class AbstractSetIterator<E> implements Iterator<E> {
         private final Iterator<Entry<K, Integer>> oldIterator = offsets.entrySet().iterator();
-        private final Iterator<K> newIterator = newKeys.keySet().iterator();
+        private final Iterator<K> newIterator = newKeys == null ? Collections.emptyIterator()
+                : newKeys.keySet().iterator();
         private int expectedModCount = modCount;
         private K currentKey;
         private K nextKey;
@@ -644,9 +675,8 @@ public abstract class MutableOffsetMap<K, V> extends AbstractMap<K, V> implement
 
         @Override
         public final void remove() {
-            requireNonNull(currentKey != null);
-
             checkModCount();
+            checkState(currentKey != null);
             final Integer offset = offsets.get(currentKey);
             if (offset != null) {
                 cloneArray();