X-Git-Url: https://git.opendaylight.org/gerrit/gitweb?a=blobdiff_plain;f=opendaylight%2Fmd-sal%2Fsal-inmemory-datastore%2Fsrc%2Fmain%2Fjava%2Forg%2Fopendaylight%2Fcontroller%2Fmd%2Fsal%2Fdom%2Fstore%2Fimpl%2FResolveDataChangeState.java;h=3db4115af67908bed3c4edfcfbb2d91cdae3baae;hb=61bc22edce038c773f19e716e1a99147876232ed;hp=828e4e30a18e12538ae1b36a76c13f1044a84755;hpb=15e9db7d90e46f014aeba760263623094bc48f14;p=controller.git

diff --git a/opendaylight/md-sal/sal-inmemory-datastore/src/main/java/org/opendaylight/controller/md/sal/dom/store/impl/ResolveDataChangeState.java b/opendaylight/md-sal/sal-inmemory-datastore/src/main/java/org/opendaylight/controller/md/sal/dom/store/impl/ResolveDataChangeState.java
index 828e4e30a1..3db4115af6 100644
--- a/opendaylight/md-sal/sal-inmemory-datastore/src/main/java/org/opendaylight/controller/md/sal/dom/store/impl/ResolveDataChangeState.java
+++ b/opendaylight/md-sal/sal-inmemory-datastore/src/main/java/org/opendaylight/controller/md/sal/dom/store/impl/ResolveDataChangeState.java
@@ -51,9 +51,9 @@ final class ResolveDataChangeState {
     private final YangInstanceIdentifier nodeId;
     private final Collection<Node> nodes;
 
-    private final Map<DataChangeListenerRegistration<?>, Builder> subBuilders = new HashMap<>();
-    private final Map<DataChangeListenerRegistration<?>, Builder> oneBuilders = new HashMap<>();
-    private final Map<DataChangeListenerRegistration<?>, Builder> baseBuilders = new HashMap<>();
+    private final Map<DataChangeListenerRegistration<?>, Builder> subBuilders;
+    private final Map<DataChangeListenerRegistration<?>, Builder> oneBuilders;
+    private final Map<DataChangeListenerRegistration<?>, Builder> baseBuilders;
 
     private ResolveDataChangeState(final YangInstanceIdentifier nodeId,
             final Iterable<Builder> inheritedSub, final Collection<Builder> inheritedOne,
@@ -66,22 +66,36 @@ final class ResolveDataChangeState {
         /*
          * Collect the nodes which need to be propagated from us to the child.
          */
+        final Map<DataChangeListenerRegistration<?>, Builder> sub = new HashMap<>();
+        final Map<DataChangeListenerRegistration<?>, Builder> one = new HashMap<>();
+        final Map<DataChangeListenerRegistration<?>, Builder> base = new HashMap<>();
         for (Node n : nodes) {
             for (DataChangeListenerRegistration<?> l : n.getListeners()) {
                 final Builder b = DOMImmutableDataChangeEvent.builder(DataChangeScope.BASE);
                 switch (l.getScope()) {
                 case BASE:
-                    baseBuilders.put(l, b);
+                    base.put(l, b);
                     break;
                 case ONE:
-                    oneBuilders.put(l, b);
+                    one.put(l, b);
                     break;
                 case SUBTREE:
-                    subBuilders.put(l, b);
+                    sub.put(l, b);
                     break;
                 }
             }
         }
+
+        baseBuilders = maybeEmpty(base);
+        oneBuilders = maybeEmpty(one);
+        subBuilders = maybeEmpty(sub);
+    }
+
+    private static <K, V> Map<K, V> maybeEmpty(final Map<K, V> map) {
+        if (map.isEmpty()) {
+            return Collections.emptyMap();
+        }
+        return map;
     }
 
     /**
@@ -104,15 +118,34 @@ final class ResolveDataChangeState {
      */
     public ResolveDataChangeState child(final PathArgument childId) {
         /*
-         * We instantiate a concatenation only when needed, otherwise
-         * we reuse the collection. This speeds up Iterables.isEmpty()
-         * in needsProcessing().
+         * We instantiate a concatenation only when needed:
+         *
+         * 1) If our collection is empty, we reuse the parent's. This is typically the case
+         *    for intermediate node, which should be the vast majority.
+         * 2) If the parent's iterable is a Collection and it is empty, reuse our collection.
+         *    This is the case for the first node which defines a subtree listener in a
+         *    particular subtree.
+         * 3) Concatenate the two collections. This happens when we already have some
+         *    subtree listeners and we encounter a node which adds a few more.
+         *
+         * This allows us to lower number of objects allocated and also
+         * speeds up Iterables.isEmpty() in needsProcessing().
+         *
+         * Note that the check for Collection in 2) relies on precisely this logic, which
+         * ensures that we simply cannot see an empty concatenation, but rather start off with
+         * an empty collection, then switch to a non-empty collection and finally switch to
+         * a concatenation. This saves us from instantiating iterators, which a trivial
+         * Iterables.isEmpty() would do as soon as we cross case 3).
          */
         final Iterable<Builder> sb;
-        if (subBuilders.isEmpty()) {
-            sb = inheritedSub;
+        if (!subBuilders.isEmpty()) {
+            if (inheritedSub instanceof Collection && ((Collection<?>) inheritedSub).isEmpty()) {
+                sb = subBuilders.values();
+            } else {
+                sb = Iterables.concat(inheritedSub, subBuilders.values());
+            }
         } else {
-            sb = Iterables.concat(inheritedSub, subBuilders.values());
+            sb = inheritedSub;
         }
 
         return new ResolveDataChangeState(nodeId.node(childId), sb,
@@ -142,12 +175,25 @@ final class ResolveDataChangeState {
         if (!inheritedOne.isEmpty()) {
             return true;
         }
-        // Have SUBTREE listeners
-        if (!Iterables.isEmpty(inheritedSub)) {
-            return true;
+
+        /*
+         * Have SUBTREE listeners
+         *
+         * This is slightly magical replacement for !Iterables.isEmpty(inheritedSub).
+         * It relies on the logic in child(), which gives us the guarantee that when
+         * inheritedSub is not a Collection, it is guaranteed to be non-empty (which
+         * means we need to process). If it is a collection, we still need to check
+         * it for emptiness.
+         *
+         * Unlike Iterables.isEmpty() this code does not instantiate any temporary
+         * objects and is thus more efficient.
+         */
+        if (inheritedSub instanceof Collection) {
+            return !((Collection<?>) inheritedSub).isEmpty();
         }
 
-        return false;
+        // Non-Collection => non-empty => have to process
+        return true;
     }
 
     /**