From 2ff3d8442e0996681953cf73f5172f90c2507b4a Mon Sep 17 00:00:00 2001 From: Robert Varga Date: Fri, 12 Sep 2014 23:47:17 +0200 Subject: [PATCH] BUG-650: improve modification sealing performance The seal operation is on a fast path, so performing synchronization is not really advised. Downgrade synchronized blocks to volatile modifications, which have better effects on the optimizer. We introduce a SynchronizedDataTreeModification which the users can use when they require a serialized thread-safe access. Change-Id: Ieffcf1f5fef4d4df32fcdc30f992672d79dcf7bf Signed-off-by: Robert Varga --- .../SynchronizedDataTreeModification.java | 60 +++++++++++++++ .../tree/InMemoryDataTreeModification.java | 73 ++++++++++--------- 2 files changed, 99 insertions(+), 34 deletions(-) create mode 100644 yang/yang-data-api/src/main/java/org/opendaylight/yangtools/yang/data/api/schema/tree/SynchronizedDataTreeModification.java diff --git a/yang/yang-data-api/src/main/java/org/opendaylight/yangtools/yang/data/api/schema/tree/SynchronizedDataTreeModification.java b/yang/yang-data-api/src/main/java/org/opendaylight/yangtools/yang/data/api/schema/tree/SynchronizedDataTreeModification.java new file mode 100644 index 0000000000..d78f6fa11a --- /dev/null +++ b/yang/yang-data-api/src/main/java/org/opendaylight/yangtools/yang/data/api/schema/tree/SynchronizedDataTreeModification.java @@ -0,0 +1,60 @@ +/* + * Copyright (c) 2014 Cisco Systems, Inc. and others. All rights reserved. + * + * This program and the accompanying materials are made available under the + * 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.yangtools.yang.data.api.schema.tree; + +import com.google.common.base.Optional; +import com.google.common.base.Preconditions; +import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier; +import org.opendaylight.yangtools.yang.data.api.schema.NormalizedNode; + +/** + * A {@link DataTreeModification} implementation which delegates all calls to + * another instance, making sure no method is being invoked from multiple threads + * concurrently. + */ +public final class SynchronizedDataTreeModification implements DataTreeModification { + private final DataTreeModification delegate; + + private SynchronizedDataTreeModification(final DataTreeModification delegate) { + this.delegate = Preconditions.checkNotNull(delegate); + } + + public DataTreeModification create(final DataTreeModification delegate) { + return new SynchronizedDataTreeModification(delegate); + } + + @Override + public synchronized Optional> readNode(final YangInstanceIdentifier path) { + return delegate.readNode(path); + } + + @Override + public synchronized DataTreeModification newModification() { + return delegate.newModification(); + } + + @Override + public synchronized void delete(final YangInstanceIdentifier path) { + delegate.delete(path); + } + + @Override + public synchronized void merge(final YangInstanceIdentifier path, final NormalizedNode data) { + delegate.merge(path, data); + } + + @Override + public synchronized void write(final YangInstanceIdentifier path, final NormalizedNode data) { + delegate.write(path, data); + } + + @Override + public synchronized void ready() { + delegate.ready(); + } +} diff --git a/yang/yang-data-impl/src/main/java/org/opendaylight/yangtools/yang/data/impl/schema/tree/InMemoryDataTreeModification.java b/yang/yang-data-impl/src/main/java/org/opendaylight/yangtools/yang/data/impl/schema/tree/InMemoryDataTreeModification.java index de4bcef796..9e0c60f6a0 100644 --- a/yang/yang-data-impl/src/main/java/org/opendaylight/yangtools/yang/data/impl/schema/tree/InMemoryDataTreeModification.java +++ b/yang/yang-data-impl/src/main/java/org/opendaylight/yangtools/yang/data/impl/schema/tree/InMemoryDataTreeModification.java @@ -7,10 +7,10 @@ */ package org.opendaylight.yangtools.yang.data.impl.schema.tree; +import com.google.common.base.Optional; +import com.google.common.base.Preconditions; import java.util.Map.Entry; - -import javax.annotation.concurrent.GuardedBy; - +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; @@ -23,32 +23,31 @@ import org.opendaylight.yangtools.yang.data.impl.schema.NormalizedNodeUtils; import org.slf4j.Logger; import org.slf4j.LoggerFactory; -import com.google.common.base.Optional; -import com.google.common.base.Preconditions; - final class InMemoryDataTreeModification implements DataTreeModification { + private static final AtomicIntegerFieldUpdater UPDATER = + AtomicIntegerFieldUpdater.newUpdater(InMemoryDataTreeModification.class, "sealed"); private static final Logger LOG = LoggerFactory.getLogger(InMemoryDataTreeModification.class); + private final RootModificationApplyOperation strategyTree; private final InMemoryDataTreeSnapshot snapshot; private final ModifiedNode rootNode; private final Version version; - @GuardedBy("this") - private boolean sealed = false; + private volatile int sealed = 0; InMemoryDataTreeModification(final InMemoryDataTreeSnapshot snapshot, final RootModificationApplyOperation resolver) { this.snapshot = Preconditions.checkNotNull(snapshot); this.strategyTree = Preconditions.checkNotNull(resolver).snapshot(); this.rootNode = ModifiedNode.createUnmodified(snapshot.getRootNode(), false); + /* * We could allocate version beforehand, since Version contract - * states two allocated version must be allways different. - * + * states two allocated version must be always different. + * * Preallocating version simplifies scenarios such as * chaining of modifications, since version for particular * node in modification and in data tree (if successfully - * commited) will be same and will not change. - * + * committed) will be same and will not change. */ this.version = snapshot.getRootNode().getSubtreeVersion().next(); } @@ -62,20 +61,21 @@ final class InMemoryDataTreeModification implements DataTreeModification { } @Override - public synchronized void write(final YangInstanceIdentifier path, final NormalizedNode value) { + public void write(final YangInstanceIdentifier path, final NormalizedNode value) { checkSealed(); + resolveModificationFor(path).write(value); } @Override - public synchronized void merge(final YangInstanceIdentifier path, final NormalizedNode data) { + public void merge(final YangInstanceIdentifier path, final NormalizedNode data) { checkSealed(); + mergeImpl(resolveModificationFor(path),data); } private void mergeImpl(final OperationWithModification op,final NormalizedNode data) { - - if(data instanceof NormalizedNodeContainer) { + if (data instanceof NormalizedNodeContainer) { @SuppressWarnings({ "rawtypes", "unchecked" }) NormalizedNodeContainer> dataContainer = (NormalizedNodeContainer) data; for(NormalizedNode child : dataContainer.getValue()) { @@ -87,13 +87,14 @@ final class InMemoryDataTreeModification implements DataTreeModification { } @Override - public synchronized void delete(final YangInstanceIdentifier path) { + public void delete(final YangInstanceIdentifier path) { checkSealed(); + resolveModificationFor(path).delete(); } @Override - public synchronized Optional> readNode(final YangInstanceIdentifier path) { + public Optional> readNode(final YangInstanceIdentifier path) { /* * Walk the tree from the top, looking for the first node between root and * the requested path which has been modified. If no such node exists, @@ -115,7 +116,7 @@ final class InMemoryDataTreeModification implements DataTreeModification { private Optional resolveSnapshot(final YangInstanceIdentifier path, final ModifiedNode modification) { final Optional> potentialSnapshot = modification.getSnapshotCache(); - if(potentialSnapshot.isPresent()) { + if (potentialSnapshot.isPresent()) { return potentialSnapshot.get(); } @@ -123,14 +124,14 @@ final class InMemoryDataTreeModification implements DataTreeModification { return resolveModificationStrategy(path).apply(modification, modification.getOriginal(), version); } catch (Exception e) { - LOG.error("Could not create snapshot for {}:{}", path,modification,e); + LOG.error("Could not create snapshot for {}:{}", path, modification, e); throw e; } } private ModificationApplyOperation resolveModificationStrategy(final YangInstanceIdentifier path) { LOG.trace("Resolving modification apply strategy for {}", path); - if(rootNode.getType() == ModificationType.UNMODIFIED) { + if (rootNode.getType() == ModificationType.UNMODIFIED) { strategyTree.upgradeIfPossible(); } @@ -138,14 +139,17 @@ final class InMemoryDataTreeModification implements DataTreeModification { } private OperationWithModification resolveModificationFor(final YangInstanceIdentifier path) { - ModifiedNode modification = rootNode; // We ensure strategy is present. - ModificationApplyOperation operation = resolveModificationStrategy(path); - boolean isOrdered = true; + final ModificationApplyOperation operation = resolveModificationStrategy(path); + + final boolean isOrdered; if (operation instanceof SchemaAwareApplyOperation) { isOrdered = ((SchemaAwareApplyOperation) operation).isOrdered(); + } else { + isOrdered = true; } + ModifiedNode modification = rootNode; for (PathArgument pathArg : path.getPathArguments()) { modification = modification.modifyChild(pathArg, isOrdered); } @@ -153,15 +157,15 @@ final class InMemoryDataTreeModification implements DataTreeModification { } @Override - public synchronized void ready() { - Preconditions.checkState(!sealed, "Attempted to seal an already-sealed Data Tree."); - sealed = true; + public void ready() { + final boolean wasRunning = UPDATER.compareAndSet(this, 0, 1); + Preconditions.checkState(wasRunning, "Attempted to seal an already-sealed Data Tree."); + rootNode.seal(); } - @GuardedBy("this") private void checkSealed() { - Preconditions.checkState(!sealed, "Data Tree is sealed. No further modifications allowed."); + Preconditions.checkState(sealed == 0, "Data Tree is sealed. No further modifications allowed."); } @Override @@ -170,16 +174,17 @@ final class InMemoryDataTreeModification implements DataTreeModification { } @Override - public synchronized DataTreeModification newModification() { - Preconditions.checkState(sealed, "Attempted to chain on an unsealed modification"); + public DataTreeModification newModification() { + Preconditions.checkState(sealed == 1, "Attempted to chain on an unsealed modification"); - if(rootNode.getType() == ModificationType.UNMODIFIED) { + if (rootNode.getType() == ModificationType.UNMODIFIED) { + // Simple fast case: just use the underlying modification return snapshot.newModification(); } /* - * We will use preallocated version, this means returned snapshot will - * have same version each time this method is called. + * We will use preallocated version, this means returned snapshot will + * have same version each time this method is called. */ TreeNode originalSnapshotRoot = snapshot.getRootNode(); Optional tempRoot = strategyTree.apply(rootNode, Optional.of(originalSnapshotRoot), version); -- 2.36.6