Bug 7271: Modify AbstractDOMStoreTreeChangePublisher to batch candidates

The AbstractDOMStoreTreeChangePublisher was modified to keep track of
DataTreeCandidates per listener via a Map in order to notify listeners
of all DataTreeCandidate changes in one event. This allows the
initial event on registration to send all list entry nodes. The
previous notifyListeners abstract method was removed in lieu of a
new notifyListener that takes a listener registration and the list
of DataTreeCandidates

Change-Id: I02f14f202a46bbf9afe8de1941701849a41a431c
Signed-off-by: Tom Pantelis <tpanteli@brocade.com>

diff --git a/dom/mdsal-dom-inmemory-datastore/src/main/java/org/opendaylight/mdsal/dom/store/inmemory/InMemoryDOMDataTreeShardChangePublisher.java b/dom/mdsal-dom-inmemory-datastore/src/main/java/org/opendaylight/mdsal/dom/store/inmemory/InMemoryDOMDataTreeShardChangePublisher.java
index a833b01eb9f3c2f650174c33abaf23828f4c0b36..6d2f7d2de4f1d4a62637091d8df400286d7b2616 100644 (file)
--- a/dom/mdsal-dom-inmemory-datastore/src/main/java/org/opendaylight/mdsal/dom/store/inmemory/InMemoryDOMDataTreeShardChangePublisher.java
+++ b/dom/mdsal-dom-inmemory-datastore/src/main/java/org/opendaylight/mdsal/dom/store/inmemory/InMemoryDOMDataTreeShardChangePublisher.java
@@ -21,8 +21,6 @@ import org.opendaylight.yangtools.util.concurrent.QueuedNotificationManager.Batc
 import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier;
 import org.opendaylight.yangtools.yang.data.api.schema.tree.DataTree;
 import org.opendaylight.yangtools.yang.data.api.schema.tree.DataTreeCandidate;
-import org.opendaylight.yangtools.yang.data.api.schema.tree.DataTreeCandidateNode;
-import org.opendaylight.yangtools.yang.data.api.schema.tree.DataTreeCandidates;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -38,8 +36,8 @@ final class InMemoryDOMDataTreeShardChangePublisher extends AbstractDOMShardTree
             }
         };
 
-    private final QueuedNotificationManager<AbstractDOMDataTreeChangeListenerRegistration<?>,
-        DataTreeCandidate> notificationManager;
+    private final QueuedNotificationManager<AbstractDOMDataTreeChangeListenerRegistration<?>, DataTreeCandidate>
+        notificationManager;
 
     InMemoryDOMDataTreeShardChangePublisher(final Executor executor,
                                             final int maxQueueSize,
@@ -52,16 +50,10 @@ final class InMemoryDOMDataTreeShardChangePublisher extends AbstractDOMShardTree
     }
 
     @Override
