From af2229f91a0f82a1fe28ac45b308204a8b15b044 Mon Sep 17 00:00:00 2001 From: Robert Varga Date: Mon, 15 Sep 2014 00:06:54 +0200 Subject: [PATCH] BUG-650: use SameThreadExecutor for commits Profiling has shown that the cost of performing a forced context switch in execution path of the data store leads to ~2x performance degradation (23600 vs. 40000 ops/s), with average of 20 runs: InMemoryDataStoreWithExecutorServiceBenchmark: total stddev write100KSingleNodeWithOneInnerItemInCommitPerWriteBenchmark 4227.384 ms 61.172 write100KSingleNodeWithOneInnerItemInOneCommitBenchmark 286.954 ms 14.350 write10KSingleNodeWithTenInnerItemsInCommitPerWriteBenchmark 364.004 ms 12.687 write10KSingleNodeWithTenInnerItemsInOneCommitBenchmark 17.936 ms 0.883 write50KSingleNodeWithTwoInnerItemsInCommitPerWriteBenchmark 1979.140 ms 56.529 write50KSingleNodeWithTwoInnerItemsInOneCommitBenchmark 136.749 ms 6.402 InMemoryDataStoreWithSameThreadedExecutorBenchmark: total stddev write100KSingleNodeWithOneInnerItemInCommitPerWriteBenchmark 2475.137 ms 220.396 write100KSingleNodeWithOneInnerItemInOneCommitBenchmark 267.298 ms 7.063 write10KSingleNodeWithTenInnerItemsInCommitPerWriteBenchmark 180.537 ms 1.337 write10KSingleNodeWithTenInnerItemsInOneCommitBenchmark 19.582 ms 0.200 write50KSingleNodeWithTwoInnerItemsInCommitPerWriteBenchmark 1127.771 ms 87.438 write50KSingleNodeWithTwoInnerItemsInOneCommitBenchmark 134.401 ms 2.110 The analysis is that the underlying component (yang-data-impl's DataTree) can process operations at a rate exceeding 30K ops/s, obviously depending on size, which means a transaction is completed every ~35 microseconds. When we factor in the fact that there is at most one transaction issued at a particular moment (due to ordering/conflict resolution), the ill effects of forced context switches become very much pronounced. This patch switches the executor service to SameThreadExecutor, which foregoes queueing and executes the task on the submitting thread (which is the datastore coordinator thread, not some user thread). The option to switch the executor service is left intact, but may be removed in future pending further benchmarks. Change-Id: Ic1c4c0b1b80aa77c2d85810736bdc370a465eee8 Signed-off-by: Robert Varga --- .../dom/store/impl/InMemoryDOMDataStore.java | 22 ++++++++----------- .../impl/InMemoryDOMDataStoreFactory.java | 15 ++++++------- 2 files changed, 16 insertions(+), 21 deletions(-) diff --git a/opendaylight/md-sal/sal-inmemory-datastore/src/main/java/org/opendaylight/controller/md/sal/dom/store/impl/InMemoryDOMDataStore.java b/opendaylight/md-sal/sal-inmemory-datastore/src/main/java/org/opendaylight/controller/md/sal/dom/store/impl/InMemoryDOMDataStore.java index 3d61c7b6b6..74fa73afb9 100644 --- a/opendaylight/md-sal/sal-inmemory-datastore/src/main/java/org/opendaylight/controller/md/sal/dom/store/impl/InMemoryDOMDataStore.java +++ b/opendaylight/md-sal/sal-inmemory-datastore/src/main/java/org/opendaylight/controller/md/sal/dom/store/impl/InMemoryDOMDataStore.java @@ -14,7 +14,6 @@ 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.ListeningExecutorService; -import com.google.common.util.concurrent.MoreExecutors; import java.util.concurrent.Callable; import java.util.concurrent.ExecutorService; import java.util.concurrent.TimeUnit; @@ -80,29 +79,26 @@ public class InMemoryDOMDataStore extends TransactionReadyPrototype implements D private final DataTree dataTree = InMemoryDataTreeFactory.getInstance().create(); private final ListenerTree listenerTree = ListenerTree.create(); private final AtomicLong txCounter = new AtomicLong(0); - private final ListeningExecutorService listeningExecutor; private final QueuedNotificationManager, DOMImmutableDataChangeEvent> dataChangeListenerNotificationManager; private final ExecutorService dataChangeListenerExecutor; - - private final ExecutorService domStoreExecutor; + private final ListeningExecutorService commitExecutor; private final boolean debugTransactions; private final String name; private volatile AutoCloseable closeable; - public InMemoryDOMDataStore(final String name, final ExecutorService domStoreExecutor, + public InMemoryDOMDataStore(final String name, final ListeningExecutorService commitExecutor, final ExecutorService dataChangeListenerExecutor) { - this(name, domStoreExecutor, dataChangeListenerExecutor, + this(name, commitExecutor, dataChangeListenerExecutor, InMemoryDOMDataStoreConfigProperties.DEFAULT_MAX_DATA_CHANGE_LISTENER_QUEUE_SIZE, false); } - public InMemoryDOMDataStore(final String name, final ExecutorService domStoreExecutor, + public InMemoryDOMDataStore(final String name, final ListeningExecutorService commitExecutor, final ExecutorService dataChangeListenerExecutor, final int maxDataChangeListenerQueueSize, final boolean debugTransactions) { this.name = Preconditions.checkNotNull(name); - this.domStoreExecutor = Preconditions.checkNotNull(domStoreExecutor); - this.listeningExecutor = MoreExecutors.listeningDecorator(this.domStoreExecutor); + this.commitExecutor = Preconditions.checkNotNull(commitExecutor); this.dataChangeListenerExecutor = Preconditions.checkNotNull(dataChangeListenerExecutor); this.debugTransactions = debugTransactions; @@ -121,7 +117,7 @@ public class InMemoryDOMDataStore extends TransactionReadyPrototype implements D } public ExecutorService getDomStoreExecutor() { - return domStoreExecutor; + return commitExecutor; } @Override @@ -156,7 +152,7 @@ public class InMemoryDOMDataStore extends TransactionReadyPrototype implements D @Override public void close() { - ExecutorServiceUtil.tryGracefulShutdown(listeningExecutor, 30, TimeUnit.SECONDS); + ExecutorServiceUtil.tryGracefulShutdown(commitExecutor, 30, TimeUnit.SECONDS); ExecutorServiceUtil.tryGracefulShutdown(dataChangeListenerExecutor, 30, TimeUnit.SECONDS); if(closeable != null) { @@ -386,7 +382,7 @@ public class InMemoryDOMDataStore extends TransactionReadyPrototype implements D @Override public ListenableFuture canCommit() { - return listeningExecutor.submit(new Callable() { + return commitExecutor.submit(new Callable() { @Override public Boolean call() throws TransactionCommitFailedException { try { @@ -410,7 +406,7 @@ public class InMemoryDOMDataStore extends TransactionReadyPrototype implements D @Override public ListenableFuture preCommit() { - return listeningExecutor.submit(new Callable() { + return commitExecutor.submit(new Callable() { @Override public Void call() { candidate = dataTree.prepare(modification); diff --git a/opendaylight/md-sal/sal-inmemory-datastore/src/main/java/org/opendaylight/controller/md/sal/dom/store/impl/InMemoryDOMDataStoreFactory.java b/opendaylight/md-sal/sal-inmemory-datastore/src/main/java/org/opendaylight/controller/md/sal/dom/store/impl/InMemoryDOMDataStoreFactory.java index dc1482c6ab..2ee8e182c2 100644 --- a/opendaylight/md-sal/sal-inmemory-datastore/src/main/java/org/opendaylight/controller/md/sal/dom/store/impl/InMemoryDOMDataStoreFactory.java +++ b/opendaylight/md-sal/sal-inmemory-datastore/src/main/java/org/opendaylight/controller/md/sal/dom/store/impl/InMemoryDOMDataStoreFactory.java @@ -7,6 +7,8 @@ */ package org.opendaylight.controller.md.sal.dom.store.impl; +import com.google.common.util.concurrent.ListeningExecutorService; +import com.google.common.util.concurrent.MoreExecutors; import java.util.concurrent.ExecutorService; import javax.annotation.Nullable; import org.opendaylight.controller.sal.core.api.model.SchemaService; @@ -57,7 +59,7 @@ public final class InMemoryDOMDataStoreFactory { @Nullable final InMemoryDOMDataStoreConfigProperties properties) { InMemoryDOMDataStoreConfigProperties actualProperties = properties; - if(actualProperties == null) { + if (actualProperties == null) { actualProperties = InMemoryDOMDataStoreConfigProperties.getDefault(); } @@ -65,21 +67,18 @@ public final class InMemoryDOMDataStoreFactory { // task execution time to get higher throughput as DataChangeListeners typically provide // much of the business logic for a data model. If the executor queue size limit is reached, // subsequent submitted notifications will block the calling thread. - int dclExecutorMaxQueueSize = actualProperties.getMaxDataChangeExecutorQueueSize(); int dclExecutorMaxPoolSize = actualProperties.getMaxDataChangeExecutorPoolSize(); ExecutorService dataChangeListenerExecutor = SpecialExecutors.newBlockingBoundedFastThreadPool( dclExecutorMaxPoolSize, dclExecutorMaxQueueSize, name + "-DCL" ); - ExecutorService domStoreExecutor = SpecialExecutors.newBoundedSingleThreadExecutor( - actualProperties.getMaxDataStoreExecutorQueueSize(), "DOMStore-" + name ); - - InMemoryDOMDataStore dataStore = new InMemoryDOMDataStore(name, - domStoreExecutor, dataChangeListenerExecutor, + final ListeningExecutorService commitExecutor = MoreExecutors.sameThreadExecutor(); + final InMemoryDOMDataStore dataStore = new InMemoryDOMDataStore(name, + commitExecutor, dataChangeListenerExecutor, actualProperties.getMaxDataChangeListenerQueueSize(), debugTransactions); - if(schemaService != null) { + if (schemaService != null) { schemaService.registerSchemaContextListener(dataStore); } -- 2.36.6