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

Reply via email to