I think that Gerhard had it right though. If DeltaSpike is used as a
library in say an EAR, it's broken anyway. I'm not sure how best to fix
this one. To his point of perf, my very scientific tests of running the
tests repeatedly from IDEA looked like they both had the same runtime perf.

We can always revert it if we decide we want it back. Certainly something
to discuss here on the list though.

On Tue, May 1, 2012 at 10:38 PM, Mark Struberg <[email protected]> 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: "[email protected]" <[email protected]>
> > To: [email protected]
> > 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 <[email protected]>
> > Authored: Tue May 1 21:48:43 2012 -0600
> > Committer: Jason Porter <[email protected]>
> > 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);
> >
>



-- 
Jason Porter
http://lightguard-jp.blogspot.com
http://twitter.com/lightguardjp

Software Engineer
Open Source Advocate
Author of Seam Catch - Next Generation Java Exception Handling

PGP key id: 926CCFF5
PGP key available at: keyserver.net, pgp.mit.edu

Reply via email to