BUG-4556: lazy computation of MXBean maps 31/29331/5
authorRobert Varga <rovarga@cisco.com>
Thu, 5 Nov 2015 16:15:30 +0000 (17:15 +0100)
committerGerrit Code Review <gerrit@opendaylight.org>
Tue, 10 Nov 2015 16:10:58 +0000 (16:10 +0000)
Further analysis of our feature:install CPU usage shows that we spend
inordinate amount of time constructing MXBean maps. Make the
construction more asynchronous.

Change-Id: I69450bfe8debb65160c40aed6a75ff3d3bef831d
Signed-off-by: Robert Varga <rovarga@cisco.com>
opendaylight/config/config-manager-facade-xml/src/main/java/org/opendaylight/controller/config/facade/xml/osgi/YangStoreService.java
opendaylight/config/config-manager-facade-xml/src/main/java/org/opendaylight/controller/config/facade/xml/osgi/YangStoreSnapshot.java

index 3149b08..969b129 100644 (file)
@@ -12,15 +12,14 @@ import com.google.common.base.Function;
 import com.google.common.collect.Collections2;
 import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.Sets;
-import java.lang.ref.SoftReference;
+import com.google.common.util.concurrent.ThreadFactoryBuilder;
 import java.util.Collections;
 import java.util.HashSet;
 import java.util.Map;
 import java.util.Set;
 import java.util.concurrent.ExecutorService;
 import java.util.concurrent.Executors;
-import java.util.concurrent.ThreadFactory;
-import java.util.concurrent.atomic.AtomicReference;
+import javax.annotation.concurrent.GuardedBy;
 import org.opendaylight.controller.config.util.capability.Capability;
 import org.opendaylight.controller.config.util.capability.ModuleListener;
 import org.opendaylight.controller.config.util.capability.YangModuleCapability;
