Improve ShardedDOMDataTreeWriteTransaction performance

Make internal maps/collection immutable, as it really is
and forgo internal copying of their values in iterators.

Change-Id: Icb0e59d52464498fe87440467fec9095cc8d414e
Signed-off-by: Robert Varga <rovarga@cisco.com>
(cherry picked from commit edecff765d21129c17050d77400f15b670c74bdf)

diff --git a/dom/mdsal-dom-broker/src/main/java/org/opendaylight/mdsal/dom/broker/ShardedDOMDataTreeProducer.java b/dom/mdsal-dom-broker/src/main/java/org/opendaylight/mdsal/dom/broker/ShardedDOMDataTreeProducer.java
index 3ae355a071f414227281dc9ebeeed23a4311d1dc..b19d956192f23abd9f992b075a98fd272e1c0678 100644 (file)
--- a/dom/mdsal-dom-broker/src/main/java/org/opendaylight/mdsal/dom/broker/ShardedDOMDataTreeProducer.java
+++ b/dom/mdsal-dom-broker/src/main/java/org/opendaylight/mdsal/dom/broker/ShardedDOMDataTreeProducer.java
@@ -11,13 +11,13 @@ import com.google.common.base.Preconditions;
 import com.google.common.base.Verify;
 import com.google.common.collect.ArrayListMultimap;
 import com.google.common.collect.BiMap;
+import com.google.common.collect.Collections2;
 import com.google.common.collect.ImmutableBiMap;
 import com.google.common.collect.ImmutableBiMap.Builder;
 import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.Multimap;
 import java.util.Collection;
-import java.util.Collections;
 import java.util.Map;
 import java.util.Map.Entry;
 import java.util.Set;
@@ -31,6 +31,7 @@ import org.opendaylight.mdsal.dom.api.DOMDataTreeProducerException;
 import org.opendaylight.mdsal.dom.api.DOMDataTreeShard;
 import org.opendaylight.mdsal.dom.store.inmemory.DOMDataTreeShardProducer;
 import org.opendaylight.mdsal.dom.store.inmemory.WriteableDOMDataTreeShard;
+import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -60,7 +61,9 @@ class ShardedDOMDataTreeProducer implements DOMDataTreeProducer {
     private volatile ShardedDOMDataTreeWriteTransaction lastTx;
 
     @GuardedBy("this")
-    private Map<DOMDataTreeIdentifier, DOMDataTreeProducer> children = Collections.emptyMap();
+    private Map<DOMDataTreeIdentifier, DOMDataTreeProducer> children = ImmutableMap.of();
+    @GuardedBy("this")
+    private Set<YangInstanceIdentifier> childRoots = ImmutableSet.of();
     @GuardedBy("this")
     private boolean closed;
 
@@ -135,7 +138,7 @@ class ShardedDOMDataTreeProducer implements DOMDataTreeProducer {
             if (current != null) {
                 submitTransaction(current);
             }
-            ret = new ShardedDOMDataTreeWriteTransaction(this, idToProducer, children, true);
+            ret = new ShardedDOMDataTreeWriteTransaction(this, idToProducer, childRoots);
         } else {
             // Non-isolated case, see if we can reuse the transaction
             if (current != null) {
@@ -143,7 +146,7 @@ class ShardedDOMDataTreeProducer implements DOMDataTreeProducer {
                         current.getIdentifier());
                 ret = current;
             } else {
-                ret = new ShardedDOMDataTreeWriteTransaction(this, idToProducer, children, false);
+                ret = new ShardedDOMDataTreeWriteTransaction(this, idToProducer, childRoots);
             }
         }
 
@@ -208,6 +211,8 @@ class ShardedDOMDataTreeProducer implements DOMDataTreeProducer {
         }
 
         children = cb.build();
+        childRoots = ImmutableSet.copyOf(Collections2.transform(children.keySet(),
+            DOMDataTreeIdentifier::getRootIdentifier));
         return ret;
     }
 

