Add a singleton NodeIdentifierWithPredicates 04/82604/20
authorRobert Varga <robert.varga@pantheon.tech>
Thu, 20 Jun 2019 14:12:07 +0000 (16:12 +0200)
committerRobert Varga <robert.varga@pantheon.tech>
Tue, 27 Aug 2019 22:29:02 +0000 (00:29 +0200)
Heap dumps from OFP-centric load shows that single-predicate
identifiers are dominating the workload. This adds a dedicated
class to handle those cases.

The upshot of a dedicated class vs. a singleton map is a reduction
of one object, i.e. with SharedSingletonMap we get:

NodeIdentifierWithPredicates (24/32b)
SharedSingletonMap (24/32b)

whereas a dedicated class does only:

NodeIdentifierWithPredicates.Singleton (32/40b)

Which reduces total storage requirements from 48/64 bytes down to
32/40 bytes, saving 16/24 bytes (33/37.5%).

Change-Id: I3172ab9435d67adc12ae1734f9ad35f1dc8f891f
Signed-off-by: Robert Varga <robert.varga@pantheon.tech>
yang/yang-data-api/src/main/java/org/opendaylight/yangtools/yang/data/api/NIPv1.java
yang/yang-data-api/src/main/java/org/opendaylight/yangtools/yang/data/api/YangInstanceIdentifier.java
yang/yang-data-api/src/test/java/org/opendaylight/yangtools/yang/data/api/YangInstanceIdentifierTest.java

