deltaspike-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Stuart Douglas <sdoug...@redhat.com>
Subject Re: git commit: Removing cache in AnnotationInstanceProvider
Date Wed, 02 May 2012 04:49:26 GMT
AFAIK java.lang.reflect.Proxy actually has its own internal cache of proxy classes, so removing
the cache should not result in the generation of any more proxy class objects. 

Also from the WeakHashMap javadoc:

"
The value objects in a
 * <tt>WeakHashMap</tt> are held by ordinary strong references.  Thus care
 * should be taken to ensure that value objects do not strongly refer to their
 * own keys, either directly or indirectly, since that will prevent the keys
 * from being discarded.
"

I would be worried that the reference from the annotation would prevent the class loader from
being cleaned up, thus causing a memory leak.

Stuart
 

On 02/05/2012, at 2:38 PM, Mark Struberg wrote:

> -1
> 
> The idea was to use that cache for all instances which don't have any member values to
reduce the number of created temporary classes/objects.
> 
> 
> If we don't do that, we might cause mem leaks over a longer time.
> 
> LieGrue,
> strub
> 
> 
> ----- Original Message -----
>> From: "lightguardjp@apache.org" <lightguardjp@apache.org>
>> To: deltaspike-commits@incubator.apache.org
>> Cc: 
>> Sent: Wednesday, May 2, 2012 5:49 AM
>> Subject: git commit: Removing cache in AnnotationInstanceProvider
>> 
>> Updated Branches:
>>   refs/heads/master c82837e3a -> 3a582c96e
>> 
>> 
>> Removing cache in AnnotationInstanceProvider
>> 
>> 
>> Project: http://git-wip-us.apache.org/repos/asf/incubator-deltaspike/repo
>> Commit: 
>> http://git-wip-us.apache.org/repos/asf/incubator-deltaspike/commit/3a582c96
>> Tree: http://git-wip-us.apache.org/repos/asf/incubator-deltaspike/tree/3a582c96
>> Diff: http://git-wip-us.apache.org/repos/asf/incubator-deltaspike/diff/3a582c96
>> 
>> Branch: refs/heads/master
>> Commit: 3a582c96e963939ab7eb566616d9d4a2728896fe
>> Parents: c82837e
>> Author: Jason Porter <lightguardjp@apache.org>
>> Authored: Tue May 1 21:48:43 2012 -0600
>> Committer: Jason Porter <lightguardjp@apache.org>
>> Committed: Tue May 1 21:48:43 2012 -0600
>> 
>> ----------------------------------------------------------------------
>> .../util/metadata/AnnotationInstanceProvider.java  |   58 +--------------
>> .../metadata/AnnotationInstanceProviderTest.java   |   23 +------
>> 2 files changed, 3 insertions(+), 78 deletions(-)
>> ----------------------------------------------------------------------
>> 
>> 
>> http://git-wip-us.apache.org/repos/asf/incubator-deltaspike/blob/3a582c96/deltaspike/core/api/src/main/java/org/apache/deltaspike/core/util/metadata/AnnotationInstanceProvider.java
>> ----------------------------------------------------------------------
>> diff --git 
>> a/deltaspike/core/api/src/main/java/org/apache/deltaspike/core/util/metadata/AnnotationInstanceProvider.java

