From 4d9034700f0ac327948c3be5ea347e628d82fd81 Mon Sep 17 00:00:00 2001 From: Robert Varga Date: Thu, 20 Jun 2019 16:12:07 +0200 Subject: [PATCH] Add a singleton NodeIdentifierWithPredicates 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 --- .../yangtools/yang/data/api/NIPv1.java | 2 +- .../yang/data/api/YangInstanceIdentifier.java | 223 ++++++++++++------ .../data/api/YangInstanceIdentifierTest.java | 4 +- 3 files changed, 159 insertions(+), 70 deletions(-) diff --git a/yang/yang-data-api/src/main/java/org/opendaylight/yangtools/yang/data/api/NIPv1.java b/yang/yang-data-api/src/main/java/org/opendaylight/yangtools/yang/data/api/NIPv1.java index f07c307432..de051e504c 100644 --- a/yang/yang-data-api/src/main/java/org/opendaylight/yangtools/yang/data/api/NIPv1.java +++ b/yang/yang-data-api/src/main/java/org/opendaylight/yangtools/yang/data/api/NIPv1.java @@ -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) in.readObject(), qname); + nip = NodeIdentifierWithPredicates.of(qname, (Map) in.readObject()); } private Object readResolve() { diff --git a/yang/yang-data-api/src/main/java/org/opendaylight/yangtools/yang/data/api/YangInstanceIdentifier.java b/yang/yang-data-api/src/main/java/org/opendaylight/yangtools/yang/data/api/YangInstanceIdentifier.java index 0aa4a97de3..40265f83ff 100644 --- a/yang/yang-data-api/src/main/java/org/opendaylight/yangtools/yang/data/api/YangInstanceIdentifier.java +++ b/yang/yang-data-api/src/main/java/org/opendaylight/yangtools/yang/data/api/YangInstanceIdentifier.java @@ -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> entrySet() { + return SingletonSet.of(new SimpleImmutableEntry<>(key, value)); + } + + @Override + public SingletonSet keySet() { + return SingletonSet.of(key); + } + + @Override + public SingletonSet values() { + return SingletonSet.of(value); + } + + @Override + public int size() { + return 1; + } + + @Override + public ImmutableMap 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 keyValues; + + Regular(final QName node, final Map keyValues) { + super(node); + this.keyValues = requireNonNull(keyValues); + } + + @Override + public Set> entrySet() { + return keyValues.entrySet(); + } + + @Override + public Set keySet() { + return keyValues.keySet(); + } + + @Override + public Collection values() { + return keyValues.values(); + } + + @Override + public int size() { + return keyValues.size(); + } + + @Override + public Map asMap() { + return keyValues; + } + + @Override + Object keyValue(final QName qname) { + return keyValues.get(qname); + } + + @Override + boolean equalMapping(final NodeIdentifierWithPredicates other) { + final Map 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 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 keyValues; + private static final long serialVersionUID = -4787195606494761540L; - // Exposed for NIPv1 - NodeIdentifierWithPredicates(final Map 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 entry) { + return of(node, entry.getKey(), entry.getValue()); } public static @NonNull NodeIdentifierWithPredicates of(final QName node, final Map 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 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 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 keyValues, final QName node) { + return of(node, keyValues.entrySet().iterator().next()); } /** @@ -607,9 +726,7 @@ public abstract class YangInstanceIdentifier implements Path> entrySet() { - return keyValues.entrySet(); - } + public abstract @NonNull Set> entrySet(); /** * Return the predicate key in the iteration order of {@link #entrySet()}. @@ -617,9 +734,7 @@ public abstract class YangInstanceIdentifier implements Path keySet() { - return keyValues.keySet(); - } + public abstract @NonNull Set keySet(); /** * Return the predicate values in the iteration order of {@link #entrySet()}. @@ -627,29 +742,25 @@ public abstract class YangInstanceIdentifier implements Path values() { - return keyValues.values(); - } + public abstract @NonNull Collection 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 @Nullable T getValue(final QName key, final Class valueClass) { + public final @Nullable T getValue(final QName key, final Class 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 asMap() { - return keyValues; - } + public abstract @NonNull Map asMap(); @Override - protected int hashCodeImpl() { - final int prime = 31; - int result = super.hashCodeImpl(); - result = prime * result; - - for (Entry 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 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 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 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); } } diff --git a/yang/yang-data-api/src/test/java/org/opendaylight/yangtools/yang/data/api/YangInstanceIdentifierTest.java b/yang/yang-data-api/src/test/java/org/opendaylight/yangtools/yang/data/api/YangInstanceIdentifierTest.java index 5f88c958ab..0c21a60db0 100644 --- a/yang/yang-data-api/src/test/java/org/opendaylight/yangtools/yang/data/api/YangInstanceIdentifierTest.java +++ b/yang/yang-data-api/src/test/java/org/opendaylight/yangtools/yang/data/api/YangInstanceIdentifierTest.java @@ -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()); -- 2.36.6