From: Tom Pantelis Date: Tue, 6 Oct 2015 21:44:10 +0000 (-0400) Subject: Lazily create schema context in GlobalBundleScanning* X-Git-Tag: release/beryllium~110 X-Git-Url: https://git.opendaylight.org/gerrit/gitweb?p=controller.git;a=commitdiff_plain;h=b5d0cd043394f9ef5865324c340b2d1cd78cb639;ds=sidebyside Lazily create schema context in GlobalBundleScanning* On GlobalBundleScanningSchemaServiceImpl startup, it calls tryToUpdateSchemaContext when bundleTracker.open() completes. This parses and creates a schema context and notifies listeners of the updated schema context. However we don't have to call tryToUpdateSchemaContext in start() unless there actually are listeners registered as the purpose of tryToUpdateSchemaContext is to update listeners. So made this change. It's not likely there would be any listeners at that point anway. I also changed the synchronization so as not to synchronize on 'this' or the class instance as both are unsafe (users could sync on the instance causing unwanted contention, either innocently or maliciously). Change-Id: I435358b0851671b7fbfdc9784577c91ff20556df Signed-off-by: Tom Pantelis --- diff --git a/opendaylight/md-sal/sal-dom-broker/src/main/java/org/opendaylight/controller/sal/dom/broker/GlobalBundleScanningSchemaServiceImpl.java b/opendaylight/md-sal/sal-dom-broker/src/main/java/org/opendaylight/controller/sal/dom/broker/GlobalBundleScanningSchemaServiceImpl.java index ac220a830d..cf5e5fafce 100644 --- a/opendaylight/md-sal/sal-dom-broker/src/main/java/org/opendaylight/controller/sal/dom/broker/GlobalBundleScanningSchemaServiceImpl.java +++ b/opendaylight/md-sal/sal-dom-broker/src/main/java/org/opendaylight/controller/sal/dom/broker/GlobalBundleScanningSchemaServiceImpl.java @@ -18,6 +18,8 @@ import java.util.ArrayList; import java.util.Collections; import java.util.Enumeration; import java.util.List; +import java.util.concurrent.atomic.AtomicReference; +import javax.annotation.concurrent.GuardedBy; import org.opendaylight.controller.sal.core.api.model.SchemaService; import org.opendaylight.yangtools.concepts.ListenerRegistration; import org.opendaylight.yangtools.concepts.Registration; @@ -41,6 +43,9 @@ import org.slf4j.LoggerFactory; public class GlobalBundleScanningSchemaServiceImpl implements SchemaContextProvider, SchemaService, ServiceTrackerCustomizer, AutoCloseable { private static final Logger LOG = LoggerFactory.getLogger(GlobalBundleScanningSchemaServiceImpl.class); + private static AtomicReference globalInstance = new AtomicReference<>(); + + @GuardedBy(value = "lock") private final ListenerRegistry listeners = new ListenerRegistry<>(); private final URLSchemaContextResolver contextResolver = URLSchemaContextResolver.create("global-bundle"); private final BundleScanner scanner = new BundleScanner(); @@ -50,30 +55,30 @@ public class GlobalBundleScanningSchemaServiceImpl implements SchemaContextProvi private BundleTracker> bundleTracker; private boolean starting = true; private volatile boolean stopping; - private static GlobalBundleScanningSchemaServiceImpl instance; + private final Object lock = new Object(); private GlobalBundleScanningSchemaServiceImpl(final BundleContext context) { this.context = Preconditions.checkNotNull(context); } - public synchronized static GlobalBundleScanningSchemaServiceImpl createInstance(final BundleContext ctx) { - Preconditions.checkState(instance == null); - instance = new GlobalBundleScanningSchemaServiceImpl(ctx); + public static GlobalBundleScanningSchemaServiceImpl createInstance(final BundleContext ctx) { + GlobalBundleScanningSchemaServiceImpl instance = new GlobalBundleScanningSchemaServiceImpl(ctx); + Preconditions.checkState(globalInstance.compareAndSet(null, instance)); instance.start(); return instance; } - public synchronized static GlobalBundleScanningSchemaServiceImpl getInstance() { + public static GlobalBundleScanningSchemaServiceImpl getInstance() { + GlobalBundleScanningSchemaServiceImpl instance = globalInstance.get(); Preconditions.checkState(instance != null, "Global Instance was not instantiated"); return instance; } @VisibleForTesting - public static synchronized void destroyInstance() { - try { + public static void destroyInstance() { + GlobalBundleScanningSchemaServiceImpl instance = globalInstance.getAndSet(null); + if(instance != null) { instance.close(); - } finally { - instance = null; } } @@ -88,13 +93,20 @@ public class GlobalBundleScanningSchemaServiceImpl implements SchemaContextProvi listenerTracker = new ServiceTracker<>(context, SchemaContextListener.class, GlobalBundleScanningSchemaServiceImpl.this); bundleTracker = new BundleTracker<>(context, Bundle.RESOLVED | Bundle.STARTING | Bundle.STOPPING | Bundle.ACTIVE, scanner); - bundleTracker.open(); - LOG.debug("BundleTracker.open() complete"); + synchronized(lock) { + bundleTracker.open(); + + LOG.debug("BundleTracker.open() complete"); + + boolean hasExistingListeners = Iterables.size(listeners.getListeners()) > 0; + if(hasExistingListeners) { + tryToUpdateSchemaContext(); + } + } listenerTracker.open(); starting = false; - tryToUpdateSchemaContext(); LOG.debug("start() complete"); } @@ -125,12 +137,14 @@ public class GlobalBundleScanningSchemaServiceImpl implements SchemaContextProvi } @Override - public synchronized ListenerRegistration registerSchemaContextListener(final SchemaContextListener listener) { - Optional potentialCtx = contextResolver.getSchemaContext(); - if(potentialCtx.isPresent()) { - listener.onGlobalContextUpdated(potentialCtx.get()); + public ListenerRegistration registerSchemaContextListener(final SchemaContextListener listener) { + synchronized(lock) { + Optional potentialCtx = contextResolver.getSchemaContext(); + if(potentialCtx.isPresent()) { + listener.onGlobalContextUpdated(potentialCtx.get()); + } + return listeners.register(listener); } - return listeners.register(listener); } @Override @@ -148,7 +162,8 @@ public class GlobalBundleScanningSchemaServiceImpl implements SchemaContextProvi } } - private synchronized void updateContext(final SchemaContext snapshot) { + @GuardedBy(value = "lock") + private void notifyListeners(final SchemaContext snapshot) { Object[] services = listenerTracker.getServices(); for (ListenerRegistration listener : listeners) { try { @@ -213,7 +228,7 @@ public class GlobalBundleScanningSchemaServiceImpl implements SchemaContextProvi */ @Override - public synchronized void removedBundle(final Bundle bundle, final BundleEvent event, final Iterable urls) { + public void removedBundle(final Bundle bundle, final BundleEvent event, final Iterable urls) { for (Registration url : urls) { try { url.close(); @@ -234,7 +249,7 @@ public class GlobalBundleScanningSchemaServiceImpl implements SchemaContextProvi } @Override - public synchronized SchemaContextListener addingService(final ServiceReference reference) { + public SchemaContextListener addingService(final ServiceReference reference) { SchemaContextListener listener = context.getService(reference); SchemaContext _ctxContext = getGlobalContext(); @@ -244,17 +259,20 @@ public class GlobalBundleScanningSchemaServiceImpl implements SchemaContextProvi return listener; } - public synchronized void tryToUpdateSchemaContext() { + public void tryToUpdateSchemaContext() { if (starting || stopping) { return; } - Optional schema = contextResolver.getSchemaContext(); - if(schema.isPresent()) { - if(LOG.isDebugEnabled()) { - LOG.debug("Got new SchemaContext: # of modules {}", schema.get().getAllModuleIdentifiers().size()); - } - updateContext(schema.get()); + synchronized(lock) { + Optional schema = contextResolver.getSchemaContext(); + if(schema.isPresent()) { + if(LOG.isDebugEnabled()) { + LOG.debug("Got new SchemaContext: # of modules {}", schema.get().getAllModuleIdentifiers().size()); + } + + notifyListeners(schema.get()); + } } }