Improve InMemoryDataTreeModification.isSealed() performance 61/82561/7
authorRobert Varga <robert.varga@pantheon.tech>
Tue, 18 Jun 2019 21:32:22 +0000 (23:32 +0200)
committerRobert Varga <robert.varga@pantheon.tech>
Wed, 9 Oct 2019 06:06:11 +0000 (08:06 +0200)
The check for the modification being sealed does not need to really
establish a barrier between individual accesses. Use VarHandles to
express that the check needs to be ordered only before seal().

JIRA: YANGTOOLS-1025
Change-Id: I86433a95c3321e3bcabec9bba0f5a772556e1af4
Signed-off-by: Robert Varga <robert.varga@pantheon.tech>
yang/yang-data-impl/src/main/java/org/opendaylight/yangtools/yang/data/impl/schema/tree/InMemoryDataTreeModification.java

index 0b19cf16b0fcfd8bff8d3c1e4ec1d0ae43a30f0e..72dac50e493e5118ef1874d426c6dfc225ef53de 100644 (file)
@@ -11,10 +11,11 @@ import static com.google.common.base.Preconditions.checkArgument;
 import static com.google.common.base.Preconditions.checkState;
 import static java.util.Objects.requireNonNull;
 
+import java.lang.invoke.MethodHandles;
+import java.lang.invoke.VarHandle;
 import java.util.Collection;
 import java.util.Map.Entry;
 import java.util.Optional;
-import java.util.concurrent.atomic.AtomicIntegerFieldUpdater;
 import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier;
 import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier.PathArgument;
 import org.opendaylight.yangtools.yang.data.api.schema.NormalizedNode;
@@ -31,8 +32,6 @@ import org.slf4j.LoggerFactory;
 
 final class InMemoryDataTreeModification extends AbstractCursorAware implements CursorAwareDataTreeModification,
         SchemaContextProvider {
-    private static final AtomicIntegerFieldUpdater<InMemoryDataTreeModification> SEALED_UPDATER =
-            AtomicIntegerFieldUpdater.newUpdater(InMemoryDataTreeModification.class, "sealed");
     private static final Logger LOG = LoggerFactory.getLogger(InMemoryDataTreeModification.class);
 
     private final RootApplyStrategy strategyTree;
@@ -40,7 +39,19 @@ final class InMemoryDataTreeModification extends AbstractCursorAware implements
     private final ModifiedNode rootNode;
     private final Version version;
 
-    private volatile int sealed = 0;
+    private static final VarHandle SEALED;
+
+    static {
+        try {
+            SEALED = MethodHandles.lookup().findVarHandle(InMemoryDataTreeModification.class, "sealed", int.class);
+        } catch (ReflectiveOperationException e) {
+            throw new ExceptionInInitializerError(e);
+        }
+    }
+
+    // All access needs to go through this handle
+    @SuppressWarnings("unused")
+    private volatile int sealed;
 
     InMemoryDataTreeModification(final InMemoryDataTreeSnapshot snapshot,
             final RootApplyStrategy resolver) {
@@ -181,7 +192,7 @@ final class InMemoryDataTreeModification extends AbstractCursorAware implements
     }
 
     private void checkSealed() {
-        checkState(sealed == 0, "Data Tree is sealed. No further modifications allowed.");
+        checkState(!isSealed(), "Data Tree is sealed. No further modifications allowed.");
     }
 
     @Override
@@ -191,7 +202,7 @@ final class InMemoryDataTreeModification extends AbstractCursorAware implements
 
     @Override
     public InMemoryDataTreeModification newModification() {
-        checkState(sealed == 1, "Attempted to chain on an unsealed modification");
+        checkState(isSealed(), "Attempted to chain on an unsealed modification");
 
         if (rootNode.getOperation() == LogicalOperation.NONE) {
             // Simple fast case: just use the underlying modification
@@ -217,7 +228,8 @@ final class InMemoryDataTreeModification extends AbstractCursorAware implements
     }
 
     boolean isSealed() {
-        return sealed == 1;
+        // a quick check, synchronizes *only* on the sealed field
+        return (int) SEALED.getAcquire(this) != 0;
     }
 
     private static void applyChildren(final DataTreeModificationCursor cursor, final ModifiedNode node) {
@@ -292,7 +304,9 @@ final class InMemoryDataTreeModification extends AbstractCursorAware implements
 
     @Override
     public void ready() {
-        final boolean wasRunning = SEALED_UPDATER.compareAndSet(this, 0, 1);
+        // We want a full CAS with setVolatile() memory semantics, as we want to force happen-before
+        // for everything, including whatever user code works.
+        final boolean wasRunning = SEALED.compareAndSet(this, 0, 1);
         checkState(wasRunning, "Attempted to seal an already-sealed Data Tree.");
 
         AbstractReadyIterator current = AbstractReadyIterator.create(rootNode, getStrategy());