From 6ffa8194f3ae4630f958bf4ab36c79709b951799 Mon Sep 17 00:00:00 2001 From: Robert Varga Date: Fri, 16 Sep 2016 22:39:45 +0200 Subject: [PATCH] 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 (cherry picked from commit edecff765d21129c17050d77400f15b670c74bdf) --- .../broker/ShardedDOMDataTreeProducer.java | 13 ++-- .../ShardedDOMDataTreeWriteTransaction.java | 67 ++++++++----------- 2 files changed, 38 insertions(+), 42 deletions(-) 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 3ae355a071..b19d956192 100644 --- 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 children = Collections.emptyMap(); + private Map children = ImmutableMap.of(); + @GuardedBy("this") + private Set 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 b2deac6179..83080f7be1 100644 --- 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 idToTransaction; + private final Collection childBoundaries; private final ShardedDOMDataTreeProducer producer; private final String identifier; - private final Set childBoundaries = new HashSet<>(); + + private final SettableFuture future = SettableFuture.create(); + private final CheckedFuture submitFuture = + Futures.makeChecked(future, SUBMIT_FAILED_MAPPER); + @GuardedBy("this") private boolean closed = false; @GuardedBy("this") private DOMDataTreeWriteCursor openCursor; - private final SettableFuture future = SettableFuture.create(); - private final CheckedFuture submitFuture = - Futures.makeChecked(future, TransactionCommitFailedExceptionMapper.create("submit")); - - private final boolean isolated; - ShardedDOMDataTreeWriteTransaction(final ShardedDOMDataTreeProducer producer, final Map idToProducer, - final Map childProducers, - final boolean isolated) { - this.isolated = isolated; + final Set 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 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 doSubmit( - Consumer success, - BiConsumer failure) { + final Consumer success, + final BiConsumer failure) { - final Set txns = ImmutableSet.copyOf(idToTransaction.values()); - final ListenableFuture> listListenableFuture = - Futures.allAsList(txns.stream().map(tx -> { - LOG.debug("Readying tx {}", identifier); - tx.ready(); - return tx.submit(); - }).collect(Collectors.toList())); + final ListenableFuture> listListenableFuture = Futures.allAsList( + idToTransaction.values().stream().map(tx -> { + LOG.debug("Readying tx {}", identifier); + tx.ready(); + return tx.submit(); + }).collect(Collectors.toList())); final SettableFuture ret = SettableFuture.create(); Futures.addCallback(listListenableFuture, new FutureCallback>() { @@ -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; -- 2.36.6