BUG-650: speedup AbstractDOMForwardedTransactionFactory

Another speedup -- as it turns out we do not have to take the full lock
to close down the transaction factory. A volatile write is enough to do
that -- which means a volatile read is enough to check for it having
been closed.

Change-Id: I3488ccccc4d91d34665b3ff6e70e047407be48bb
Signed-off-by: Robert Varga <rovarga@cisco.com>

diff --git a/opendaylight/md-sal/sal-dom-broker/src/main/java/org/opendaylight/controller/md/sal/dom/broker/impl/AbstractDOMForwardedTransactionFactory.java b/opendaylight/md-sal/sal-dom-broker/src/main/java/org/opendaylight/controller/md/sal/dom/broker/impl/AbstractDOMForwardedTransactionFactory.java
index f48667b04f21bea53606c043f3f5bc06c2476df5..6838e39093abcf35f6ad3d1e06894d8a346b46b5 100644 (file)
--- a/opendaylight/md-sal/sal-dom-broker/src/main/java/org/opendaylight/controller/md/sal/dom/broker/impl/AbstractDOMForwardedTransactionFactory.java
+++ b/opendaylight/md-sal/sal-dom-broker/src/main/java/org/opendaylight/controller/md/sal/dom/broker/impl/AbstractDOMForwardedTransactionFactory.java
@@ -12,7 +12,7 @@ import com.google.common.collect.ImmutableMap;
 import java.util.EnumMap;
 import java.util.Map;
 import java.util.Map.Entry;
 import java.util.EnumMap;
 import java.util.Map;
 import java.util.Map.Entry;

-import javax.annotation.concurrent.GuardedBy;
+import java.util.concurrent.atomic.AtomicIntegerFieldUpdater;

 import org.opendaylight.controller.md.sal.common.api.data.LogicalDatastoreType;
 import org.opendaylight.controller.md.sal.dom.api.DOMDataReadOnlyTransaction;
 import org.opendaylight.controller.md.sal.dom.api.DOMDataReadWriteTransaction;
 import org.opendaylight.controller.md.sal.common.api.data.LogicalDatastoreType;
 import org.opendaylight.controller.md.sal.dom.api.DOMDataReadOnlyTransaction;
 import org.opendaylight.controller.md.sal.dom.api.DOMDataReadWriteTransaction;


@@ -38,11 +38,12 @@ import org.opendaylight.controller.sal.core.spi.data.DOMStoreWriteTransaction;
  * @param <T>
  *            Type of {@link DOMStoreTransactionFactory} factory.
  */
  * @param <T>
  *            Type of {@link DOMStoreTransactionFactory} factory.
  */

