Author: markt Date: Wed Jan 25 14:26:53 2017 New Revision: 1780189 URL: http://svn.apache.org/viewvc?rev=1780189&view=rev Log: Reduce the contention in the default InstanceManager implementation when multiple threads are managing objects and need to reference the annotation cache. Prior to this patch, TestDefaultInstanceManager#testConcurrency() showed a roughly linear increase in execution time as the thread count increased. With this patch applied, the test shows broadly no change in execution time as the thread count is increased from 1 to 8 on a machine with 8 cores available.
Modified: tomcat/trunk/java/org/apache/catalina/core/DefaultInstanceManager.java tomcat/trunk/java/org/apache/catalina/core/LocalStrings.properties tomcat/trunk/java/org/apache/catalina/core/StandardContext.java tomcat/trunk/java/org/apache/tomcat/InstanceManager.java tomcat/trunk/webapps/docs/changelog.xml Modified: tomcat/trunk/java/org/apache/catalina/core/DefaultInstanceManager.java URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/core/DefaultInstanceManager.java?rev=1780189&r1=1780188&r2=1780189&view=diff ============================================================================== --- tomcat/trunk/java/org/apache/catalina/core/DefaultInstanceManager.java (original) +++ tomcat/trunk/java/org/apache/catalina/core/DefaultInstanceManager.java Wed Jan 25 14:26:53 2017 @@ -34,7 +34,6 @@ import java.util.List; import java.util.Map; import java.util.Properties; import java.util.Set; -import java.util.WeakHashMap; import javax.annotation.PostConstruct; import javax.annotation.PreDestroy; @@ -53,6 +52,7 @@ import org.apache.catalina.util.Introspe import org.apache.juli.logging.Log; import org.apache.tomcat.InstanceManager; import org.apache.tomcat.util.ExceptionUtils; +import org.apache.tomcat.util.collections.ManagedConcurrentWeakHashMap; import org.apache.tomcat.util.res.StringManager; public class DefaultInstanceManager implements InstanceManager { @@ -74,8 +74,8 @@ public class DefaultInstanceManager impl protected final boolean privileged; protected final boolean ignoreAnnotations; private final Set<String> restrictedClasses; - private final Map<Class<?>, AnnotationCacheEntry[]> annotationCache = - new WeakHashMap<>(); + private final ManagedConcurrentWeakHashMap<Class<?>, AnnotationCacheEntry[]> annotationCache = + new ManagedConcurrentWeakHashMap<>(); private final Map<String, String> postConstructMethods; private final Map<String, String> preDestroyMethods; @@ -190,10 +190,7 @@ public class DefaultInstanceManager impl // At the end the postconstruct annotated // method is invoked - AnnotationCacheEntry[] annotations; - synchronized (annotationCache) { - annotations = annotationCache.get(clazz); - } + AnnotationCacheEntry[] annotations = annotationCache.get(clazz); for (AnnotationCacheEntry entry : annotations) { if (entry.getType() == AnnotationCacheEntryType.POST_CONSTRUCT) { Method postConstruct = getMethod(clazz, entry); @@ -227,10 +224,7 @@ public class DefaultInstanceManager impl // At the end the postconstruct annotated // method is invoked - AnnotationCacheEntry[] annotations = null; - synchronized (annotationCache) { - annotations = annotationCache.get(clazz); - } + AnnotationCacheEntry[] annotations = annotationCache.get(clazz); if (annotations == null) { // instance not created through the instance manager return; @@ -249,6 +243,12 @@ public class DefaultInstanceManager impl } + @Override + public void backgroundProcess() { + annotationCache.maintain(); + } + + /** * Make sure that the annotations cache has been populated for the provided * class. @@ -268,10 +268,7 @@ public class DefaultInstanceManager impl List<AnnotationCacheEntry> annotations = null; while (clazz != null) { - AnnotationCacheEntry[] annotationsArray = null; - synchronized (annotationCache) { - annotationsArray = annotationCache.get(clazz); - } + AnnotationCacheEntry[] annotationsArray = annotationCache.get(clazz); if (annotationsArray == null) { if (annotations == null) { annotations = new ArrayList<>(); @@ -446,10 +443,7 @@ public class DefaultInstanceManager impl Class<?> clazz = instance.getClass(); while (clazz != null) { - AnnotationCacheEntry[] annotations; - synchronized (annotationCache) { - annotations = annotationCache.get(clazz); - } + AnnotationCacheEntry[] annotations = annotationCache.get(clazz); for (AnnotationCacheEntry entry : annotations) { if (entry.getType() == AnnotationCacheEntryType.SETTER) { lookupMethodResource(context, instance, @@ -472,9 +466,7 @@ public class DefaultInstanceManager impl * @return the cache size */ protected int getAnnotationCacheSize() { - synchronized (annotationCache) { - return annotationCache.size(); - } + return annotationCache.size(); } Modified: tomcat/trunk/java/org/apache/catalina/core/LocalStrings.properties URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/core/LocalStrings.properties?rev=1780189&r1=1780188&r2=1780189&view=diff ============================================================================== --- tomcat/trunk/java/org/apache/catalina/core/LocalStrings.properties (original) +++ tomcat/trunk/java/org/apache/catalina/core/LocalStrings.properties Wed Jan 25 14:26:53 2017 @@ -112,6 +112,7 @@ pushBuilder.noPath=It is illegal to call standardContext.invalidWrapperClass={0} is not a subclass of StandardWrapper standardContext.applicationListener=Error configuring application listener of class {0} standardContext.applicationSkipped=Skipped installing application listeners due to previous error(s) +standardContext.backgroundProcess.instanceManager=Exception processing instance manager {0} background process standardContext.backgroundProcess.loader=Exception processing loader {0} background process standardContext.backgroundProcess.manager=Exception processing manager {0} background process standardContext.backgroundProcess.resources=Exception processing resources {0} background process Modified: tomcat/trunk/java/org/apache/catalina/core/StandardContext.java URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/core/StandardContext.java?rev=1780189&r1=1780188&r2=1780189&view=diff ============================================================================== --- tomcat/trunk/java/org/apache/catalina/core/StandardContext.java (original) +++ tomcat/trunk/java/org/apache/catalina/core/StandardContext.java Wed Jan 25 14:26:53 2017 @@ -5558,6 +5558,16 @@ public class StandardContext extends Con resources), e); } } + InstanceManager instanceManager = getInstanceManager(); + if (instanceManager != null) { + try { + instanceManager.backgroundProcess(); + } catch (Exception e) { + log.warn(sm.getString( + "standardContext.backgroundProcess.instanceManager", + resources), e); + } + } super.backgroundProcess(); } Modified: tomcat/trunk/java/org/apache/tomcat/InstanceManager.java URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/tomcat/InstanceManager.java?rev=1780189&r1=1780188&r2=1780189&view=diff ============================================================================== --- tomcat/trunk/java/org/apache/tomcat/InstanceManager.java (original) +++ tomcat/trunk/java/org/apache/tomcat/InstanceManager.java Wed Jan 25 14:26:53 2017 @@ -39,4 +39,13 @@ public interface InstanceManager { public void destroyInstance(Object o) throws IllegalAccessException, InvocationTargetException; + + /** + * Called by the component using the InstanceManager periodically to perform + * any regular maintenance that might be required. By default, this method + * is a NO-OP. + */ + public default void backgroundProcess() { + // NO-OP by default + } } Modified: tomcat/trunk/webapps/docs/changelog.xml URL: http://svn.apache.org/viewvc/tomcat/trunk/webapps/docs/changelog.xml?rev=1780189&r1=1780188&r2=1780189&view=diff ============================================================================== --- tomcat/trunk/webapps/docs/changelog.xml (original) +++ tomcat/trunk/webapps/docs/changelog.xml Wed Jan 25 14:26:53 2017 @@ -61,6 +61,11 @@ <code>getDefaultWebXmlListener()</code>. Patch provided by Aaron Anderson. (markt) </scode> + <fix> + Reduce the contention in the default <code>InstanceManager</code> + implementation when multiple threads are managing objects and need to + reference the annotation cache. (markt) + </fix> </changelog> </subsection> <subsection name="Coyote"> --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org