Author: jukka Date: Mon Apr 28 15:43:18 2014 New Revision: 1590684 URL: http://svn.apache.org/r1590684 Log: OAK-1771: Avoid lock contention in Tracker.getServices()
Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/osgi/OsgiWhiteboard.java jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/spi/whiteboard/AbstractServiceTracker.java jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/spi/whiteboard/Tracker.java Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/osgi/OsgiWhiteboard.java URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/osgi/OsgiWhiteboard.java?rev=1590684&r1=1590683&r2=1590684&view=diff ============================================================================== --- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/osgi/OsgiWhiteboard.java (original) +++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/osgi/OsgiWhiteboard.java Mon Apr 28 15:43:18 2014 @@ -18,13 +18,19 @@ package org.apache.jackrabbit.oak.osgi; import static com.google.common.base.Preconditions.checkArgument; import static com.google.common.base.Preconditions.checkNotNull; -import static java.util.Arrays.asList; +import static com.google.common.collect.Lists.newArrayList; +import static com.google.common.collect.Maps.newHashMap; +import static com.google.common.collect.Maps.newTreeMap; +import static java.util.Collections.emptyList; +import static java.util.Collections.singletonList; import java.util.Collections; import java.util.Dictionary; import java.util.Hashtable; import java.util.List; import java.util.Map; +import java.util.SortedMap; +import java.util.concurrent.atomic.AtomicReference; import javax.annotation.Nonnull; @@ -32,8 +38,10 @@ import org.apache.jackrabbit.oak.spi.whi import org.apache.jackrabbit.oak.spi.whiteboard.Tracker; import org.apache.jackrabbit.oak.spi.whiteboard.Whiteboard; import org.osgi.framework.BundleContext; +import org.osgi.framework.ServiceReference; import org.osgi.framework.ServiceRegistration; import org.osgi.util.tracker.ServiceTracker; +import org.osgi.util.tracker.ServiceTrackerCustomizer; /** * OSGi-based whiteboard implementation. @@ -69,17 +77,63 @@ public class OsgiWhiteboard implements W }; } + /** + * Returns a tracker for services of the given type. The returned tracker + * is optimized for frequent {@link Tracker#getServices()} calls through + * the use of a pre-compiled list of services that's atomically updated + * whenever services are added, modified or removed. + */ @Override - public <T> Tracker<T> track(Class<T> type) { + public <T> Tracker<T> track(final Class<T> type) { checkNotNull(type); + final AtomicReference<List<T>> list = + new AtomicReference<List<T>>(Collections.<T>emptyList()); + final ServiceTrackerCustomizer customizer = + new ServiceTrackerCustomizer() { + private final Map<ServiceReference, T> services = + newHashMap(); + @Override @SuppressWarnings("unchecked") + public synchronized Object addingService( + ServiceReference reference) { + Object service = context.getService(reference); + if (type.isInstance(service)) { + services.put(reference, (T) service); + list.set(getServiceList(services)); + return service; + } else { + context.ungetService(reference); + return null; + } + } + @Override @SuppressWarnings("unchecked") + public synchronized void modifiedService( + ServiceReference reference, Object service) { + // TODO: Figure out if the old reference instance + // would automatically reflect the updated properties. + // For now we play it safe by replacing the old key + // with the new reference instance passed as argument. + services.remove(reference); + services.put(reference, (T) service); + list.set(getServiceList(services)); + } + @Override + public synchronized void removedService( + ServiceReference reference, Object service) { + services.remove(reference); + list.set(getServiceList(services)); + // TODO: Note that the service might still be in use + // by some client that called getServices() before + // this method was invoked. + context.ungetService(reference); + } + }; final ServiceTracker tracker = - new ServiceTracker(context, type.getName(), null); + new ServiceTracker(context, type.getName(), customizer); tracker.open(); return new Tracker<T>() { - @Override @SuppressWarnings("unchecked") + @Override public List<T> getServices() { - Object[] services = tracker.getServices(); - return (List<T>) (services != null ? asList(services) : Collections.emptyList()); + return list.get(); } @Override public void stop() { @@ -88,4 +142,26 @@ public class OsgiWhiteboard implements W }; } + /** + * Utility method that sorts the service objects in the given map + * according to their service rankings and returns the resulting list. + * + * @param services currently available services + * @return ordered list of the services + */ + private static <T> List<T> getServiceList( + Map<ServiceReference, T> services) { + switch (services.size()) { + case 0: + return emptyList(); + case 1: + return singletonList( + services.values().iterator().next()); + default: + SortedMap<ServiceReference, T> sorted = newTreeMap(); + sorted.putAll(services); + return newArrayList(sorted.values()); + } + } + } Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/spi/whiteboard/AbstractServiceTracker.java URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/spi/whiteboard/AbstractServiceTracker.java?rev=1590684&r1=1590683&r2=1590684&view=diff ============================================================================== --- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/spi/whiteboard/AbstractServiceTracker.java (original) +++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/spi/whiteboard/AbstractServiceTracker.java Mon Apr 28 15:43:18 2014 @@ -20,6 +20,7 @@ package org.apache.jackrabbit.oak.spi.wh import static com.google.common.base.Preconditions.checkNotNull; import static com.google.common.base.Preconditions.checkState; +import static java.util.Collections.emptyList; import java.util.List; @@ -32,32 +33,58 @@ import javax.annotation.Nonnull; */ public abstract class AbstractServiceTracker<T> { + /** + * Sentinel object used as the {@link #tracker} value of an already + * stopped instance. + */ + private final Tracker<T> stopped = new Tracker<T>() { + @Override + public List<T> getServices() { + return emptyList(); + } + @Override + public void stop() { + // do nothing + } + }; + + /** + * The type of services tracked by this instance. + */ private final Class<T> type; - private Tracker<T> tracker = null; + /** + * The underlying {@link Tracker}, or the {@link #stopped} sentinel + * sentinel object when this instance is not active. This variable + * is {@code volatile} so that the {@link #getServices()} method will + * always see the latest state without having to be synchronized. + */ + private volatile Tracker<T> tracker = stopped; - public AbstractServiceTracker(@Nonnull Class<T> type) { + protected AbstractServiceTracker(@Nonnull Class<T> type) { this.type = checkNotNull(type); } public synchronized void start(Whiteboard whiteboard) { - checkState(tracker == null); + checkState(tracker == stopped); tracker = whiteboard.track(type); } public synchronized void stop() { - checkState(tracker != null); - tracker.stop(); - tracker = null; + checkState(tracker != stopped); + Tracker<T> t = tracker; + tracker = stopped; + t.stop(); } /** - * Returns all services of type {@code T} currently available. + * Returns all services of type {@code T} that are currently available. + * This method is intentionally not synchronized to prevent lock + * contention when accessed frequently in highly concurrent code. * - * @return services currently available. + * @return currently available services */ - protected synchronized List<T> getServices() { - checkState(tracker != null); + protected List<T> getServices() { return tracker.getServices(); } Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/spi/whiteboard/Tracker.java URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/spi/whiteboard/Tracker.java?rev=1590684&r1=1590683&r2=1590684&view=diff ============================================================================== --- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/spi/whiteboard/Tracker.java (original) +++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/spi/whiteboard/Tracker.java Mon Apr 28 15:43:18 2014 @@ -18,10 +18,6 @@ package org.apache.jackrabbit.oak.spi.wh import java.util.List; -import javax.annotation.CheckForNull; - -import com.google.common.base.Predicate; - /** * Tracker for whiteboard services. */