-public abstract class AbstractDOMForwardedTransactionFactory<T extends DOMStoreTransactionFactory> implements DOMDataCommitImplementation, AutoCloseable {
-
+abstract class AbstractDOMForwardedTransactionFactory<T extends DOMStoreTransactionFactory> implements DOMDataCommitImplementation, AutoCloseable {
+    @SuppressWarnings("rawtypes")
+    private static final AtomicIntegerFieldUpdater<AbstractDOMForwardedTransactionFactory> UPDATER =
+            AtomicIntegerFieldUpdater.newUpdater(AbstractDOMForwardedTransactionFactory.class, "closed");

     private final Map<LogicalDatastoreType, T> storeTxFactories;
     private final Map<LogicalDatastoreType, T> storeTxFactories;

-
-    private boolean closed;
+    private volatile int closed = 0;

 
     protected AbstractDOMForwardedTransactionFactory(final Map<LogicalDatastoreType, ? extends T> txFactories) {
         this.storeTxFactories = ImmutableMap.copyOf(txFactories);
 
     protected AbstractDOMForwardedTransactionFactory(final Map<LogicalDatastoreType, ? extends T> txFactories) {
         this.storeTxFactories = ImmutableMap.copyOf(txFactories);


@@ -72,7 +73,7 @@ public abstract class AbstractDOMForwardedTransactionFactory<T extends DOMStoreT
      *
      * @return New composite read-only transaction.
      */
      *
      * @return New composite read-only transaction.
      */

-    public DOMDataReadOnlyTransaction newReadOnlyTransaction() {
+    public final DOMDataReadOnlyTransaction newReadOnlyTransaction() {

         checkNotClosed();
 
         final Map<LogicalDatastoreType, DOMStoreReadTransaction> txns = new EnumMap<>(LogicalDatastoreType.class);
         checkNotClosed();
 
         final Map<LogicalDatastoreType, DOMStoreReadTransaction> txns = new EnumMap<>(LogicalDatastoreType.class);


@@ -82,8 +83,6 @@ public abstract class AbstractDOMForwardedTransactionFactory<T extends DOMStoreT
         return new DOMForwardedReadOnlyTransaction(newTransactionIdentifier(), txns);
     }
 
         return new DOMForwardedReadOnlyTransaction(newTransactionIdentifier(), txns);
     }
 

-
-

     /**
      * Creates a new composite write-only transaction
      *
     /**
      * Creates a new composite write-only transaction
      *


@@ -123,7 +122,7 @@ public abstract class AbstractDOMForwardedTransactionFactory<T extends DOMStoreT
      * @return New composite write-only transaction associated with this
      *         factory.
      */
      * @return New composite write-only transaction associated with this
      *         factory.
      */

-    public DOMDataWriteTransaction newWriteOnlyTransaction() {
+    public final DOMDataWriteTransaction newWriteOnlyTransaction() {

         checkNotClosed();
 
         final Map<LogicalDatastoreType, DOMStoreWriteTransaction> txns = new EnumMap<>(LogicalDatastoreType.class);
         checkNotClosed();
 
         final Map<LogicalDatastoreType, DOMStoreWriteTransaction> txns = new EnumMap<>(LogicalDatastoreType.class);


@@ -176,9 +175,8 @@ public abstract class AbstractDOMForwardedTransactionFactory<T extends DOMStoreT
      *
      * @return New composite read-write transaction associated with this
      *         factory.
      *
      * @return New composite read-write transaction associated with this
      *         factory.

-     *

      */
      */

-    public DOMDataReadWriteTransaction newReadWriteTransaction() {
+    public final DOMDataReadWriteTransaction newReadWriteTransaction() {

         checkNotClosed();
 
         final Map<LogicalDatastoreType, DOMStoreReadWriteTransaction> txns = new EnumMap<>(LogicalDatastoreType.class);
         checkNotClosed();
 
         final Map<LogicalDatastoreType, DOMStoreReadWriteTransaction> txns = new EnumMap<>(LogicalDatastoreType.class);


@@ -203,21 +201,19 @@ public abstract class AbstractDOMForwardedTransactionFactory<T extends DOMStoreT
     }
 
     /**
     }
 
     /**

-     *

      * Checks if instance is not closed.
      *
      * @throws IllegalStateException If instance of this class was closed.
      *
      */
      * Checks if instance is not closed.
      *
      * @throws IllegalStateException If instance of this class was closed.
      *
      */

-    @GuardedBy("this")
-    protected synchronized void checkNotClosed() {
-        Preconditions.checkState(!closed,"Transaction factory was closed. No further operations allowed.");
+    protected final void checkNotClosed() {
+        Preconditions.checkState(closed == 0, "Transaction factory was closed. No further operations allowed.");

     }
 
     @Override
     }
 
     @Override

-    @GuardedBy("this")
-    public synchronized void close() {
-        closed = true;
+    public void close() {
+        final int wasClosed = UPDATER.getAndSet(this, 1);
+        Preconditions.checkState(wasClosed == 0, "Transaction factory was already closed");

     }
 
 }
     }
 
 }

diff --git a/opendaylight/md-sal/sal-dom-broker/src/main/java/org/opendaylight/controller/md/sal/dom/broker/impl/DOMForwardedReadOnlyTransaction.java b/opendaylight/md-sal/sal-dom-broker/src/main/java/org/opendaylight/controller/md/sal/dom/broker/impl/DOMForwardedReadOnlyTransaction.java
index b5a8e42981425d704f7cc0d51507000ca32c158d..124bf9f0bef7de6ac3482cdf3ac907f448b998f6 100644 (file)
--- a/opendaylight/md-sal/sal-dom-broker/src/main/java/org/opendaylight/controller/md/sal/dom/broker/impl/DOMForwardedReadOnlyTransaction.java
+++ b/opendaylight/md-sal/sal-dom-broker/src/main/java/org/opendaylight/controller/md/sal/dom/broker/impl/DOMForwardedReadOnlyTransaction.java
@@ -38,7 +38,8 @@ class DOMForwardedReadOnlyTransaction extends
         return getSubtransaction(store).read(path);
     }
 
         return getSubtransaction(store).read(path);
     }
 

-    @Override public CheckedFuture<Boolean, ReadFailedException> exists(
+    @Override
+    public CheckedFuture<Boolean, ReadFailedException> exists(

         final LogicalDatastoreType store,
         final YangInstanceIdentifier path) {
         return getSubtransaction(store).exists(path);
         final LogicalDatastoreType store,
         final YangInstanceIdentifier path) {
         return getSubtransaction(store).exists(path);