You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by ma...@apache.org on 2011/09/07 20:11:21 UTC
svn commit: r1166290 -
/tomcat/trunk/java/org/apache/catalina/core/DefaultInstanceManager.java
Author: markt
Date: Wed Sep 7 18:11:20 2011
New Revision: 1166290
URL: http://svn.apache.org/viewvc?rev=1166290&view=rev
Log:
Add a simple annotation cache to improve performance for applications that use lots of non-poolable tags.
Modified:
tomcat/trunk/java/org/apache/catalina/core/DefaultInstanceManager.java
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=1166290&r1=1166289&r2=1166290&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/catalina/core/DefaultInstanceManager.java (original)
+++ tomcat/trunk/java/org/apache/catalina/core/DefaultInstanceManager.java Wed Sep 7 18:11:20 2011
@@ -21,6 +21,7 @@ package org.apache.catalina.core;
import java.io.IOException;
import java.io.InputStream;
+import java.lang.reflect.AccessibleObject;
import java.lang.reflect.Field;
import java.lang.reflect.InvocationTargetException;
import java.lang.reflect.Method;
@@ -29,8 +30,11 @@ import java.security.AccessController;
import java.security.PrivilegedAction;
import java.security.PrivilegedActionException;
import java.security.PrivilegedExceptionAction;
+import java.util.ArrayList;
+import java.util.List;
import java.util.Map;
import java.util.Properties;
+import java.util.concurrent.ConcurrentHashMap;
import javax.annotation.PostConstruct;
import javax.annotation.PreDestroy;
@@ -65,6 +69,8 @@ public class DefaultInstanceManager impl
private Properties restrictedFilters = new Properties();
private Properties restrictedListeners = new Properties();
private Properties restrictedServlets = new Properties();
+ private Map<Class<?>,List<AnnotationCacheEntry>> annotationCache =
+ new ConcurrentHashMap<Class<?>, List<AnnotationCacheEntry>>();
public DefaultInstanceManager(Context context, Map<String, Map<String, String>> injectionMap, org.apache.catalina.Context catalinaContext, ClassLoader containerClassLoader) {
classLoader = catalinaContext.getLoader().getClassLoader();
@@ -276,97 +282,138 @@ public class DefaultInstanceManager impl
Class<?> clazz = instance.getClass();
while (clazz != null) {
- // Initialize fields annotations
- Field[] fields = null;
- if (Globals.IS_SECURITY_ENABLED) {
- final Class<?> clazz2 = clazz;
- fields = AccessController.doPrivileged(
- new PrivilegedAction<Field[]>(){
- @Override
- public Field[] run(){
- return clazz2.getDeclaredFields();
+ List<AnnotationCacheEntry> annotations = annotationCache.get(clazz);
+ if (annotations == null) {
+ annotations = new ArrayList<AnnotationCacheEntry>();
+ // Initialize fields annotations
+ Field[] fields = null;
+ if (Globals.IS_SECURITY_ENABLED) {
+ final Class<?> clazz2 = clazz;
+ fields = AccessController.doPrivileged(
+ new PrivilegedAction<Field[]>(){
+ @Override
+ public Field[] run(){
+ return clazz2.getDeclaredFields();
+ }
+ });
+ } else {
+ fields = clazz.getDeclaredFields();
+ }
+ for (Field field : fields) {
+ if (injections != null && injections.containsKey(field.getName())) {
+ lookupFieldResource(context, instance, field,
+ injections.get(field.getName()), clazz);
+ annotations.add(new AnnotationCacheEntry(field,
+ injections.get(field.getName())));
+ } else if (field.isAnnotationPresent(Resource.class)) {
+ Resource annotation = field.getAnnotation(Resource.class);
+ lookupFieldResource(context, instance, field,
+ annotation.name(), clazz);
+ annotations.add(new AnnotationCacheEntry(field,
+ annotation.name()));
+ } else if (field.isAnnotationPresent(EJB.class)) {
+ EJB annotation = field.getAnnotation(EJB.class);
+ lookupFieldResource(context, instance, field,
+ annotation.name(), clazz);
+ annotations.add(new AnnotationCacheEntry(field,
+ annotation.name()));
+ } else if (field.isAnnotationPresent(WebServiceRef.class)) {
+ WebServiceRef annotation =
+ field.getAnnotation(WebServiceRef.class);
+ lookupFieldResource(context, instance, field,
+ annotation.name(), clazz);
+ annotations.add(new AnnotationCacheEntry(field,
+ annotation.name()));
+ } else if (field.isAnnotationPresent(PersistenceContext.class)) {
+ PersistenceContext annotation =
+ field.getAnnotation(PersistenceContext.class);
+ lookupFieldResource(context, instance, field,
+ annotation.name(), clazz);
+ annotations.add(new AnnotationCacheEntry(field,
+ annotation.name()));
+ } else if (field.isAnnotationPresent(PersistenceUnit.class)) {
+ PersistenceUnit annotation =
+ field.getAnnotation(PersistenceUnit.class);
+ lookupFieldResource(context, instance, field,
+ annotation.name(), clazz);
+ annotations.add(new AnnotationCacheEntry(field,
+ annotation.name()));
}
- });
- } else {
- fields = clazz.getDeclaredFields();
- }
- for (Field field : fields) {
- if (injections != null && injections.containsKey(field.getName())) {
- lookupFieldResource(context, instance, field,
- injections.get(field.getName()), clazz);
- } else if (field.isAnnotationPresent(Resource.class)) {
- Resource annotation = field.getAnnotation(Resource.class);
- lookupFieldResource(context, instance, field,
- annotation.name(), clazz);
- } else if (field.isAnnotationPresent(EJB.class)) {
- EJB annotation = field.getAnnotation(EJB.class);
- lookupFieldResource(context, instance, field,
- annotation.name(), clazz);
- } else if (field.isAnnotationPresent(WebServiceRef.class)) {
- WebServiceRef annotation =
- field.getAnnotation(WebServiceRef.class);
- lookupFieldResource(context, instance, field,
- annotation.name(), clazz);
- } else if (field.isAnnotationPresent(PersistenceContext.class)) {
- PersistenceContext annotation =
- field.getAnnotation(PersistenceContext.class);
- lookupFieldResource(context, instance, field,
- annotation.name(), clazz);
- } else if (field.isAnnotationPresent(PersistenceUnit.class)) {
- PersistenceUnit annotation =
- field.getAnnotation(PersistenceUnit.class);
- lookupFieldResource(context, instance, field,
- annotation.name(), clazz);
}
- }
-
- // Initialize methods annotations
- Method[] methods = null;
- if (Globals.IS_SECURITY_ENABLED) {
- final Class<?> clazz2 = clazz;
- methods = AccessController.doPrivileged(
- new PrivilegedAction<Method[]>(){
- @Override
- public Method[] run(){
- return clazz2.getDeclaredMethods();
+
+ // Initialize methods annotations
+ Method[] methods = null;
+ if (Globals.IS_SECURITY_ENABLED) {
+ final Class<?> clazz2 = clazz;
+ methods = AccessController.doPrivileged(
+ new PrivilegedAction<Method[]>(){
+ @Override
+ public Method[] run(){
+ return clazz2.getDeclaredMethods();
+ }
+ });
+ } else {
+ methods = clazz.getDeclaredMethods();
+ }
+ for (Method method : methods) {
+ String methodName = method.getName();
+ if (injections != null && methodName.startsWith("set") && methodName.length() > 3) {
+ String fieldName = Character.toLowerCase(methodName.charAt(3)) + methodName.substring(4);
+ if (injections.containsKey(fieldName)) {
+ lookupMethodResource(context, instance, method,
+ injections.get(fieldName), clazz);
+ annotations.add(new AnnotationCacheEntry(method,
+ injections.get(method.getName())));
+ break;
+ }
}
- });
- } else {
- methods = clazz.getDeclaredMethods();
- }
- for (Method method : methods) {
- String methodName = method.getName();
- if (injections != null && methodName.startsWith("set") && methodName.length() > 3) {
- String fieldName = Character.toLowerCase(methodName.charAt(3)) + methodName.substring(4);
- if (injections.containsKey(fieldName)) {
+ if (method.isAnnotationPresent(Resource.class)) {
+ Resource annotation = method.getAnnotation(Resource.class);
+ lookupMethodResource(context, instance, method,
+ annotation.name(), clazz);
+ annotations.add(new AnnotationCacheEntry(method,
+ annotation.name()));
+ } else if (method.isAnnotationPresent(EJB.class)) {
+ EJB annotation = method.getAnnotation(EJB.class);
lookupMethodResource(context, instance, method,
- injections.get(fieldName), clazz);
- break;
+ annotation.name(), clazz);
+ annotations.add(new AnnotationCacheEntry(method,
+ annotation.name()));
+ } else if (method.isAnnotationPresent(WebServiceRef.class)) {
+ WebServiceRef annotation =
+ method.getAnnotation(WebServiceRef.class);
+ lookupMethodResource(context, instance, method,
+ annotation.name(), clazz);
+ annotations.add(new AnnotationCacheEntry(method,
+ annotation.name()));
+ } else if (method.isAnnotationPresent(PersistenceContext.class)) {
+ PersistenceContext annotation =
+ method.getAnnotation(PersistenceContext.class);
+ lookupMethodResource(context, instance, method,
+ annotation.name(), clazz);
+ annotations.add(new AnnotationCacheEntry(method,
+ annotation.name()));
+ } else if (method.isAnnotationPresent(PersistenceUnit.class)) {
+ PersistenceUnit annotation =
+ method.getAnnotation(PersistenceUnit.class);
+ lookupMethodResource(context, instance, method,
+ annotation.name(), clazz);
+ annotations.add(new AnnotationCacheEntry(method,
+ annotation.name()));
}
}
- if (method.isAnnotationPresent(Resource.class)) {
- Resource annotation = method.getAnnotation(Resource.class);
- lookupMethodResource(context, instance, method,
- annotation.name(), clazz);
- } else if (method.isAnnotationPresent(EJB.class)) {
- EJB annotation = method.getAnnotation(EJB.class);
- lookupMethodResource(context, instance, method,
- annotation.name(), clazz);
- } else if (method.isAnnotationPresent(WebServiceRef.class)) {
- WebServiceRef annotation =
- method.getAnnotation(WebServiceRef.class);
- lookupMethodResource(context, instance, method,
- annotation.name(), clazz);
- } else if (method.isAnnotationPresent(PersistenceContext.class)) {
- PersistenceContext annotation =
- method.getAnnotation(PersistenceContext.class);
- lookupMethodResource(context, instance, method,
- annotation.name(), clazz);
- } else if (method.isAnnotationPresent(PersistenceUnit.class)) {
- PersistenceUnit annotation =
- method.getAnnotation(PersistenceUnit.class);
- lookupMethodResource(context, instance, method,
- annotation.name(), clazz);
+ annotationCache.put(clazz, annotations);
+ } else {
+ for (AnnotationCacheEntry entry : annotations) {
+ if (entry.getAccessibleObject() instanceof Method) {
+ lookupMethodResource(context, instance,
+ (Method) entry.getAccessibleObject(),
+ entry.getName(), clazz);
+ } else {
+ lookupFieldResource(context, instance,
+ (Field) entry.getAccessibleObject(),
+ entry.getName(), clazz);
+ }
}
}
clazz = clazz.getSuperclass();
@@ -529,4 +576,23 @@ public class DefaultInstanceManager impl
}
return jndiName;
}
+
+ private static final class AnnotationCacheEntry {
+ private final AccessibleObject accessibleObject;
+ private final String name;
+
+ public AnnotationCacheEntry(AccessibleObject accessibleObject,
+ String name) {
+ this.accessibleObject = accessibleObject;
+ this.name = name;
+ }
+
+ public AccessibleObject getAccessibleObject() {
+ return accessibleObject;
+ }
+
+ public String getName() {
+ return name;
+ }
+ }
}
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: svn commit: r1166290 - /tomcat/trunk/java/org/apache/catalina/core/DefaultInstanceManager.java
Posted by Mark Thomas <ma...@apache.org>.
On 19/09/2011 12:12, Mark Thomas wrote:
> On 12/09/2011 12:15, Konstantin Kolinko wrote:
>> 2011/9/7 Mark Thomas <ma...@apache.org>:
>>> On 07/09/2011 19:49, Konstantin Kolinko wrote:
>>>> 2011/9/7 <ma...@apache.org>:
>>>>> Author: markt
>>>>> Date: Wed Sep 7 18:11:20 2011
>>>>> New Revision: 1166290
>>>>>
>>>>> URL: http://svn.apache.org/viewvc?rev=1166290&view=rev
>>>>> Log:
>>>>> Add a simple annotation cache to improve performance for applications that use lots of non-poolable tags.
>
> <snip/>
>
>>>> 2. I wonder when the cache is cleared. E.g. if we are in development
>>>> mode or have enabled unloading unused JSPs in JspServlet
>>>> configuration. If I understand correctly it accumulates references to
>>>> Class objects but never releases them.
>>>
>>> When the context is stopped.
>>
>> This issue is still pending.
>> Though I think it concerns only development mode, and benefits of this
>> cache out-weight the resource leak.
>
> As written, it makes the maxLoadedJsps Jasper option useless.
>
> I have given this some thought this morning and I plan on looking at a
> WeakHashMap (plus synchronisation to handle the concurrency) based solution.
Fixed in r1172664.
<snip/>
>> 4. There is code that calls setAccessible() for Method, e.g.:
>>
>> Method postConstruct = (Method) entry.getAccessibleObject();
>> boolean accessibility = postConstruct.isAccessible();
>> postConstruct.setAccessible(true);
>> postConstruct.invoke(instance);
>> postConstruct.setAccessible(accessibility);
>>
>> There will be a race condition if several threads are changing
>> (setting or restoring) the method accessibility flag on the same
>> Method right before invoke() or isAccessible() calls.
>
> I'll try and address this along with the WeakHashMap changes since I'll
> be adding some syncs already.
Fixed in r1172689.
The changes have also been back-ported to 7.0.x.
Mark
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: svn commit: r1166290 - /tomcat/trunk/java/org/apache/catalina/core/DefaultInstanceManager.java
Posted by Mark Thomas <ma...@apache.org>.
On 12/09/2011 12:15, Konstantin Kolinko wrote:
> 2011/9/7 Mark Thomas <ma...@apache.org>:
>> On 07/09/2011 19:49, Konstantin Kolinko wrote:
>>> 2011/9/7 <ma...@apache.org>:
>>>> Author: markt
>>>> Date: Wed Sep 7 18:11:20 2011
>>>> New Revision: 1166290
>>>>
>>>> URL: http://svn.apache.org/viewvc?rev=1166290&view=rev
>>>> Log:
>>>> Add a simple annotation cache to improve performance for applications that use lots of non-poolable tags.
<snip/>
>>> 2. I wonder when the cache is cleared. E.g. if we are in development
>>> mode or have enabled unloading unused JSPs in JspServlet
>>> configuration. If I understand correctly it accumulates references to
>>> Class objects but never releases them.
>>
>> When the context is stopped.
>
> This issue is still pending.
> Though I think it concerns only development mode, and benefits of this
> cache out-weight the resource leak.
As written, it makes the maxLoadedJsps Jasper option useless.
I have given this some thought this morning and I plan on looking at a
WeakHashMap (plus synchronisation to handle the concurrency) based solution.
> 3. Regarding the instances not created through the instance manager:
> http://svn.apache.org/viewvc?rev=1166775&view=rev
>
> The code is now skipping annotations in those classes.
> The old code would scan them for preDestroy annotations.
> I have not looked what behaviour is required by the specs here, but I
> think that the old behaviour was more correct.
I tend to the view that if the InstanceManager didn't create the object,
it isn't responsible for destroying it. That isn't the old behaviour and
I am +0 if you want to change the current code back to the old behaviour.
> 4. There is code that calls setAccessible() for Method, e.g.:
>
> Method postConstruct = (Method) entry.getAccessibleObject();
> boolean accessibility = postConstruct.isAccessible();
> postConstruct.setAccessible(true);
> postConstruct.invoke(instance);
> postConstruct.setAccessible(accessibility);
>
> There will be a race condition if several threads are changing
> (setting or restoring) the method accessibility flag on the same
> Method right before invoke() or isAccessible() calls.
I'll try and address this along with the WeakHashMap changes since I'll
be adding some syncs already.
Mark
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: svn commit: r1166290 - /tomcat/trunk/java/org/apache/catalina/core/DefaultInstanceManager.java
Posted by Konstantin Kolinko <kn...@gmail.com>.
2011/9/7 Mark Thomas <ma...@apache.org>:
> On 07/09/2011 19:49, Konstantin Kolinko wrote:
>> 2011/9/7 <ma...@apache.org>:
>>> Author: markt
>>> Date: Wed Sep 7 18:11:20 2011
>>> New Revision: 1166290
>>>
>>> URL: http://svn.apache.org/viewvc?rev=1166290&view=rev
>>> Log:
>>> Add a simple annotation cache to improve performance for applications that use lots of non-poolable tags.
>>
>> 1. I think most classes do not have annotations. If I understand
>> correctly the code allocates new ArrayList() for each class. Maybe we
>> can optimize for classes that do not have annotations?
>> (E.g. using a shared instance of Collections.emptyList()).
>
> That would be simple to do.
Ok, I like how you did it.
>> 2. I wonder when the cache is cleared. E.g. if we are in development
>> mode or have enabled unloading unused JSPs in JspServlet
>> configuration. If I understand correctly it accumulates references to
>> Class objects but never releases them.
>
> When the context is stopped.
This issue is still pending.
Though I think it concerns only development mode, and benefits of this
cache out-weight the resource leak.
3. Regarding the instances not created through the instance manager:
http://svn.apache.org/viewvc?rev=1166775&view=rev
The code is now skipping annotations in those classes.
The old code would scan them for preDestroy annotations.
I have not looked what behaviour is required by the specs here, but I
think that the old behaviour was more correct.
4. There is code that calls setAccessible() for Method, e.g.:
Method postConstruct = (Method) entry.getAccessibleObject();
boolean accessibility = postConstruct.isAccessible();
postConstruct.setAccessible(true);
postConstruct.invoke(instance);
postConstruct.setAccessible(accessibility);
There will be a race condition if several threads are changing
(setting or restoring) the method accessibility flag on the same
Method right before invoke() or isAccessible() calls.
Best regards,
Konstantin Kolinko
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: svn commit: r1166290 - /tomcat/trunk/java/org/apache/catalina/core/DefaultInstanceManager.java
Posted by Mark Thomas <ma...@apache.org>.
On 07/09/2011 19:49, Konstantin Kolinko wrote:
> 2011/9/7 <ma...@apache.org>:
>> Author: markt
>> Date: Wed Sep 7 18:11:20 2011
>> New Revision: 1166290
>>
>> URL: http://svn.apache.org/viewvc?rev=1166290&view=rev
>> Log:
>> Add a simple annotation cache to improve performance for applications that use lots of non-poolable tags.
>
> 1. I think most classes do not have annotations. If I understand
> correctly the code allocates new ArrayList() for each class. Maybe we
> can optimize for classes that do not have annotations?
> (E.g. using a shared instance of Collections.emptyList()).
That would be simple to do.
> 2. I wonder when the cache is cleared. E.g. if we are in development
> mode or have enabled unloading unused JSPs in JspServlet
> configuration. If I understand correctly it accumulates references to
> Class objects but never releases them.
When the context is stopped.
Mark
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: svn commit: r1166290 - /tomcat/trunk/java/org/apache/catalina/core/DefaultInstanceManager.java
Posted by Konstantin Kolinko <kn...@gmail.com>.
2011/9/7 <ma...@apache.org>:
> Author: markt
> Date: Wed Sep 7 18:11:20 2011
> New Revision: 1166290
>
> URL: http://svn.apache.org/viewvc?rev=1166290&view=rev
> Log:
> Add a simple annotation cache to improve performance for applications that use lots of non-poolable tags.
1. I think most classes do not have annotations. If I understand
correctly the code allocates new ArrayList() for each class. Maybe we
can optimize for classes that do not have annotations?
(E.g. using a shared instance of Collections.emptyList()).
2. I wonder when the cache is cleared. E.g. if we are in development
mode or have enabled unloading unused JSPs in JspServlet
configuration. If I understand correctly it accumulates references to
Class objects but never releases them.
Best regards,
Konstantin Kolinko
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org