diff --git a/dom/mdsal-dom-broker/src/main/java/org/opendaylight/mdsal/dom/broker/ShardedDOMDataTreeWriteTransaction.java b/dom/mdsal-dom-broker/src/main/java/org/opendaylight/mdsal/dom/broker/ShardedDOMDataTreeWriteTransaction.java
index b2deac6179fa5562a384439562f5a5d7244fb0bf..83080f7be141b5129b84a32bb0cc62bd595e034b 100644 (file)
--- a/dom/mdsal-dom-broker/src/main/java/org/opendaylight/mdsal/dom/broker/ShardedDOMDataTreeWriteTransaction.java
+++ b/dom/mdsal-dom-broker/src/main/java/org/opendaylight/mdsal/dom/broker/ShardedDOMDataTreeWriteTransaction.java
@@ -8,15 +8,15 @@
 package org.opendaylight.mdsal.dom.broker;
 
 import com.google.common.base.Preconditions;
-import com.google.common.collect.ImmutableSet;
+import com.google.common.collect.ImmutableMap;
+import com.google.common.collect.Maps;
 import com.google.common.util.concurrent.CheckedFuture;
 import com.google.common.util.concurrent.FutureCallback;
 import com.google.common.util.concurrent.Futures;
 import com.google.common.util.concurrent.ListenableFuture;
 import com.google.common.util.concurrent.SettableFuture;
+import java.util.Collection;
 import java.util.Deque;
-import java.util.HashMap;
-import java.util.HashSet;
 import java.util.LinkedList;
 import java.util.List;
 import java.util.Map;
@@ -32,7 +32,6 @@ import javax.annotation.concurrent.NotThreadSafe;
 import org.opendaylight.mdsal.common.api.TransactionCommitFailedException;
 import org.opendaylight.mdsal.dom.api.DOMDataTreeCursorAwareTransaction;
 import org.opendaylight.mdsal.dom.api.DOMDataTreeIdentifier;
-import org.opendaylight.mdsal.dom.api.DOMDataTreeProducer;
 import org.opendaylight.mdsal.dom.api.DOMDataTreeWriteCursor;
 import org.opendaylight.mdsal.dom.store.inmemory.DOMDataTreeShardProducer;
 import org.opendaylight.mdsal.dom.store.inmemory.DOMDataTreeShardWriteTransaction;
@@ -45,48 +44,45 @@ import org.slf4j.LoggerFactory;
 @NotThreadSafe
 final class ShardedDOMDataTreeWriteTransaction implements DOMDataTreeCursorAwareTransaction {
     private static final Logger LOG = LoggerFactory.getLogger(ShardedDOMDataTreeWriteTransaction.class);
+    private static final TransactionCommitFailedExceptionMapper SUBMIT_FAILED_MAPPER =
+            TransactionCommitFailedExceptionMapper.create("submit");
     private static final AtomicLong COUNTER = new AtomicLong();
+
     private final Map<DOMDataTreeIdentifier, DOMDataTreeShardWriteTransaction> idToTransaction;
+    private final Collection<YangInstanceIdentifier> childBoundaries;
     private final ShardedDOMDataTreeProducer producer;
     private final String identifier;
-    private final Set<YangInstanceIdentifier> childBoundaries = new HashSet<>();
+
+    private final SettableFuture<Void> future = SettableFuture.create();
+    private final CheckedFuture<Void, TransactionCommitFailedException> submitFuture =
+            Futures.makeChecked(future, SUBMIT_FAILED_MAPPER);
+
     @GuardedBy("this")
     private boolean closed =  false;
 
     @GuardedBy("this")
     private DOMDataTreeWriteCursor openCursor;
 
-    private final SettableFuture<Void> future = SettableFuture.create();
-    private final CheckedFuture<Void, TransactionCommitFailedException> submitFuture =
-            Futures.makeChecked(future, TransactionCommitFailedExceptionMapper.create("submit"));
-
-    private final boolean isolated;
-
     ShardedDOMDataTreeWriteTransaction(final ShardedDOMDataTreeProducer producer,
                                        final Map<DOMDataTreeIdentifier, DOMDataTreeShardProducer> idToProducer,
-                                       final Map<DOMDataTreeIdentifier, DOMDataTreeProducer> childProducers,
-                                       final boolean isolated) {
-        this.isolated = isolated;
+                                       final Set<YangInstanceIdentifier> childRoots) {
         this.producer = Preconditions.checkNotNull(producer);
-        idToTransaction = new HashMap<>();
-        Preconditions.checkNotNull(idToProducer).forEach((id, prod) -> idToTransaction.put(
-                id, prod.createTransaction()));
         this.identifier = "SHARDED-DOM-" + COUNTER.getAndIncrement();
-        LOG.debug("Created new transaction{}", identifier);
-        childProducers.forEach((id, prod) -> childBoundaries.add(id.getRootIdentifier()));
+        idToTransaction = ImmutableMap.copyOf(Maps.transformValues(idToProducer,
+            DOMDataTreeShardProducer::createTransaction));
+        childBoundaries = Preconditions.checkNotNull(childRoots);
+        LOG.debug("Created new transaction {}", identifier);
     }
 
-    // FIXME: use atomic operations
-    @GuardedBy("this")
     private DOMDataTreeShardWriteTransaction lookup(final DOMDataTreeIdentifier prefix) {
         for (final Entry<DOMDataTreeIdentifier, DOMDataTreeShardWriteTransaction> e : idToTransaction.entrySet()) {
             if (e.getKey().contains(prefix)) {
                 Preconditions.checkArgument(!producer.isDelegatedToChild(prefix),
-                        "Path %s is delegated to child producer.",
-                        prefix);
+                    "Path %s is delegated to child producer.", prefix);
                 return e.getValue();
             }
         }
+
         throw new IllegalArgumentException(String.format("Path %s is not accessible from transaction %s",
                 prefix, this));
     }