>> b/deltaspike/core/api/src/main/java/org/apache/deltaspike/core/util/metadata/AnnotationInstanceProvider.java
>> index 398e429..7f285e6 100644
>> --- 
>> a/deltaspike/core/api/src/main/java/org/apache/deltaspike/core/util/metadata/AnnotationInstanceProvider.java
>> +++ 
>> b/deltaspike/core/api/src/main/java/org/apache/deltaspike/core/util/metadata/AnnotationInstanceProvider.java
>> @@ -20,8 +20,6 @@
>> package org.apache.deltaspike.core.util.metadata;
>> 
>> 
>> -import org.apache.deltaspike.core.util.ClassUtils;
>> -
>> import java.io.Serializable;
>> import java.lang.annotation.Annotation;
>> import java.lang.reflect.InvocationHandler;
>> @@ -31,8 +29,6 @@ import java.lang.reflect.Proxy;
>> import java.util.Arrays;
>> import java.util.Collections;
>> import java.util.Map;
>> -import java.util.WeakHashMap;
>> -import java.util.concurrent.ConcurrentHashMap;
>> 
>> /**
>>   * <p>A small helper class to create an Annotation instance of the given 
>> annotation class
>> @@ -55,11 +51,6 @@ public class AnnotationInstanceProvider implements 
>> Annotation, InvocationHandler
>>      private static final Object[] EMPTY_OBJECT_ARRAY = new Object[0];
>>      private static final Class[] EMPTY_CLASS_ARRAY = new Class[0];
>> 
>> -    // NOTE that this cache needs to be a WeakHashMap in order to prevent a 
>> memory leak
>> -    // (the garbage collector should be able to remove the ClassLoader).
>> -    private static volatile Map<ClassLoader, Map<String, 
>> Annotation>> annotationCache
>> -        = new WeakHashMap<ClassLoader, Map<String, Annotation>>();
>> -
>>      private final Class<? extends Annotation> annotationClass;
>>      private final Map<String, ?> memberValues;
>> 
>> @@ -93,16 +84,7 @@ public class AnnotationInstanceProvider implements 
>> Annotation, InvocationHandler
>> 
>>          String key = annotationClass.getName() + "_" + 
>> values.hashCode();
>> 
>> -        Map<String, Annotation> cache = getAnnotationCache();
>> -
>> -        Annotation annotation = cache.get(key);
>> -
>> -        if (annotation == null)
>> -        {
>> -            annotation = initAnnotation(key, annotationClass, cache, values);
>> -        }
>> -
>> -        return (T) annotation;
>> +            return (T) initAnnotation(key, annotationClass, values);
>>      }
>> 
>>      /**
>> @@ -120,47 +102,11 @@ public class AnnotationInstanceProvider implements 
>> Annotation, InvocationHandler
>> 
>>      private static synchronized <T extends Annotation> Annotation 
>> initAnnotation(String key,
>>                                                                                 
>>   Class<T> annotationClass,
>> -                                                                               

>> Map<String, Annotation> cache,
>>                                                                                 
>>   Map<String, ?> values)
>>      {
>> -        Annotation annotation = cache.get(key);
>> -
>> -        // switch into paranoia mode
>> -        if (annotation == null)
>> -        {
>> -            annotation = (Annotation) 
>> Proxy.newProxyInstance(annotationClass.getClassLoader(),
>> +            return (Annotation) 
>> Proxy.newProxyInstance(annotationClass.getClassLoader(),
>>                      new Class[]{annotationClass},
>>                      new AnnotationInstanceProvider(annotationClass, values));
>> -
>> -            cache.put(key, annotation);
>> -        }
>> -
>> -        return annotation;
>> -    }
>> -
>> -    private static Map<String, Annotation> getAnnotationCache()
>> -    {
>> -        ClassLoader classLoader = ClassUtils.getClassLoader(null);
>> -        Map<String, Annotation> cache = annotationCache.get(classLoader);
>> -
>> -        if (cache == null)
>> -        {
>> -            cache = init(classLoader);
>> -        }
>> -
>> -        return cache;
>> -    }
>> -
>> -    private static synchronized Map<String, Annotation> init(ClassLoader 
>> classLoader)
>> -    {
>> -        // switch into paranoia mode
>> -        Map<String, Annotation> cache = annotationCache.get(classLoader);
>> -        if (cache == null)
>> -        {
>> -            cache = new ConcurrentHashMap<String, Annotation>();
>> -            annotationCache.put(classLoader, cache);
>> -        }
>> -        return cache;
>>      }
>> 
>>      /**
>> 
>> http://git-wip-us.apache.org/repos/asf/incubator-deltaspike/blob/3a582c96/deltaspike/core/api/src/test/java/org/apache/deltaspike/test/api/util/metadata/AnnotationInstanceProviderTest.java
>> ----------------------------------------------------------------------
>> diff --git 
>> a/deltaspike/core/api/src/test/java/org/apache/deltaspike/test/api/util/metadata/AnnotationInstanceProviderTest.java

>> b/deltaspike/core/api/src/test/java/org/apache/deltaspike/test/api/util/metadata/AnnotationInstanceProviderTest.java
>> index 377ff67..2c6311a 100644
>> --- 
>> a/deltaspike/core/api/src/test/java/org/apache/deltaspike/test/api/util/metadata/AnnotationInstanceProviderTest.java
>> +++ 
>> b/deltaspike/core/api/src/test/java/org/apache/deltaspike/test/api/util/metadata/AnnotationInstanceProviderTest.java
>> @@ -74,27 +74,6 @@ public class AnnotationInstanceProviderTest
>>      }
>> 
>>      @Test
>> -    public void assertSameInstance()
>> -    {
>> -        Annotation a1 = AnnotationInstanceProvider.of(RequestScoped.class);
>> -        Annotation a2 = AnnotationInstanceProvider.of(RequestScoped.class);
>> -
>> -        assertThat(a2, sameInstance(a1));
>> -    }
>> -
>> -    @Test
>> -    public void assertSameInstanceWithSameMembers()
>> -    {
>> -        Map<String, String> memberValues = new HashMap<String, 
>> String>();
>> -        memberValues.put("value", "test");
>> -
>> -        Annotation a1 = AnnotationInstanceProvider.of(Named.class, 
>> memberValues);
>> -        Annotation a2 = AnnotationInstanceProvider.of(Named.class, 
>> memberValues);
>> -
>> -        assertThat(a2, sameInstance(a1));
>> -    }
>> -
>> -    @Test
>>      public void assertDifferentInstanceWithDifferentMembers()
>>      {
>>          Map<String, String> memberValues = new HashMap<String, 
>> String>();
>> @@ -110,7 +89,7 @@ public class AnnotationInstanceProviderTest
>>      }
>> 
>>      @Test
>> -    public void assertSameInstanceUsingEquals()
>> +    public void assertSameUsingEquals()
>>      {
>>          Annotation a1 = AnnotationInstanceProvider.of(RequestScoped.class);
>>          Annotation a2 = AnnotationInstanceProvider.of(RequestScoped.class);
>> 


Mime
View raw message