-    protected void notifyListeners(
-            @Nonnull final Collection<AbstractDOMDataTreeChangeListenerRegistration<?>> registrations,
-                                   @Nonnull final YangInstanceIdentifier path,
-                                   @Nonnull final DataTreeCandidateNode node) {
-        final DataTreeCandidate candidate = DataTreeCandidates.newDataTreeCandidate(path, node);
-
-        for (final AbstractDOMDataTreeChangeListenerRegistration<?> reg : registrations) {
-            LOG.debug("Enqueueing candidate {} to registration {}", candidate, registrations);
-            notificationManager.submitNotification(reg, candidate);
-        }
+    protected void notifyListener(AbstractDOMDataTreeChangeListenerRegistration<?> registration,
+            Collection<DataTreeCandidate> changes) {
+        LOG.debug("Enqueueing candidates {} for registration {}", changes, registration);
+        notificationManager.submitNotifications(registration, changes);
     }
 
     @Override

diff --git a/dom/mdsal-dom-inmemory-datastore/src/main/java/org/opendaylight/mdsal/dom/store/inmemory/InMemoryDOMStoreTreeChangePublisher.java b/dom/mdsal-dom-inmemory-datastore/src/main/java/org/opendaylight/mdsal/dom/store/inmemory/InMemoryDOMStoreTreeChangePublisher.java
index 5e11a216f3add8366eabc73dc8d664cbe8e68cb7..d7d48191ccce8781b2c98f6eb88f0ba6c9abd80d 100644 (file)
--- a/dom/mdsal-dom-inmemory-datastore/src/main/java/org/opendaylight/mdsal/dom/store/inmemory/InMemoryDOMStoreTreeChangePublisher.java
+++ b/dom/mdsal-dom-inmemory-datastore/src/main/java/org/opendaylight/mdsal/dom/store/inmemory/InMemoryDOMStoreTreeChangePublisher.java
@@ -21,7 +21,6 @@ import org.opendaylight.yangtools.util.concurrent.QueuedNotificationManager.Batc
 import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier;
 import org.opendaylight.yangtools.yang.data.api.schema.NormalizedNode;
 import org.opendaylight.yangtools.yang.data.api.schema.tree.DataTreeCandidate;
-import org.opendaylight.yangtools.yang.data.api.schema.tree.DataTreeCandidateNode;
 import org.opendaylight.yangtools.yang.data.api.schema.tree.DataTreeCandidates;
 import org.opendaylight.yangtools.yang.data.api.schema.tree.DataTreeSnapshot;
 import org.slf4j.Logger;
@@ -30,7 +29,6 @@ import org.slf4j.LoggerFactory;
 final class InMemoryDOMStoreTreeChangePublisher extends AbstractDOMStoreTreeChangePublisher {
     private static final BatchedInvoker<AbstractDOMDataTreeChangeListenerRegistration<?>, DataTreeCandidate>
         MANAGER_INVOKER = (listener, notifications) -> {
-            // FIXME: this is inefficient, as we could grab the entire queue for the listener and post it
             final DOMDataTreeChangeListener inst = listener.getInstance();
             if (inst != null) {
                 inst.onDataTreeChanged(ImmutableList.copyOf(notifications));
@@ -47,14 +45,10 @@ final class InMemoryDOMStoreTreeChangePublisher extends AbstractDOMStoreTreeChan
     }
 
     @Override
-    protected void notifyListeners(final Collection<AbstractDOMDataTreeChangeListenerRegistration<?>> registrations,
-            final YangInstanceIdentifier path, final DataTreeCandidateNode node) {
-        final DataTreeCandidate candidate = DataTreeCandidates.newDataTreeCandidate(path, node);
-
-        for (AbstractDOMDataTreeChangeListenerRegistration<?> reg : registrations) {
-            LOG.debug("Enqueueing candidate {} to registration {}", candidate, registrations);
-            notificationManager.submitNotification(reg, candidate);
-        }
+    protected void notifyListener(AbstractDOMDataTreeChangeListenerRegistration<?> registration,
+            Collection<DataTreeCandidate> changes) {
+        LOG.debug("Enqueueing candidates {} for registration {}", changes, registration);
+        notificationManager.submitNotifications(registration, changes);
     }
 
     @Override

diff --git a/dom/mdsal-dom-inmemory-datastore/src/test/java/org/opendaylight/mdsal/dom/store/inmemory/AbstractDOMShardTreeChangePublisherTest.java b/dom/mdsal-dom-inmemory-datastore/src/test/java/org/opendaylight/mdsal/dom/store/inmemory/AbstractDOMShardTreeChangePublisherTest.java
index 89d744d793df507fd1700bdea9977bb35148c11c..4aefa8627c291d0f1497926a5e0c6a5f8273ef52 100644 (file)
--- a/dom/mdsal-dom-inmemory-datastore/src/test/java/org/opendaylight/mdsal/dom/store/inmemory/AbstractDOMShardTreeChangePublisherTest.java
+++ b/dom/mdsal-dom-inmemory-datastore/src/test/java/org/opendaylight/mdsal/dom/store/inmemory/AbstractDOMShardTreeChangePublisherTest.java
@@ -38,7 +38,6 @@ import org.opendaylight.yangtools.yang.common.QName;
 import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier;
 import org.opendaylight.yangtools.yang.data.api.schema.DataContainerNode;
 import org.opendaylight.yangtools.yang.data.api.schema.tree.DataTreeCandidate;
-import org.opendaylight.yangtools.yang.data.api.schema.tree.DataTreeCandidateNode;
 import org.opendaylight.yangtools.yang.data.api.schema.tree.DataTreeSnapshot;
 import org.opendaylight.yangtools.yang.data.api.schema.tree.ModificationType;
 import org.opendaylight.yangtools.yang.data.impl.schema.builder.impl.ImmutableContainerNodeBuilder;
@@ -128,9 +127,8 @@ public class AbstractDOMShardTreeChangePublisherTest extends AbstractDOMShardTre
     }
 
     @Override
-    protected void notifyListeners(
-            @Nonnull final Collection<AbstractDOMDataTreeChangeListenerRegistration<?>> registrations,
-                   @Nonnull final YangInstanceIdentifier path, @Nonnull final DataTreeCandidateNode node) {
+    protected void notifyListener(AbstractDOMDataTreeChangeListenerRegistration<?> registration,
+            Collection<DataTreeCandidate> changes) {
         // NOOP
     }
 
@@ -143,4 +141,4 @@ public class AbstractDOMShardTreeChangePublisherTest extends AbstractDOMShardTre
     public void reset() {
         resetMocks();
     }
-}
\ No newline at end of file
+}

diff --git a/dom/mdsal-dom-spi/src/main/java/org/opendaylight/mdsal/dom/spi/store/AbstractDOMStoreTreeChangePublisher.java b/dom/mdsal-dom-spi/src/main/java/org/opendaylight/mdsal/dom/spi/store/AbstractDOMStoreTreeChangePublisher.java
index bffb506c7f97dd1d50d2962edcc308aa031ec879..191562ebc657d703272efbce7f025483a515d2dd 100644 (file)
--- a/dom/mdsal-dom-spi/src/main/java/org/opendaylight/mdsal/dom/spi/store/AbstractDOMStoreTreeChangePublisher.java
+++ b/dom/mdsal-dom-spi/src/main/java/org/opendaylight/mdsal/dom/spi/store/AbstractDOMStoreTreeChangePublisher.java
@@ -7,9 +7,15 @@
  */
 package org.opendaylight.mdsal.dom.spi.store;
 
+import com.google.common.base.Supplier;
 import com.google.common.collect.ImmutableList;
+import com.google.common.collect.Multimap;
+import com.google.common.collect.Multimaps;
+import java.util.ArrayList;
 import java.util.Collection;
+import java.util.IdentityHashMap;
 import java.util.List;
+import java.util.Map;
 import javax.annotation.Nonnull;
 import org.opendaylight.mdsal.dom.api.DOMDataTreeChangeListener;
 import org.opendaylight.mdsal.dom.spi.AbstractDOMDataTreeChangeListenerRegistration;
@@ -20,6 +26,7 @@ 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.tree.DataTreeCandidate;
 import org.opendaylight.yangtools.yang.data.api.schema.tree.DataTreeCandidateNode;
+import org.opendaylight.yangtools.yang.data.api.schema.tree.DataTreeCandidates;
 import org.opendaylight.yangtools.yang.data.api.schema.tree.ModificationType;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -32,17 +39,16 @@ public abstract class AbstractDOMStoreTreeChangePublisher
         implements DOMStoreTreeChangePublisher {
     private static final Logger LOG = LoggerFactory.getLogger(AbstractDOMStoreTreeChangePublisher.class);
 
+    private static final Supplier<List<DataTreeCandidate>> LIST_SUPPLIER = () -> new ArrayList<>();
+
     /**
-     * Callback for subclass to notify specified registrations
-     * of a candidate at a specified path. This method is guaranteed
+     * Callback for subclass to notify a specified registration of a list of candidates. This method is guaranteed
      * to be only called from within {@link #processCandidateTree(DataTreeCandidate)}.
-     * @param registrations Registrations which are affected by the candidate node
-     * @param path Path of changed candidate node. Guaranteed to match the path specified by the registration
-     * @param node Candidate node
+     * @param registration the registration to notify
+     * @param changes the list of DataTreeCandidate changes
      */
-    protected abstract void notifyListeners(
-            @Nonnull Collection<AbstractDOMDataTreeChangeListenerRegistration<?>> registrations,
-            @Nonnull YangInstanceIdentifier path, @Nonnull DataTreeCandidateNode node);
+    protected abstract void notifyListener(@Nonnull AbstractDOMDataTreeChangeListenerRegistration<?> registration,
+            @Nonnull Collection<DataTreeCandidate> changes);
 
     /**
      * Callback notifying the subclass that the specified registration is being
@@ -72,9 +78,15 @@ public abstract class AbstractDOMStoreTreeChangePublisher
 
         try (final RegistrationTreeSnapshot<AbstractDOMDataTreeChangeListenerRegistration<?>> snapshot
                 = takeSnapshot()) {
-            final List<PathArgument> toLookup
-                = ImmutableList.copyOf(candidate.getRootPath().getPathArguments());
-            lookupAndNotify(toLookup, 0, snapshot.getRootNode(), candidate);
+            final List<PathArgument> toLookup = ImmutableList.copyOf(candidate.getRootPath().getPathArguments());
+            final Multimap<AbstractDOMDataTreeChangeListenerRegistration<?>, DataTreeCandidate> listenerChanges =
+                    Multimaps.newListMultimap(new IdentityHashMap<>(), LIST_SUPPLIER);
+            lookupAndNotify(toLookup, 0, snapshot.getRootNode(), candidate, listenerChanges);
+
+            for (Map.Entry<AbstractDOMDataTreeChangeListenerRegistration<?>, Collection<DataTreeCandidate>> entry:
+                    listenerChanges.asMap().entrySet()) {
+                notifyListener(entry.getKey(), entry.getValue());
+            }
         }
     }
 
@@ -105,28 +117,30 @@ public abstract class AbstractDOMStoreTreeChangePublisher
 
     private void lookupAndNotify(final List<PathArgument> args,
             final int offset, final RegistrationTreeNode<AbstractDOMDataTreeChangeListenerRegistration<?>> node,
-                    final DataTreeCandidate candidate) {
+            final DataTreeCandidate candidate,
+            final Multimap<AbstractDOMDataTreeChangeListenerRegistration<?>, DataTreeCandidate> listenerChanges) {
         if (args.size() != offset) {
             final PathArgument arg = args.get(offset);
 
             final RegistrationTreeNode<AbstractDOMDataTreeChangeListenerRegistration<?>> exactChild
                 = node.getExactChild(arg);
             if (exactChild != null) {
-                lookupAndNotify(args, offset + 1, exactChild, candidate);
+                lookupAndNotify(args, offset + 1, exactChild, candidate, listenerChanges);
             }
 
             for (RegistrationTreeNode<AbstractDOMDataTreeChangeListenerRegistration<?>> c :
                     node.getInexactChildren(arg)) {
-                lookupAndNotify(args, offset + 1, c, candidate);
+                lookupAndNotify(args, offset + 1, c, candidate, listenerChanges);
             }
         } else {
-            notifyNode(candidate.getRootPath(), node, candidate.getRootNode());
+            notifyNode(candidate.getRootPath(), node, candidate.getRootNode(), listenerChanges);
         }
     }
 
     private void notifyNode(final YangInstanceIdentifier path,
             final RegistrationTreeNode<AbstractDOMDataTreeChangeListenerRegistration<?>> regNode,
-            final DataTreeCandidateNode candNode) {
+            final DataTreeCandidateNode candNode,
+            final Multimap<AbstractDOMDataTreeChangeListenerRegistration<?>, DataTreeCandidate> listenerChanges) {
         if (candNode.getModificationType() == ModificationType.UNMODIFIED) {
             LOG.debug("Skipping unmodified candidate {}", path);
             return;
@@ -134,7 +148,7 @@ public abstract class AbstractDOMStoreTreeChangePublisher
 
         final Collection<AbstractDOMDataTreeChangeListenerRegistration<?>> regs = regNode.getRegistrations();
         if (!regs.isEmpty()) {
-            notifyListeners(regs, path, candNode);
+            addToListenerChanges(regs, path, candNode, listenerChanges);
         }
 
         for (DataTreeCandidateNode candChild : candNode.getChildNodes()) {
@@ -142,14 +156,24 @@ public abstract class AbstractDOMStoreTreeChangePublisher
                 final RegistrationTreeNode<AbstractDOMDataTreeChangeListenerRegistration<?>> regChild =
                         regNode.getExactChild(candChild.getIdentifier());
                 if (regChild != null) {
-                    notifyNode(path.node(candChild.getIdentifier()), regChild, candChild);
+                    notifyNode(path.node(candChild.getIdentifier()), regChild, candChild, listenerChanges);
                 }
 
                 for (RegistrationTreeNode<AbstractDOMDataTreeChangeListenerRegistration<?>> rc :
                     regNode.getInexactChildren(candChild.getIdentifier())) {
-                    notifyNode(path.node(candChild.getIdentifier()), rc, candChild);
+                    notifyNode(path.node(candChild.getIdentifier()), rc, candChild, listenerChanges);
                 }
             }
         }
     }
+
+    private void addToListenerChanges(final Collection<AbstractDOMDataTreeChangeListenerRegistration<?>> registrations,
+            final YangInstanceIdentifier path, final DataTreeCandidateNode node,
+            final Multimap<AbstractDOMDataTreeChangeListenerRegistration<?>, DataTreeCandidate> listenerChanges) {
+        final DataTreeCandidate dataTreeCandidate = DataTreeCandidates.newDataTreeCandidate(path, node);
+
+        for (AbstractDOMDataTreeChangeListenerRegistration<?> reg : registrations) {
+            listenerChanges.put(reg, dataTreeCandidate);
+        }
+    }
 }

diff --git a/dom/mdsal-dom-spi/src/test/java/org/opendaylight/mdsal/dom/spi/store/AbstractDOMStoreTreeChangePublisherTest.java b/dom/mdsal-dom-spi/src/test/java/org/opendaylight/mdsal/dom/spi/store/AbstractDOMStoreTreeChangePublisherTest.java
index 40c9b378997b68238ecfb422951f709973ea0eb3..f6ecffcc0f07aa7b1e24ce503b48edd59bfbd1fa 100644 (file)
--- a/dom/mdsal-dom-spi/src/test/java/org/opendaylight/mdsal/dom/spi/store/AbstractDOMStoreTreeChangePublisherTest.java
+++ b/dom/mdsal-dom-spi/src/test/java/org/opendaylight/mdsal/dom/spi/store/AbstractDOMStoreTreeChangePublisherTest.java
@@ -70,8 +70,8 @@ public class AbstractDOMStoreTreeChangePublisherTest extends AbstractDOMStoreTre
     }
 
     @Override
-    protected void notifyListeners(@Nonnull Collection<AbstractDOMDataTreeChangeListenerRegistration<?>> registrations,
-                                   @Nonnull YangInstanceIdentifier path, @Nonnull DataTreeCandidateNode node) {
+    protected void notifyListener(AbstractDOMDataTreeChangeListenerRegistration<?> registration,
+            Collection<DataTreeCandidate> changes) {
         notifyInvoked = true;
     }
 
@@ -79,4 +79,4 @@ public class AbstractDOMStoreTreeChangePublisherTest extends AbstractDOMStoreTre
     protected void registrationRemoved(@Nonnull AbstractDOMDataTreeChangeListenerRegistration<?> registration) {
         removeInvoked = true;
     }
-}
\ No newline at end of file
+}