@@ -106,7 +102,7 @@ final class ShardedDOMDataTreeWriteTransaction implements DOMDataTreeCursorAware
         if (openCursor != null) {
             openCursor.close();
         }
-        for (final DOMDataTreeShardWriteTransaction tx : ImmutableSet.copyOf(idToTransaction.values())) {
+        for (final DOMDataTreeShardWriteTransaction tx : idToTransaction.values()) {
             tx.close();
         }
 
@@ -134,16 +130,15 @@ final class ShardedDOMDataTreeWriteTransaction implements DOMDataTreeCursorAware
     }
 
     CheckedFuture<Void, TransactionCommitFailedException> doSubmit(
-            Consumer<ShardedDOMDataTreeWriteTransaction> success,
-            BiConsumer<ShardedDOMDataTreeWriteTransaction, Throwable> failure) {
+            final Consumer<ShardedDOMDataTreeWriteTransaction> success,
+            final BiConsumer<ShardedDOMDataTreeWriteTransaction, Throwable> failure) {
 
-        final Set<DOMDataTreeShardWriteTransaction> txns = ImmutableSet.copyOf(idToTransaction.values());
-        final ListenableFuture<List<Void>> listListenableFuture =
-                Futures.allAsList(txns.stream().map(tx -> {
-                    LOG.debug("Readying tx {}", identifier);
-                    tx.ready();
-                    return tx.submit();
-                }).collect(Collectors.toList()));
+        final ListenableFuture<List<Void>> listListenableFuture = Futures.allAsList(
+            idToTransaction.values().stream().map(tx -> {
+                LOG.debug("Readying tx {}", identifier);
+                tx.ready();
+                return tx.submit();
+            }).collect(Collectors.toList()));
 
         final SettableFuture<Void> ret = SettableFuture.create();
         Futures.addCallback(listListenableFuture, new FutureCallback<List<Void>>() {
@@ -160,7 +155,7 @@ final class ShardedDOMDataTreeWriteTransaction implements DOMDataTreeCursorAware
             }
         });
 
-        return Futures.makeChecked(ret, TransactionCommitFailedExceptionMapper.create("submit"));
+        return Futures.makeChecked(ret, SUBMIT_FAILED_MAPPER);
     }
 
     void onTransactionSuccess(final Void result) {
@@ -175,10 +170,6 @@ final class ShardedDOMDataTreeWriteTransaction implements DOMDataTreeCursorAware
         openCursor = null;
     }
 
-    boolean isIsolated() {
-        return isolated;
-    }
-
     private class DelegatingCursor implements DOMDataTreeWriteCursor {
 
         private final DOMDataTreeWriteCursor delegate;