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 <robert.varga@pantheon.tech>

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 1b9a21f85e1b1ba24014a51ed09c4e21492f36d2..f1b51ce2617db4ebee53d3765fd96bc9d9449b50 100644 (file)
--- 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<String, QName> CACHE = CacheBuilder.newBuilder().maximumSize(MAX_QNAME_CACHE_SIZE)
-            .weakValues().build(new CacheLoader<String, QName>() {
+    private static final LoadingCache<String, QName> STRING_CACHE = CacheBuilder.newBuilder()
+            .maximumSize(MAX_QNAME_CACHE_SIZE).weakValues().build(new CacheLoader<String, QName>() {
                 @Override
                 public QName load(final String key) {
                     return QName.create(key).intern();
                 }
             });
 
+    private static final LoadingCache<Key, QName> KEY_CACHE = CacheBuilder.newBuilder()
+            .maximumSize(MAX_QNAME_CACHE_SIZE).weakValues().build(new CacheLoader<Key, QName>() {
+                @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 4e7216df88eac82e5ea4b4d875c29fc9f4b66a14..9b2ce2b64d4fab9d71e5ac844983008ad8b85979 100755 (executable)
--- 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<String> codedStringMap = new ArrayList<>();
@@ -71,8 +69,6 @@ public class NormalizedNodeInputStreamReader implements NormalizedNodeDataInput
     @SuppressWarnings("rawtypes")
     private NormalizedNodeBuilder<NodeWithValue, Object, LeafSetEntryNode<Object>> 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 9515240e932e76031109658e26e50b50ac44b6ba..9f5419d20339dbedacf14e2aae8f17fbffb5b4b5 100644 (file)
--- 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));
     }
 }