BUG-650: optimize subtree listener collection 19/10919/1
authorRobert Varga <rovarga@cisco.com>
Mon, 8 Sep 2014 18:29:16 +0000 (20:29 +0200)
committerRobert Varga <rovarga@cisco.com>
Mon, 8 Sep 2014 18:29:16 +0000 (20:29 +0200)
This further optimizes passing of subtree listeners by retaining the
first collection of subtree listeners rather than immediately skipping
to interable concatenation.

Change-Id: I08fd63b092043ef814a47daba67744042d5c29be
Signed-off-by: Robert Varga <rovarga@cisco.com>
opendaylight/md-sal/sal-inmemory-datastore/src/main/java/org/opendaylight/controller/md/sal/dom/store/impl/ResolveDataChangeState.java

index 858087e..e18b632 100644 (file)
@@ -118,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,