From: Robert Varga <rovarga@cisco.com>
Date: Mon, 2 Jun 2014 14:11:20 +0000 (+0200)
Subject: BUG-1120: optimize notification delivery fast path
X-Git-Tag: release/helium~718^2
X-Git-Url: https://git.opendaylight.org/gerrit/gitweb?p=controller.git;a=commitdiff_plain;h=ede2f6f9c3aa3d7b520c3240ef5e10f0090e7d9f

BUG-1120: optimize notification delivery fast path

Now that the interface contract is completely separated, our fast path
is as good as it gets, except it still allocates a few objects in the
lookup path. These are expected to be generally small, but we do now
they are constant while a particular generation is in place.

This means we can stategically place a LoadingCache, which will
eliminate the need to allocate the objects for notifications which are
often used -- thus the fast path really becomes a lookup by class into a
prepared iterable of listeners. For initial implementation we will place
an upper bound of 1000 entries and see where it leads us.

Change-Id: I2cf39f0d4681aa60afb9d517fb7beef99838bbab
Signed-off-by: Robert Varga <rovarga@cisco.com>
---

diff --git a/opendaylight/md-sal/sal-binding-broker/src/main/java/org/opendaylight/controller/sal/binding/impl/ListenerMapGeneration.java b/opendaylight/md-sal/sal-binding-broker/src/main/java/org/opendaylight/controller/sal/binding/impl/ListenerMapGeneration.java
index 5ded885f8e..4d893aa7be 100644
--- a/opendaylight/md-sal/sal-binding-broker/src/main/java/org/opendaylight/controller/sal/binding/impl/ListenerMapGeneration.java
+++ b/opendaylight/md-sal/sal-binding-broker/src/main/java/org/opendaylight/controller/sal/binding/impl/ListenerMapGeneration.java
@@ -15,7 +15,11 @@ import java.util.Set;
 import org.opendaylight.yangtools.yang.binding.Notification;
 
 import com.google.common.base.Predicate;
+import com.google.common.cache.CacheBuilder;
+import com.google.common.cache.CacheLoader;
+import com.google.common.cache.LoadingCache;
 import com.google.common.collect.ImmutableMultimap;
+import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.Iterables;
 import com.google.common.collect.Multimap;
 
@@ -23,14 +27,44 @@ import com.google.common.collect.Multimap;
  * An immutable view of the current generation of listeners.
  */
 final class ListenerMapGeneration {
-    private final Multimap<Class<? extends Notification>, NotificationListenerRegistration<?>> listeners;
+    private static final int CACHE_MAX_ENTRIES = 1000;
+
+    /**
+     * Constant map of notification type to subscribed listeners.
+     */
+    private final Multimap<Class<? extends Notification>, NotificationListenerRegistration<?>> typeToListeners;
+
+    /**
+     * Dynamic cache of notification implementation to matching listeners. This cache loads entries based on
+     * the contents of the {@link #typeToListeners} map.
+     */
+    private final LoadingCache<Class<?>, Iterable<NotificationListenerRegistration<?>>> implementationToListeners =
+            CacheBuilder.newBuilder()
+            .weakKeys()
+            .maximumSize(CACHE_MAX_ENTRIES)
+            .build(new CacheLoader<Class<?>, Iterable<NotificationListenerRegistration<?>>>() {
+                @Override
+                public Iterable<NotificationListenerRegistration<?>> load(final Class<?> key) {
+                    final Set<NotificationListenerRegistration<?>> regs = new HashSet<>();
+
+                    for (final Class<?> type : getNotificationTypes(key)) {
+                        @SuppressWarnings("unchecked")
+                        final Collection<NotificationListenerRegistration<?>> l = typeToListeners.get((Class<? extends Notification>) type);
+                        if (l != null) {
+                            regs.addAll(l);
+                        }
+                    }
+
+                    return ImmutableSet.copyOf(regs);
+                }
+            });
 
     ListenerMapGeneration() {
-        listeners = ImmutableMultimap.of();
+        typeToListeners = ImmutableMultimap.of();
     }
 
     ListenerMapGeneration(final Multimap<Class<? extends Notification>, NotificationListenerRegistration<?>> listeners) {
-        this.listeners = ImmutableMultimap.copyOf(listeners);
+        this.typeToListeners = ImmutableMultimap.copyOf(listeners);
     }
 
     /**
@@ -39,44 +73,34 @@ final class ListenerMapGeneration {
      * @return Current type-to-listener map.
      */
     Multimap<Class<? extends Notification>, NotificationListenerRegistration<?>> getListeners() {
-        return listeners;
-    }
-
-    private static Iterable<Class<?>> getNotificationTypes(final Notification notification) {
-        final Class<?>[] ifaces = notification.getClass().getInterfaces();
-        return Iterables.filter(Arrays.asList(ifaces), new Predicate<Class<?>>() {
-            @Override
-            public boolean apply(final Class<?> input) {
-                if (Notification.class.equals(input)) {
-                    return false;
-                }
-                return Notification.class.isAssignableFrom(input);
-            }
-        });
+        return typeToListeners;
     }
 
     /**
      * Look up the listeners which need to see this notification delivered.
      *
      * @param notification Notification object
-     * @return Iterable of listeners, may be null
-     *
-     * FIXME: improve such that it always returns non-null.
+     * @return Iterable of listeners, guaranteed to be nonnull.
      */
     public Iterable<NotificationListenerRegistration<?>> listenersFor(final Notification notification) {
-        final Set<NotificationListenerRegistration<?>> ret = new HashSet<>();
-
-        for (final Class<?> type : getNotificationTypes(notification)) {
-            final Collection<NotificationListenerRegistration<?>> l = listeners.get((Class<? extends Notification>) type);
-            if (l != null) {
-                ret.addAll(l);
-            }
-        }
-
-        return ret;
+        // Safe to use, as our loader does not throw checked exceptions
+        return implementationToListeners.getUnchecked(notification.getClass());
     }
 
     public Iterable<Class<? extends Notification>> getKnownTypes() {
-        return listeners.keySet();
+        return typeToListeners.keySet();
+    }
+
+    private static Iterable<Class<?>> getNotificationTypes(final Class<?> cls) {
+        final Class<?>[] ifaces = cls.getInterfaces();
+        return Iterables.filter(Arrays.asList(ifaces), new Predicate<Class<?>>() {
+            @Override
+            public boolean apply(final Class<?> input) {
+                if (Notification.class.equals(input)) {
+                    return false;
+                }
+                return Notification.class.isAssignableFrom(input);
+            }
+        });
     }
 }
\ No newline at end of file