index f07c307432cca99ad6ff251e19a47a6ea33ab3fe..de051e504c9e7a21fc38e122d9769384b6b55378 100644 (file)
@@ -46,7 +46,7 @@ final class NIPv1 implements Externalizable {
     @Override
     public void readExternal(final ObjectInput in) throws IOException, ClassNotFoundException {
         final QName qname = QName.readFrom(in);
-        nip = new NodeIdentifierWithPredicates((Map<QName, Object>) in.readObject(), qname);
+        nip = NodeIdentifierWithPredicates.of(qname, (Map<QName, Object>) in.readObject());
     }
 
     private Object readResolve() {
index 0aa4a97de37c3413f75f662386fce19727266350..40265f83ffc9a801b1504ced3dbc71ce45974d69 100644 (file)
@@ -23,6 +23,7 @@ import com.google.common.collect.Sets;
 import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
 import java.io.Serializable;
 import java.lang.reflect.Array;
+import java.util.AbstractMap.SimpleImmutableEntry;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collection;
@@ -44,6 +45,7 @@ import org.opendaylight.yangtools.concepts.Path;
 import org.opendaylight.yangtools.util.HashCodeBuilder;
 import org.opendaylight.yangtools.util.ImmutableOffsetMap;
 import org.opendaylight.yangtools.util.SharedSingletonMap;
+import org.opendaylight.yangtools.util.SingletonSet;
 import org.opendaylight.yangtools.yang.common.QName;
 import org.opendaylight.yangtools.yang.common.QNameModule;
 import org.opendaylight.yangtools.yang.data.api.schema.LeafSetEntryNode;
@@ -567,38 +569,155 @@ public abstract class YangInstanceIdentifier implements Path<YangInstanceIdentif
      * Composite path argument identifying a {@link org.opendaylight.yangtools.yang.data.api.schema.MapEntryNode} leaf
      * overall data tree.
      */
-    public static final class NodeIdentifierWithPredicates extends AbstractPathArgument {
-        private static final long serialVersionUID = -4787195606494761540L;
+    public abstract static class NodeIdentifierWithPredicates extends AbstractPathArgument {
+        private static final class Singleton extends NodeIdentifierWithPredicates {
+            private static final long serialVersionUID = 1L;
+
+            private final @NonNull QName key;
+            private final @NonNull Object value;
+
+            Singleton(final QName node, final QName key, final Object value) {
+                super(node);
+                this.key = requireNonNull(key);
+                this.value = requireNonNull(value);
+            }
+
+            @Override
+            public SingletonSet<Entry<QName, Object>> entrySet() {
+                return SingletonSet.of(new SimpleImmutableEntry<>(key, value));
+            }
+
+            @Override
+            public SingletonSet<QName> keySet() {
+                return SingletonSet.of(key);
+            }
+
+            @Override
+            public SingletonSet<Object> values() {
+                return SingletonSet.of(value);
+            }
+
+            @Override
+            public int size() {
+                return 1;
+            }
+
+            @Override
+            public ImmutableMap<QName, Object> asMap() {
+                return ImmutableMap.of(key, value);
+            }
+
+            @Override
+            boolean equalMapping(final NodeIdentifierWithPredicates other) {
+                final Singleton single = (Singleton) other;
+                return key.equals(single.key) && Objects.deepEquals(value, single.value);
+            }
+
+            @Override
+            Object keyValue(final QName qname) {
+                return key.equals(qname) ? value : null;
+            }
+        }
+
+        private static final class Regular extends NodeIdentifierWithPredicates {
+            private static final long serialVersionUID = 1L;
+
+            private final @NonNull Map<QName, Object> keyValues;
+
+            Regular(final QName node, final Map<QName, Object> keyValues) {
+                super(node);
+                this.keyValues = requireNonNull(keyValues);
+            }
+
+            @Override
+            public Set<Entry<QName, Object>> entrySet() {
+                return keyValues.entrySet();
+            }
+
+            @Override
+            public Set<QName> keySet() {
+                return keyValues.keySet();
+            }
+
+            @Override
+            public Collection<Object> values() {
+                return keyValues.values();
+            }
+
+            @Override
+            public int size() {
+                return keyValues.size();
+            }
+
+            @Override
+            public Map<QName, Object> asMap() {
+                return keyValues;
+            }
+
+            @Override
+            Object keyValue(final QName qname) {
+                return keyValues.get(qname);
+            }
+
+            @Override
+            boolean equalMapping(final NodeIdentifierWithPredicates other) {
+                final Map<QName, Object> otherKeyValues = ((Regular) other).keyValues;
+                // TODO: benchmark to see if just calling equals() on the two maps is not faster
+                if (keyValues == otherKeyValues) {
+                    return true;
+                }
+                if (keyValues.size() != otherKeyValues.size()) {
+                    return false;
+                }
+
+                for (Entry<QName, Object> entry : entrySet()) {
+                    final Object otherValue = otherKeyValues.get(entry.getKey());
+                    if (otherValue == null || !Objects.deepEquals(entry.getValue(), otherValue)) {
+                        return false;
+                    }
+                }
+
+                return true;
+            }
+        }
 
-        private final @NonNull Map<QName, Object> keyValues;
+        private static final long serialVersionUID = -4787195606494761540L;
 
-        // Exposed for NIPv1
-        NodeIdentifierWithPredicates(final Map<QName, Object> keyValues, final QName node) {
+        NodeIdentifierWithPredicates(final QName node) {
             super(node);
-            this.keyValues = requireNonNull(keyValues);
         }
 
         public static @NonNull NodeIdentifierWithPredicates of(final QName node) {
-            return new NodeIdentifierWithPredicates(ImmutableMap.of(), node);
+            return new Regular(node, ImmutableMap.of());
+        }
+
+        public static @NonNull NodeIdentifierWithPredicates of(final QName node, final QName key, final Object value) {
+            return new Singleton(node, key, value);
+        }
+
+        public static @NonNull NodeIdentifierWithPredicates of(final QName node, final Entry<QName, Object> entry) {
+            return of(node, entry.getKey(), entry.getValue());
         }
 
         public static @NonNull NodeIdentifierWithPredicates of(final QName node, final Map<QName, Object> keyValues) {
-            // Retains ImmutableMap for empty maps. For larger sizes uses a shared key set.
-            return new NodeIdentifierWithPredicates(ImmutableOffsetMap.unorderedCopyOf(keyValues), node);
+            return keyValues.size() == 1 ? of(keyValues, node)
+                    // Retains ImmutableMap for empty maps. For larger sizes uses a shared key set.
+                    : new Regular(node, ImmutableOffsetMap.unorderedCopyOf(keyValues));
         }
 
         public static @NonNull NodeIdentifierWithPredicates of(final QName node,
                 final ImmutableOffsetMap<QName, Object> keyValues) {
-            return new NodeIdentifierWithPredicates(keyValues, node);
+            return keyValues.size() == 1 ? of(keyValues, node) : new Regular(node, keyValues);
         }
 
+        @Deprecated
         public static @NonNull NodeIdentifierWithPredicates of(final QName node,
                 final SharedSingletonMap<QName, Object> keyValues) {
-            return new NodeIdentifierWithPredicates(keyValues, node);
+            return of(node, keyValues.getEntry());
         }
 
-        public static @NonNull NodeIdentifierWithPredicates of(final QName node, final QName key, final Object value) {
-            return of(node, SharedSingletonMap.unorderedOf(key, value));
+        private static @NonNull NodeIdentifierWithPredicates of(final Map<QName, Object> keyValues, final QName node) {
+            return of(node, keyValues.entrySet().iterator().next());
         }
 
         /**
@@ -607,9 +726,7 @@ public abstract class YangInstanceIdentifier implements Path<YangInstanceIdentif
          * @return Predicate set.
          */
         @Beta
-        public @NonNull Set<Entry<QName, Object>> entrySet() {
-            return keyValues.entrySet();
-        }
+        public abstract @NonNull Set<Entry<QName, Object>> entrySet();
 
         /**
          * Return the predicate key in the iteration order of {@link #entrySet()}.
@@ -617,9 +734,7 @@ public abstract class YangInstanceIdentifier implements Path<YangInstanceIdentif
          * @return Predicate values.
          */
         @Beta
-        public @NonNull Set<QName> keySet() {
-            return keyValues.keySet();
-        }
+        public abstract @NonNull Set<QName> keySet();
 
         /**
          * Return the predicate values in the iteration order of {@link #entrySet()}.
@@ -627,29 +742,25 @@ public abstract class YangInstanceIdentifier implements Path<YangInstanceIdentif
          * @return Predicate values.
          */
         @Beta
-        public @NonNull Collection<Object> values() {
-            return keyValues.values();
-        }
+        public abstract @NonNull Collection<Object> values();
 
         @Beta
-        public @Nullable Object getValue(final QName key) {
-            return keyValues.get(requireNonNull(key));
+        public final @Nullable Object getValue(final QName key) {
+            return keyValue(requireNonNull(key));
         }
 
         @Beta
-        public <T> @Nullable T getValue(final QName key, final Class<T> valueClass) {
+        public final <T> @Nullable T getValue(final QName key, final Class<T> valueClass) {
             return valueClass.cast(getValue(key));
         }
 
         /**
          * Return the number of predicates present.
          *
-         * @return Thee number of predicates present.
+         * @return The number of predicates present.
          */
         @Beta
-        public int size() {
-            return keyValues.size();
-        }
+        public abstract int size();
 
         /**
          * A Map-like view of this identifier's predicates. The view is expected to be stable and effectively-immutable.
@@ -664,63 +775,39 @@ public abstract class YangInstanceIdentifier implements Path<YangInstanceIdentif
         @Deprecated
         // FIXME: 4.0.0: evaluate the real usefulness of this. The problem here is Map.hashCode() and Map.equals(),
         //               which limits our options.
-        public @NonNull Map<QName, Object> asMap() {
-            return keyValues;
-        }
+        public abstract @NonNull Map<QName, Object> asMap();
 
         @Override
-        protected int hashCodeImpl() {
-            final int prime = 31;
-            int result = super.hashCodeImpl();
-            result = prime * result;
-
-            for (Entry<QName, Object> entry : keyValues.entrySet()) {
-                // FIXME: 4.0.0: key and value expected to be non-null here
-                result += Objects.hashCode(entry.getKey()) + YangInstanceIdentifier.hashCode(entry.getValue());
+        protected final int hashCodeImpl() {
+            int result = 31 * super.hashCodeImpl();
+            for (Entry<QName, Object> entry : entrySet()) {
+                result += entry.getKey().hashCode() + YangInstanceIdentifier.hashCode(entry.getValue());
             }
             return result;
         }
 
         @Override
         @SuppressWarnings("checkstyle:equalsHashCode")
-        public boolean equals(final Object obj) {
-            if (!super.equals(obj)) {
-                return false;
-            }
-
-            final Map<QName, Object> otherKeyValues = ((NodeIdentifierWithPredicates) obj).keyValues;
-
-            // TODO: benchmark to see if just calling equals() on the two maps is not faster
-            if (keyValues == otherKeyValues) {
-                return true;
-            }
-            if (keyValues.size() != otherKeyValues.size()) {
-                return false;
-            }
-
-            for (Entry<QName, Object> entry : keyValues.entrySet()) {
-                if (!otherKeyValues.containsKey(entry.getKey())
-                        || !Objects.deepEquals(entry.getValue(), otherKeyValues.get(entry.getKey()))) {
+        public final boolean equals(final Object obj) {
+            return super.equals(obj) && equalMapping((NodeIdentifierWithPredicates) obj);
+        }
 
-                    return false;
-                }
-            }
+        abstract boolean equalMapping(NodeIdentifierWithPredicates other);
 
-            return true;
-        }
+        abstract @Nullable Object keyValue(@NonNull QName qname);
 
         @Override
-        public String toString() {
-            return super.toString() + '[' + keyValues + ']';
+        public final String toString() {
+            return super.toString() + '[' + asMap() + ']';
         }
 
         @Override
-        public String toRelativeString(final PathArgument previous) {
-            return super.toRelativeString(previous) + '[' + keyValues + ']';
+        public final String toRelativeString(final PathArgument previous) {
+            return super.toRelativeString(previous) + '[' + asMap() + ']';
         }
 
         @Override
-        Object writeReplace() {
+        final Object writeReplace() {
             return new NIPv2(this);
         }
     }
index 5f88c958abea2afb849c12e7db748716b2dd7bb4..0c21a60db06f6e08f0492b1a689cbddf6b788015 100644 (file)
@@ -7,11 +7,13 @@
  */
 package org.opendaylight.yangtools.yang.data.api;
 
+import static org.hamcrest.CoreMatchers.instanceOf;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertNull;
 import static org.junit.Assert.assertSame;
+import static org.junit.Assert.assertThat;
 import static org.junit.Assert.assertTrue;
 
 import com.google.common.collect.ImmutableMap;
@@ -219,7 +221,7 @@ public class YangInstanceIdentifierTest {
             final PathArgument arg, final QName nodeName, final QName key, final Object value) {
 
         assertNotNull(prefix + " is null", arg);
-        assertEquals(prefix + " class", NodeIdentifierWithPredicates.class, arg.getClass());
+        assertThat(arg, instanceOf(NodeIdentifierWithPredicates.class));
         NodeIdentifierWithPredicates node = (NodeIdentifierWithPredicates)arg;
         assertEquals(prefix + " node type", nodeName, node.getNodeType());
         assertEquals(prefix + " key values map size", 1, node.size());