@@ -32,129 +31,126 @@ import org.opendaylight.yangtools.yang.model.api.ModuleIdentifier;
 import org.opendaylight.yangtools.yang.model.api.SchemaContextProvider;
 import org.opendaylight.yangtools.yang.model.repo.api.YangTextSchemaSource;
 import org.opendaylight.yangtools.yang.model.repo.spi.SchemaSourceProvider;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
 
 public class YangStoreService implements YangStoreContext {
 
-    private static final Logger LOG = LoggerFactory.getLogger(YangStoreService.class);
+    private final SchemaSourceProvider<YangTextSchemaSource> sourceProvider;
+    private final ExecutorService notificationExecutor = Executors.newSingleThreadExecutor(
+        new ThreadFactoryBuilder().setDaemon(true).setNameFormat("yangstore-capability-notifications").build());
 
     /**
-     * This is a rather interesting locking model. We need to guard against both the
-     * cache expiring from GC and being invalidated by schema context change. The
-     * context can change while we are doing processing, so we do not want to block
-     * it, so no synchronization can happen on the methods.
-     *
-     * So what we are doing is the following:
-     *
-     * We synchronize with GC as usual, using a SoftReference.
-     *
-     * The atomic reference is used to synchronize with {@link #refresh(org.opendaylight.yangtools.sal.binding.generator.util.BindingRuntimeContext)}, e.g. when
-     * refresh happens, it will push a SoftReference(null), e.g. simulate the GC. Now
-     * that may happen while the getter is already busy acting on the old schema context,
-     * so it needs to understand that a refresh has happened and retry. To do that, it
-     * attempts a CAS operation -- if it fails, in knows that the SoftReference has
-     * been replaced and thus it needs to retry.
-     *
-     * Note that {@link #getYangStoreSnapshot()} will still use synchronize() internally
-     * to stop multiple threads doing the same work.
+     * Guarded by explicit lock to allow for properly synchronizing the initial notification and modification
+     * of the listener set.
      */
-    private final AtomicReference<SoftReference<YangStoreSnapshot>> ref =
-            new AtomicReference<>(new SoftReference<YangStoreSnapshot>(null));
-
-    private final AtomicReference<SoftReference<BindingRuntimeContext>> refBindingContext =
-            new AtomicReference<>(new SoftReference<BindingRuntimeContext>(null));
-
-    private final SchemaSourceProvider<YangTextSchemaSource> sourceProvider;
-
-    private final ExecutorService notificationExecutor = Executors.newSingleThreadExecutor(new ThreadFactory() {
-        @Override
-        public Thread newThread(final Runnable r) {
-            return new Thread(r, "yangstore-capability-notifications");
-        }
-    });
+    @GuardedBy("listeners")
+    private final Set<ModuleListener> listeners = new HashSet<ModuleListener>();
 
-    private final Set<ModuleListener> listeners = Collections.synchronizedSet(new HashSet<ModuleListener>());
+    /**
+     * This is the latest snapshot. Some of its state is always initialized, but the MXBean maps potentially cause
+     * recomputation. Accessing those two specific methods needs to re-check whether the snapshot has changed
+     * asynchronously and retry if it didi.
+     */
+    private volatile YangStoreSnapshot snap;
 
     public YangStoreService(final SchemaContextProvider schemaContextProvider,
         final SchemaSourceProvider<YangTextSchemaSource> sourceProvider) {
         this.sourceProvider = sourceProvider;
     }
 
-    synchronized YangStoreContext getYangStoreSnapshot() {
-        SoftReference<YangStoreSnapshot> r = ref.get();
-        YangStoreSnapshot ret = r.get();
-
-        while (ret == null) {
-            // We need to be compute a new value
-            // TODO sourceProvider is not a snapshot
-            ret = new YangStoreSnapshot(refBindingContext.get().get(), sourceProvider);
-
-            if (!ref.compareAndSet(r, new SoftReference<>(ret))) {
-                LOG.debug("Concurrent refresh detected, recomputing snapshot");
-                r = ref.get();
-                ret = null;
-            }
-        }
-
-        return ret;
-    }
-
     public YangStoreContext getCurrentSnapshot() {
-        return getYangStoreSnapshot();
+        return snap;
     }
 
     @Deprecated
     @Override
     public Map<String, Map<String, ModuleMXBeanEntry>> getModuleMXBeanEntryMap() {
-        return getYangStoreSnapshot().getModuleMXBeanEntryMap();
+        Map<String, Map<String, ModuleMXBeanEntry>> ret;
+        YangStoreSnapshot snapshot;
+
+        do {
+            snapshot = snap;
+            ret = snapshot.getModuleMXBeanEntryMap();
+        } while (!snapshot.equals(snap));
+
+        return ret;
     }
 
     @Override
     public Map<QName, Map<String, ModuleMXBeanEntry>> getQNamesToIdentitiesToModuleMXBeanEntries() {
-        return getYangStoreSnapshot().getQNamesToIdentitiesToModuleMXBeanEntries();
+        Map<QName, Map<String, ModuleMXBeanEntry>> ret;
+        YangStoreSnapshot snapshot;
+
+        do {
+            snapshot = snap;
+            ret = snapshot.getQNamesToIdentitiesToModuleMXBeanEntries();
+        } while (!snapshot.equals(snap));
+
+        return ret;
     }
 
     @Override
     public Set<Module> getModules() {
-        return getYangStoreSnapshot().getModules();
+        return snap.getModules();
     }
 
     @Override
     public String getModuleSource(final ModuleIdentifier moduleIdentifier) {
-        return getYangStoreSnapshot().getModuleSource(moduleIdentifier);
+        return snap.getModuleSource(moduleIdentifier);
     }
 
     @Override
     public EnumResolver getEnumResolver() {
-        return getYangStoreSnapshot().getEnumResolver();
+        return snap.getEnumResolver();
     }
 
     public void refresh(final BindingRuntimeContext runtimeContext) {
-        final YangStoreSnapshot previous = ref.get().get();
-        ref.set(new SoftReference<YangStoreSnapshot>(null));
-        refBindingContext.set(new SoftReference<>(runtimeContext));
-        notificationExecutor.submit(new CapabilityChangeNotifier(previous));
+        final YangStoreSnapshot next = new YangStoreSnapshot(runtimeContext, sourceProvider);
+        final YangStoreSnapshot previous = snap;
+        snap = next;
+        notificationExecutor.submit(new Runnable() {
+            @Override
+            public void run() {
+                notifyListeners(previous, next);
+            }
+        });
     }
 
     public AutoCloseable registerModuleListener(final ModuleListener listener) {
-        YangStoreContext context = ref.get().get();
+        final YangStoreContext context = snap;
 
-        if (context == null) {
-            context = getYangStoreSnapshot();
+        synchronized (listeners) {
+            if (context != null) {
+                listener.onCapabilitiesChanged(toCapabilities(context.getModules(), context), Collections.<Capability>emptySet());
+            }
+            this.listeners.add(listener);
         }
 
-        this.listeners.add(listener);
-        listener.onCapabilitiesChanged(toCapabilities(context.getModules(), context), Collections.<Capability>emptySet());
-
         return new AutoCloseable() {
             @Override
             public void close() {
-                YangStoreService.this.listeners.remove(listener);
+                synchronized (listeners) {
+                    listeners.remove(listener);
+                }
             }
         };
     }
 
+    void notifyListeners(final YangStoreSnapshot previous, final YangStoreSnapshot current) {
+        final Set<Module> prevModules = previous.getModules();
+        final Set<Module> currModules = current.getModules();
+        final Set<Module> removed = Sets.difference(prevModules, currModules);
+        final Set<Module> added = Sets.difference(currModules, prevModules);
+
+        final Set<Capability> addedCaps = toCapabilities(added, current);
+        final Set<Capability> removedCaps = toCapabilities(removed, current);
+
+        synchronized (listeners) {
+            for (final ModuleListener listener : listeners) {
+                listener.onCapabilitiesChanged(addedCaps, removedCaps);
+            }
+        }
+    }
+
     private static Set<Capability> toCapabilities(final Set<Module> modules, final YangStoreContext current) {
         return ImmutableSet.copyOf(Collections2.transform(modules, new Function<Module, Capability>() {
             @Override
@@ -163,33 +159,4 @@ public class YangStoreService implements YangStoreContext {
             }
         }));
     }
-
-    private final class CapabilityChangeNotifier implements Runnable {
-
-        private final YangStoreSnapshot previous;
-
-        public CapabilityChangeNotifier(final YangStoreSnapshot previous) {
-            this.previous = previous;
-        }
-
-        @Override
-        public void run() {
-            final YangStoreContext current = getYangStoreSnapshot();
-
-            if (!current.equals(previous)) {
-                final Set<Module> prevModules = previous.getModules();
-                final Set<Module> currModules = current.getModules();
-                final Set<Module> removed = Sets.difference(prevModules, currModules);
-                final Set<Module> added = Sets.difference(currModules, prevModules);
-
-                final Set<Capability> addedCaps = toCapabilities(added, current);
-                final Set<Capability> removedCaps = toCapabilities(removed, current);
-
-                for (final ModuleListener listener : listeners) {
-                    listener.onCapabilitiesChanged(addedCaps, removedCaps);
-                }
-            }
-        }
-
-    }
 }
index 4be2fa3..86dbb44 100644 (file)
@@ -16,6 +16,7 @@ import com.google.common.collect.Sets;
 import com.google.common.io.ByteStreams;
 import com.google.common.util.concurrent.CheckedFuture;
 import java.io.IOException;
+import java.lang.ref.SoftReference;
 import java.util.Collections;
 import java.util.HashMap;
 import java.util.Map;
@@ -40,79 +41,100 @@ import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
 final class YangStoreSnapshot implements YangStoreContext, EnumResolver {
-    private static final Logger LOG = LoggerFactory.getLogger(YangStoreSnapshot.class);
+    private static final class MXBeans {
+        private final Map<String /* Namespace from yang file */,
+                Map<String /* Name of module entry from yang file */, ModuleMXBeanEntry>> moduleMXBeanEntryMap;
+        private final Map<QName, Map<String, ModuleMXBeanEntry>> qNamesToIdentitiesToModuleMXBeanEntries;
+
+        MXBeans(final SchemaContext schemaContext) {
+            LOG.trace("Resolved modules:{}", schemaContext.getModules());
+
+            // JMX generator
+            Map<String, String> namespaceToPackageMapping = Maps.newHashMap();
+            PackageTranslator packageTranslator = new PackageTranslator(namespaceToPackageMapping);
+            Map<QName, ServiceInterfaceEntry> qNamesToSIEs = new HashMap<>();
+            Map<IdentitySchemaNode, ServiceInterfaceEntry> knownSEITracker = new HashMap<>();
+            // create SIE structure qNamesToSIEs
+            for (Module module : schemaContext.getModules()) {
+                String packageName = packageTranslator.getPackageName(module);
+                Map<QName, ServiceInterfaceEntry> namesToSIEntries = ServiceInterfaceEntry
+                        .create(module, packageName, knownSEITracker);
+                for (Entry<QName, ServiceInterfaceEntry> sieEntry : namesToSIEntries.entrySet()) {
+                    // merge value into qNamesToSIEs
+                    if (qNamesToSIEs.containsKey(sieEntry.getKey()) == false) {
+                        qNamesToSIEs.put(sieEntry.getKey(), sieEntry.getValue());
+                    } else {
+                        throw new IllegalStateException("Cannot add two SIE with same qname "
+                                + sieEntry.getValue());
+                    }
+                }
+            }
 
+            Map<String, Map<String, ModuleMXBeanEntry>> moduleMXBeanEntryMap = Maps.newHashMap();
 
-    private final Map<String /* Namespace from yang file */,
-        Map<String /* Name of module entry from yang file */, ModuleMXBeanEntry>> moduleMXBeanEntryMap;
+            Map<QName, Map<String /* identity local name */, ModuleMXBeanEntry>> qNamesToIdentitiesToModuleMXBeanEntries = new HashMap<>();
 
 
-    private final Map<QName, Map<String, ModuleMXBeanEntry>> qNamesToIdentitiesToModuleMXBeanEntries;
+            for (Module module : schemaContext.getModules()) {
+                String packageName = packageTranslator.getPackageName(module);
+                TypeProviderWrapper typeProviderWrapper = new TypeProviderWrapper(
+                        new TypeProviderImpl(schemaContext));
 
-    private final BindingRuntimeContext bindingContextProvider;
-    private final SchemaSourceProvider<YangTextSchemaSource> sourceProvider;
+                QName qName = QName.create(module.getNamespace(), module.getRevision(), module.getName());
 
-    public YangStoreSnapshot(final BindingRuntimeContext bindingContextProvider,
-        final SchemaSourceProvider<YangTextSchemaSource> sourceProvider) {
-        this.bindingContextProvider = bindingContextProvider;
-        this.sourceProvider = sourceProvider;
-
-        final SchemaContext schemaContext = bindingContextProvider.getSchemaContext();
-        LOG.trace("Resolved modules:{}", schemaContext.getModules());
-
-        // JMX generator
-        Map<String, String> namespaceToPackageMapping = Maps.newHashMap();
-        PackageTranslator packageTranslator = new PackageTranslator(namespaceToPackageMapping);
-        Map<QName, ServiceInterfaceEntry> qNamesToSIEs = new HashMap<>();
-        Map<IdentitySchemaNode, ServiceInterfaceEntry> knownSEITracker = new HashMap<>();
-        // create SIE structure qNamesToSIEs
-        for (Module module : schemaContext.getModules()) {
-            String packageName = packageTranslator.getPackageName(module);
-            Map<QName, ServiceInterfaceEntry> namesToSIEntries = ServiceInterfaceEntry
-                    .create(module, packageName, knownSEITracker);
-            for (Entry<QName, ServiceInterfaceEntry> sieEntry : namesToSIEntries.entrySet()) {
-                // merge value into qNamesToSIEs
-                if (qNamesToSIEs.containsKey(sieEntry.getKey()) == false) {
-                    qNamesToSIEs.put(sieEntry.getKey(), sieEntry.getValue());
-                } else {
-                    throw new IllegalStateException("Cannot add two SIE with same qname "
-                            + sieEntry.getValue());
-                }
+                Map<String /* MB identity local name */, ModuleMXBeanEntry> namesToMBEs =
+                        Collections.unmodifiableMap(ModuleMXBeanEntry.create(module, qNamesToSIEs, schemaContext,
+                                typeProviderWrapper, packageName));
+                moduleMXBeanEntryMap.put(module.getNamespace().toString(), namesToMBEs);
+
+                qNamesToIdentitiesToModuleMXBeanEntries.put(qName, namesToMBEs);
             }
+            this.moduleMXBeanEntryMap = Collections.unmodifiableMap(moduleMXBeanEntryMap);
+            this.qNamesToIdentitiesToModuleMXBeanEntries = Collections.unmodifiableMap(qNamesToIdentitiesToModuleMXBeanEntries);
         }
+    }
 
-        Map<String, Map<String, ModuleMXBeanEntry>> moduleMXBeanEntryMap = Maps.newHashMap();
-
-        Map<QName, Map<String /* identity local name */, ModuleMXBeanEntry>> qNamesToIdentitiesToModuleMXBeanEntries = new HashMap<>();
-
+    private static final Logger LOG = LoggerFactory.getLogger(YangStoreSnapshot.class);
+    private final SchemaSourceProvider<YangTextSchemaSource> sourceProvider;
+    private final BindingRuntimeContext bindingContextProvider;
 
-        for (Module module : schemaContext.getModules()) {
-            String packageName = packageTranslator.getPackageName(module);
-            TypeProviderWrapper typeProviderWrapper = new TypeProviderWrapper(
-                    new TypeProviderImpl(schemaContext));
+    /**
+     * We want to lazily compute the context of the MXBean class and have it only softly-attached to this instance,
+     * so it can be garbage collected when the memory gets tight. If the schema context changes as we are computing
+     * things, YangStoreService will detect that and retry, so we do not have to worry about that.
+     */
+    private volatile SoftReference<MXBeans> ref = new SoftReference<>(null);
 
-            QName qName = QName.create(module.getNamespace(), module.getRevision(), module.getName());
+    public YangStoreSnapshot(final BindingRuntimeContext bindingContextProvider,
+        final SchemaSourceProvider<YangTextSchemaSource> sourceProvider) {
+        this.bindingContextProvider = Preconditions.checkNotNull(bindingContextProvider);
+        this.sourceProvider = Preconditions.checkNotNull(sourceProvider);
+    }
 
-            Map<String /* MB identity local name */, ModuleMXBeanEntry> namesToMBEs =
-                    Collections.unmodifiableMap(ModuleMXBeanEntry.create(module, qNamesToSIEs, schemaContext,
-                            typeProviderWrapper, packageName));
-            moduleMXBeanEntryMap.put(module.getNamespace().toString(), namesToMBEs);
+    private MXBeans getMXBeans() {
+        MXBeans mxBean = ref.get();
 
-            qNamesToIdentitiesToModuleMXBeanEntries.put(qName, namesToMBEs);
+        if (mxBean == null) {
+            synchronized (this) {
+                mxBean = ref.get();
+                if (mxBean == null) {
+                    mxBean = new MXBeans(bindingContextProvider.getSchemaContext());
+                    ref = new SoftReference<>(mxBean);
+                }
+            }
         }
-        this.moduleMXBeanEntryMap = Collections.unmodifiableMap(moduleMXBeanEntryMap);
-        this.qNamesToIdentitiesToModuleMXBeanEntries = Collections.unmodifiableMap(qNamesToIdentitiesToModuleMXBeanEntries);
 
+        return mxBean;
     }
 
     @Override
     public Map<String, Map<String, ModuleMXBeanEntry>> getModuleMXBeanEntryMap() {
-        return moduleMXBeanEntryMap;
+        return getMXBeans().moduleMXBeanEntryMap;
     }
 
     @Override
     public Map<QName, Map<String, ModuleMXBeanEntry>> getQNamesToIdentitiesToModuleMXBeanEntries() {
-        return qNamesToIdentitiesToModuleMXBeanEntries;
+        return getMXBeans().qNamesToIdentitiesToModuleMXBeanEntries;
     }
 
     @Override