[jboss-cvs] JBossAS SVN: r73431 - in projects/aop/trunk/aop/src/main/org/jboss/aop: advice and 1 other directory.
jboss-cvs-commits at lists.jboss.org
jboss-cvs-commits at lists.jboss.org
Thu May 15 19:09:02 EDT 2008
Author: flavia.rainone at jboss.com
Date: 2008-05-15 19:09:02 -0400 (Thu, 15 May 2008)
New Revision: 73431
Modified:
projects/aop/trunk/aop/src/main/org/jboss/aop/Advisor.java
projects/aop/trunk/aop/src/main/org/jboss/aop/AspectManager.java
projects/aop/trunk/aop/src/main/org/jboss/aop/ClassAdvisor.java
projects/aop/trunk/aop/src/main/org/jboss/aop/GeneratedClassAdvisor.java
projects/aop/trunk/aop/src/main/org/jboss/aop/InstanceAdvisorDelegate.java
projects/aop/trunk/aop/src/main/org/jboss/aop/advice/GeneratedAdvisorInterceptor.java
Log:
[JBAOP-560] Changed slightly the logic of calling aspect factories, in order to avoid multiple calls to a factory when it returns null.
Modified: projects/aop/trunk/aop/src/main/org/jboss/aop/Advisor.java
===================================================================
--- projects/aop/trunk/aop/src/main/org/jboss/aop/Advisor.java 2008-05-15 21:06:18 UTC (rev 73430)
+++ projects/aop/trunk/aop/src/main/org/jboss/aop/Advisor.java 2008-05-15 23:09:02 UTC (rev 73431)
@@ -173,6 +173,12 @@
/** The meta data */
private MetaData metadata;
+ /**
+ * Indicates that a call to factory create has already been made, but the factory
+ * returned {@code null}.
+ */
+ protected static Object NULL_ASPECT = new Object();
+
public Advisor(String name, AspectManager manager)
{
this.name = name;
@@ -827,12 +833,22 @@
public Object getPerClassAspect(AspectDefinition def)
{
- return aspects.get(def.getName());
+ Object aspect = aspects.get(def.getName());
+ if (aspect == NULL_ASPECT)
+ {
+ return null;
+ }
+ return aspect;
}
public Object getPerClassAspect(String def)
{
- return aspects.get(def);
+ Object aspect = aspects.get(def);
+ if (aspect == NULL_ASPECT)
+ {
+ return null;
+ }
+ return aspect;
}
public void addPerClassAspect(AspectDefinition def)
@@ -841,7 +857,7 @@
Object aspect = def.getFactory().createPerClass(this);
if (aspect == null)
{
- return;
+ aspect = NULL_ASPECT;
}
aspects.put(def.getName(), aspect);
def.registerAdvisor(this);
Modified: projects/aop/trunk/aop/src/main/org/jboss/aop/AspectManager.java
===================================================================
--- projects/aop/trunk/aop/src/main/org/jboss/aop/AspectManager.java 2008-05-15 21:06:18 UTC (rev 73430)
+++ projects/aop/trunk/aop/src/main/org/jboss/aop/AspectManager.java 2008-05-15 23:09:02 UTC (rev 73431)
@@ -105,6 +105,9 @@
implements Translator
{
private static final Logger logger = AOPLogger.getLogger(AspectManager.class);
+
+ /** Indicates that a call to the factory has been made, but it returned null. */
+ private static final Object NULL_ASPECT = new Object();
/** Lock to be used when lazy creating the collections */
Object lazyCollectionLock = new Object();
@@ -1869,6 +1872,10 @@
}
}
}
+ if (aspect == NULL_ASPECT)
+ {
+ return null;
+ }
return aspect;
}
@@ -1886,8 +1893,14 @@
}
instance = adef.getFactory().createPerVM();
initPerVMAspectsMap();
- if (instance != null)
+ if (instance == null)
{
+ // indicates that the factory must not be called again
+ // the factory has already been called and returned null
+ perVMAspects.put(def, NULL_ASPECT);
+ }
+ else
+ {
perVMAspects.put(def, instance);
}
}
Modified: projects/aop/trunk/aop/src/main/org/jboss/aop/ClassAdvisor.java
===================================================================
--- projects/aop/trunk/aop/src/main/org/jboss/aop/ClassAdvisor.java 2008-05-15 21:06:18 UTC (rev 73430)
+++ projects/aop/trunk/aop/src/main/org/jboss/aop/ClassAdvisor.java 2008-05-15 23:09:02 UTC (rev 73431)
@@ -165,21 +165,20 @@
}
}
- Object aspect = map.get(joinpoint);
- if (aspect == null)
+ if (map.containsKey(joinpoint))
{
- synchronized (map)
+ return map.get(joinpoint);
+ }
+ synchronized (map)
+ {
+ if (map.containsKey(joinpoint))
{
- aspect = map.get(joinpoint);
- if (aspect == null)
- {
- aspect = def.getFactory().createPerJoinpoint(this, joinpoint);
- map.put(joinpoint, aspect);
- }
+ return map.get(joinpoint);
}
+ Object aspect = def.getFactory().createPerJoinpoint(this, joinpoint);
+ map.put(joinpoint, aspect);
+ return aspect;
}
-
- return aspect;
}
public Field[] getAdvisedFields()
Modified: projects/aop/trunk/aop/src/main/org/jboss/aop/GeneratedClassAdvisor.java
===================================================================
--- projects/aop/trunk/aop/src/main/org/jboss/aop/GeneratedClassAdvisor.java 2008-05-15 21:06:18 UTC (rev 73430)
+++ projects/aop/trunk/aop/src/main/org/jboss/aop/GeneratedClassAdvisor.java 2008-05-15 23:09:02 UTC (rev 73431)
@@ -889,11 +889,15 @@
perClassJoinpointAspectDefinitions.put(def, joinpoints);
}
- if (joinpoints.get(joinpoint) == null)
+ if (!joinpoints.containsKey(joinpoint))
{
Object aspect = def.getFactory().createPerJoinpoint(this, joinpoint);
- if (aspect != null)
+ if (aspect == null)
{
+ joinpoints.put(joinpoint, NULL_ASPECT);
+ }
+ else
+ {
joinpoints.put(joinpoint, aspect);
}
}
@@ -1463,7 +1467,11 @@
Map<Joinpoint, Object> joinpoints = perClassJoinpointAspectDefinitions.get(def);
if (joinpoints != null)
{
- return joinpoints.get(joinpoint);
+ Object aspect = joinpoints.get(joinpoint);
+ if (aspect != NULL_ASPECT)
+ {
+ return aspect;
+ }
}
return null;
}
Modified: projects/aop/trunk/aop/src/main/org/jboss/aop/InstanceAdvisorDelegate.java
===================================================================
--- projects/aop/trunk/aop/src/main/org/jboss/aop/InstanceAdvisorDelegate.java 2008-05-15 21:06:18 UTC (rev 73430)
+++ projects/aop/trunk/aop/src/main/org/jboss/aop/InstanceAdvisorDelegate.java 2008-05-15 23:09:02 UTC (rev 73431)
@@ -42,7 +42,8 @@
public class InstanceAdvisorDelegate implements Serializable
{
private static final long serialVersionUID = -5421366346785427537L;
-
+ /** Indicates that a call to the factory has been made, but the factory returned null */
+ private static final Object NULL_ASPECT = new Object();
protected transient WeakReference<Advisor> classAdvisor;
InstanceAdvisor instanceAdvisor;
protected transient WeakHashMap<AspectDefinition, Object> aspects;
@@ -152,8 +153,12 @@
{
Object aspect = def.getFactory().createPerJoinpoint(getClassAdvisor(),
instanceAdvisor, joinpoint);
- if (aspect != null)
+ if (aspect == null)
{
+ joins.put(joinpoint, NULL_ASPECT);
+ }
+ else
+ {
joins.put(joinpoint, aspect);
}
}
@@ -213,27 +218,41 @@
synchronized (this) // doublecheck, but I don't want to synchronize everywhere and dynamic aspects are rare
{
aspect = getJoinpointAspect(def, joinpoint);
- if (aspect != null) return aspect;
+ if (aspect != null)
+ {
+ if (aspect == NULL_ASPECT)
+ {
+ return null;
+ }
+ return aspect;
+ }
if (classAdvisor != null && getClassAdvisor() instanceof ClassAdvisor)
{
ClassAdvisor cadvisor = (ClassAdvisor) getClassAdvisor();
cadvisor.addPerInstanceJoinpointAspect(joinpoint, def);
aspect = def.getFactory().createPerJoinpoint(getClassAdvisor(), instanceAdvisor, joinpoint);
- if (aspect == null)
- {
- return null;
- }
WeakHashMap<AspectDefinition, ConcurrentHashMap<Joinpoint, Object>> copy = new WeakHashMap<AspectDefinition, ConcurrentHashMap<Joinpoint, Object>>(joinpointAspects);
Map<Joinpoint, Object> map = copy.get(def);
if (map == null)
{
map = new ConcurrentHashMap<Joinpoint, Object>();
}
- map.put(joinpoint, aspect);
+ if (aspect == null)
+ {
+ map.put(joinpoint, NULL_ASPECT);
+ }
+ else
+ {
+ map.put(joinpoint, aspect);
+ }
joinpointAspects = copy;
}
}
}
+ if (aspect == NULL_ASPECT)
+ {
+ return null;
+ }
return aspect;
}
Modified: projects/aop/trunk/aop/src/main/org/jboss/aop/advice/GeneratedAdvisorInterceptor.java
===================================================================
--- projects/aop/trunk/aop/src/main/org/jboss/aop/advice/GeneratedAdvisorInterceptor.java 2008-05-15 21:06:18 UTC (rev 73430)
+++ projects/aop/trunk/aop/src/main/org/jboss/aop/advice/GeneratedAdvisorInterceptor.java 2008-05-15 23:09:02 UTC (rev 73431)
@@ -67,6 +67,20 @@
public class GeneratedAdvisorInterceptor implements Interceptor
{
private static final Logger logger = AOPLogger.getLogger(GeneratedAdvisorInterceptor.class);
+ // important to indicate that the factory has already been called, but a null
+ // interceptor was returned
+ private static final Interceptor EMPTY_INTERCEPTOR = new Interceptor()
+ {
+ public Object invoke(Invocation invocation) throws Throwable
+ {
+ return invocation.invokeNext();
+ }
+
+ public String getName()
+ {
+ return "NULL";
+ }
+ };
private InterceptorFactory factory;
private volatile Object instance;
private String cflowString;
@@ -190,6 +204,11 @@
{
instance = advisor.getPerVMAspect(def);
}
+ if (instance == null || instance == EMPTY_INTERCEPTOR)
+ {
+ instance = EMPTY_INTERCEPTOR;
+ return null;
+ }
return instance;
}
else if (scope == Scope.PER_CLASS)
@@ -204,6 +223,12 @@
advisor.addPerClassAspect(def);
instance = advisor.getPerClassAspect(def);
}
+ if (instance == null || instance == EMPTY_INTERCEPTOR)
+ {
+ // indicates that the instance has already been retrieved
+ instance = EMPTY_INTERCEPTOR;
+ return null;
+ }
return instance;
}
else if (scope == Scope.PER_INSTANCE)
@@ -226,7 +251,15 @@
((GeneratedClassAdvisor)advisor).addPerClassJoinpointAspect(def, joinpoint);
instance = ((GeneratedClassAdvisor)advisor).getPerClassJoinpointAspect(def, joinpoint);
+ if (instance == null)
+ {
+ instance = EMPTY_INTERCEPTOR;
+ }
}
+ if (instance == EMPTY_INTERCEPTOR)
+ {
+ return null;
+ }
return instance;
}
else
@@ -269,6 +302,11 @@
instance = ((GeneratedClassAdvisor)advisor).getPerClassJoinpointAspect(def, joinpoint);
}
}
+ if (instance == null || instance == EMPTY_INTERCEPTOR)
+ {
+ instance = EMPTY_INTERCEPTOR;
+ return null;
+ }
return instance;
}
else
@@ -419,14 +457,14 @@
else
{
lazyInterceptor = create(invocation.getAdvisor(), getJoinpoint(invocation));
+ if (lazyInterceptor == null)
+ {
+ lazyInterceptor = EMPTY_INTERCEPTOR;
+ }
}
}
}
}
- if (lazyInterceptor == null)
- {
- return invocation.invokeNext();
- }
return lazyInterceptor.invoke(invocation);
}
More information about the jboss-cvs-commits
mailing list