From: Robert Varga Date: Tue, 28 May 2019 11:14:34 +0000 (+0200) Subject: Do not format QNames to string on input X-Git-Tag: release/sodium~74 X-Git-Url: https://git.opendaylight.org/gerrit/gitweb?p=controller.git;a=commitdiff_plain;h=a69604185f71923bdfc16d2d056490fff9a8d90e;hp=dc54b87a6b2fb11d233b8294b684268f4d5161de Do not format QNames to string on input Profiling has revelead the creation of String for QNameFactory lookup is taking around 19% of overall cost of NormalizedNodeInputStreamReader.readQName(). Rather than using concatenation, use a dedicated Key object, which holds the separated-out localname/namespace/revision strings, thus allocating a small object instead of a full-blown String. This improves snapshot deserialization perfomance on a 350MiB snapshot by about 16%, also resulting in lower memory pressure. JIRA: CONTROLLER-1897 Change-Id: If3e9e4b332b0a5acbe33aa10fca189bbc7da8c81 Signed-off-by: Robert Varga --- diff --git a/opendaylight/md-sal/sal-clustering-commons/src/main/java/org/opendaylight/controller/cluster/datastore/node/utils/QNameFactory.java b/opendaylight/md-sal/sal-clustering-commons/src/main/java/org/opendaylight/controller/cluster/datastore/node/utils/QNameFactory.java index 1b9a21f85e..f1b51ce261 100644 --- a/opendaylight/md-sal/sal-clustering-commons/src/main/java/org/opendaylight/controller/cluster/datastore/node/utils/QNameFactory.java +++ b/opendaylight/md-sal/sal-clustering-commons/src/main/java/org/opendaylight/controller/cluster/datastore/node/utils/QNameFactory.java @@ -7,28 +7,81 @@ */ package org.opendaylight.controller.cluster.datastore.node.utils; +import static java.util.Objects.requireNonNull; + import com.google.common.cache.CacheBuilder; import com.google.common.cache.CacheLoader; import com.google.common.cache.LoadingCache; +import java.util.Objects; +import org.eclipse.jdt.annotation.NonNull; +import org.eclipse.jdt.annotation.Nullable; +import org.opendaylight.yangtools.concepts.Immutable; import org.opendaylight.yangtools.yang.common.QName; public final class QNameFactory { + public static final class Key implements Immutable { + private final @NonNull String localName; + private final @NonNull String namespace; + private final @Nullable String revision; + + public Key(final String localName, final String namespace, final String revision) { + this.localName = requireNonNull(localName); + this.namespace = requireNonNull(namespace); + this.revision = revision; + } + + @Override + public int hashCode() { + return Objects.hash(localName, namespace, revision); + } + + @Override + public boolean equals(final Object obj) { + if (this == obj) { + return true; + } + if (!(obj instanceof Key)) { + return false; + } + final Key other = (Key) obj; + return localName.equals(other.localName) && namespace.equals(other.namespace) + && Objects.equals(revision, other.revision); + } + + QName toQName() { + return revision != null ? QName.create(namespace, revision, localName) : QName.create(namespace, localName); + } + } + private static final int MAX_QNAME_CACHE_SIZE = Integer.getInteger( "org.opendaylight.controller.cluster.datastore.node.utils.qname-cache.max-size", 10000); - private static final LoadingCache CACHE = CacheBuilder.newBuilder().maximumSize(MAX_QNAME_CACHE_SIZE) - .weakValues().build(new CacheLoader() { + private static final LoadingCache STRING_CACHE = CacheBuilder.newBuilder() + .maximumSize(MAX_QNAME_CACHE_SIZE).weakValues().build(new CacheLoader() { @Override public QName load(final String key) { return QName.create(key).intern(); } }); + private static final LoadingCache KEY_CACHE = CacheBuilder.newBuilder() + .maximumSize(MAX_QNAME_CACHE_SIZE).weakValues().build(new CacheLoader() { + @Override + public QName load(final Key key) { + return key.toQName().intern(); + } + }); + private QNameFactory() { } + @Deprecated public static QName create(final String name) { - return CACHE.getUnchecked(name); + return STRING_CACHE.getUnchecked(name); + } + + public static QName create(final Key key) { + return KEY_CACHE.getUnchecked(key); } } diff --git a/opendaylight/md-sal/sal-clustering-commons/src/main/java/org/opendaylight/controller/cluster/datastore/node/utils/stream/NormalizedNodeInputStreamReader.java b/opendaylight/md-sal/sal-clustering-commons/src/main/java/org/opendaylight/controller/cluster/datastore/node/utils/stream/NormalizedNodeInputStreamReader.java index 4e7216df88..9b2ce2b64d 100755 --- a/opendaylight/md-sal/sal-clustering-commons/src/main/java/org/opendaylight/controller/cluster/datastore/node/utils/stream/NormalizedNodeInputStreamReader.java +++ b/opendaylight/md-sal/sal-clustering-commons/src/main/java/org/opendaylight/controller/cluster/datastore/node/utils/stream/NormalizedNodeInputStreamReader.java @@ -58,8 +58,6 @@ public class NormalizedNodeInputStreamReader implements NormalizedNodeDataInput private static final Logger LOG = LoggerFactory.getLogger(NormalizedNodeInputStreamReader.class); - private static final String REVISION_ARG = "?revision="; - private final DataInput input; private final List codedStringMap = new ArrayList<>(); @@ -71,8 +69,6 @@ public class NormalizedNodeInputStreamReader implements NormalizedNodeDataInput @SuppressWarnings("rawtypes") private NormalizedNodeBuilder> leafSetEntryBuilder; - private final StringBuilder reusableStringBuilder = new StringBuilder(50); - private boolean readSignatureMarker = true; NormalizedNodeInputStreamReader(final DataInput input, final boolean versionChecked) { @@ -225,18 +221,9 @@ public class NormalizedNodeInputStreamReader implements NormalizedNodeDataInput // Read in the same sequence of writing String localName = readCodedString(); String namespace = readCodedString(); - String revision = readCodedString(); - - String qname; - if (!Strings.isNullOrEmpty(revision)) { - qname = reusableStringBuilder.append('(').append(namespace).append(REVISION_ARG).append(revision) - .append(')').append(localName).toString(); - } else { - qname = reusableStringBuilder.append('(').append(namespace).append(')').append(localName).toString(); - } + String revision = Strings.emptyToNull(readCodedString()); - reusableStringBuilder.delete(0, reusableStringBuilder.length()); - return QNameFactory.create(qname); + return QNameFactory.create(new QNameFactory.Key(localName, namespace, revision)); } diff --git a/opendaylight/md-sal/sal-clustering-commons/src/test/java/org/opendaylight/controller/cluster/datastore/node/utils/QNameFactoryTest.java b/opendaylight/md-sal/sal-clustering-commons/src/test/java/org/opendaylight/controller/cluster/datastore/node/utils/QNameFactoryTest.java index 9515240e93..9f5419d203 100644 --- a/opendaylight/md-sal/sal-clustering-commons/src/test/java/org/opendaylight/controller/cluster/datastore/node/utils/QNameFactoryTest.java +++ b/opendaylight/md-sal/sal-clustering-commons/src/test/java/org/opendaylight/controller/cluster/datastore/node/utils/QNameFactoryTest.java @@ -5,32 +5,45 @@ * terms of the Eclipse Public License v1.0 which accompanies this distribution, * and is available at http://www.eclipse.org/legal/epl-v10.html */ - package org.opendaylight.controller.cluster.datastore.node.utils; import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertFalse; -import static org.junit.Assert.assertTrue; +import static org.junit.Assert.assertNotSame; +import static org.junit.Assert.assertSame; import org.junit.Test; +import org.opendaylight.controller.cluster.datastore.node.utils.QNameFactory.Key; import org.opendaylight.controller.cluster.datastore.util.TestModel; import org.opendaylight.yangtools.yang.common.QName; +import org.opendaylight.yangtools.yang.common.Revision; public class QNameFactoryTest { @Test - public void testBasic() { + @Deprecated + public void testBasicString() { QName expected = TestModel.AUG_NAME_QNAME; QName created = QNameFactory.create(expected.toString()); - - assertFalse(expected == created); - + assertNotSame(expected, created); assertEquals(expected, created); QName cached = QNameFactory.create(expected.toString()); + assertSame(created, cached); + } - assertEquals(expected, cached); + @Test + public void testBasic() { + QName expected = TestModel.AUG_NAME_QNAME; + QName created = QNameFactory.create(createKey(expected)); + assertNotSame(expected, created); + assertEquals(expected, created); + + QName cached = QNameFactory.create(createKey(expected)); + assertSame(created, cached); + } - assertTrue(cached == created); + private static Key createKey(final QName qname) { + return new Key(qname.getLocalName(), qname.getNamespace().toString(), + qname.getRevision().map(Revision::toString).orElse(null)); } }