[jboss-cvs] JBossAS SVN: r62280 - in projects/aop/trunk/aop/src: main/org/jboss/aop/util/reference and 2 other directories.

jboss-cvs-commits at lists.jboss.org jboss-cvs-commits at lists.jboss.org
Wed Apr 11 13:11:42 EDT 2007


Author: kabir.khan at jboss.com
Date: 2007-04-11 13:11:42 -0400 (Wed, 11 Apr 2007)
New Revision: 62280

Modified:
   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/JoinPointInfo.java
   projects/aop/trunk/aop/src/main/org/jboss/aop/util/reference/MethodPersistentReference.java
   projects/aop/trunk/aop/src/resources/test/stress/config.properties
   projects/aop/trunk/aop/src/test/org/jboss/test/aop/dynamicgenadvisor/DynamicTester.java
   projects/aop/trunk/aop/src/test/org/jboss/test/aop/dynamicgenadvisor/Interceptions.java
Log:
[JBAOP-378] Do not resolve field pointcuts again for instance advisors if they have no instance specific metadata. We can just copy the chains for fields and methods. When copying the chains, we make sure that they are cloned for the instance, rather than the same reference as the class advisor JoinPointInfos hold.

There is no point in creating constructor and construction chains for instances - the instance must already exist so these chains are just extra work.

Modified: projects/aop/trunk/aop/src/main/org/jboss/aop/ClassAdvisor.java
===================================================================
--- projects/aop/trunk/aop/src/main/org/jboss/aop/ClassAdvisor.java	2007-04-11 17:08:38 UTC (rev 62279)
+++ projects/aop/trunk/aop/src/main/org/jboss/aop/ClassAdvisor.java	2007-04-11 17:11:42 UTC (rev 62280)
@@ -425,7 +425,7 @@
       doesHaveAspects = adviceBindings.size() > 0;
    }
 
-   private void resolveFieldPointcut(ArrayList newFieldInfos, AdviceBinding binding, boolean write)
+   protected void resolveFieldPointcut(ArrayList newFieldInfos, AdviceBinding binding, boolean write)
    {
       for (int i = 0; i < advisedFields.length; i++)
       {

Modified: projects/aop/trunk/aop/src/main/org/jboss/aop/GeneratedClassAdvisor.java
===================================================================
--- projects/aop/trunk/aop/src/main/org/jboss/aop/GeneratedClassAdvisor.java	2007-04-11 17:08:38 UTC (rev 62279)
+++ projects/aop/trunk/aop/src/main/org/jboss/aop/GeneratedClassAdvisor.java	2007-04-11 17:11:42 UTC (rev 62280)
@@ -196,11 +196,41 @@
          super.resolveMethodPointcut(newMethodInterceptors, binding);
          handleOverriddenMethods(binding);
       }
-      else
+
+   }
+
+   @Override
+   protected void resolveFieldPointcut(ArrayList newFieldInfos, AdviceBinding binding, boolean write)
+   {
+      GeneratedClassAdvisor classAdvisor = getClassAdvisorIfInstanceAdvisorWithNoOwnDataWithEffectOnAdvices();
+      if (classAdvisor == null)
       {
+         //We are either the class advisor or an instanceadvisor with own data so we need to do all the work
+         super.resolveFieldPointcut(newFieldInfos, binding, write);
       }
    }
 
+   @Override
+   protected void resolveConstructorPointcut(ArrayList newConstructorInfos, AdviceBinding binding)
+   {
+      GeneratedClassAdvisor classAdvisor = getClassAdvisorIfInstanceAdvisorWithNoOwnDataWithEffectOnAdvices();
+      if (classAdvisor == null)
+      {
+         //We are either the class advisor or an instanceadvisor with own data so we need to do all the work
+         super.resolveConstructorPointcut(newConstructorInfos, binding);
+      }
+   }
+
+   @Override
+   protected void resolveConstructionPointcut(ArrayList newConstructionInfos, AdviceBinding binding)
+   {
+      GeneratedClassAdvisor classAdvisor = getClassAdvisorIfInstanceAdvisorWithNoOwnDataWithEffectOnAdvices();
+      if (classAdvisor == null)
+      {
+         //We are either the class advisor or an instanceadvisor with own data so we need to do all the work
+         super.resolveConstructionPointcut(newConstructionInfos, binding);
+      }
+   }
    
    protected void addMethodInfo(MethodInfo mi)
    {
@@ -383,97 +413,137 @@
       if (classAdvisor != null)
       {
          //We are an instance advisor with no own data influencing the chains, copy these from the parent advisor
-         long[] keys = newMethodInterceptors.keys();
-         for (int i = 0; i < keys.length; i++)
-         {
-            MethodInfo classMethodInfo = classAdvisor.getMethodInfo(keys[i]);
-            MethodMatchInfo matchInfo = newMethodInterceptors.getMatchInfo(keys[i]);
-            MethodInfo myMethodInfo = matchInfo.getInfo();
-            myMethodInfo.setInterceptorChain(classMethodInfo.getInterceptorChain());
-            myMethodInfo.setInterceptors(classMethodInfo.getInterceptors());
-            
-            if (updateOldInfo(oldInfos, myMethodInfo))
-            {
-               MethodJoinPointGenerator generator = getJoinPointGenerator(myMethodInfo);
-               generator.rebindJoinpoint(myMethodInfo);
-            }
-         }
+         easyFinalizeMethodChainForInstance(classAdvisor, newMethodInterceptors);
       }
       else
       {
          //We are either the class advisor or an instanceadvisor with own data so we need to do all the work
-         TLongObjectHashMap newMethodInfos = new TLongObjectHashMap();
+         fullWorkFinalizeMethodChain(newMethodInterceptors);
+      }
+   }
+
+   private void easyFinalizeMethodChainForInstance(ClassAdvisor classAdvisor, MethodInterceptors newMethodInterceptors)
+   {
+      long[] keys = newMethodInterceptors.keys();
+      for (int i = 0; i < keys.length; i++)
+      {
+         MethodInfo classMethodInfo = classAdvisor.getMethodInfo(keys[i]);
+         MethodMatchInfo matchInfo = newMethodInterceptors.getMatchInfo(keys[i]);
+         MethodInfo myMethodInfo = matchInfo.getInfo();
+         myMethodInfo.cloneChains(classMethodInfo);
+         
+         if (updateOldInfo(oldInfos, myMethodInfo))
+         {
+            MethodJoinPointGenerator generator = getJoinPointGenerator(myMethodInfo);
+            generator.rebindJoinpoint(myMethodInfo);
+         }
+      }
+   }
    
-         long[] keys = newMethodInterceptors.keys();
-         for (int i = 0; i < keys.length; i++)
+   private void fullWorkFinalizeMethodChain(MethodInterceptors newMethodInterceptors)
+   {
+      //We are either the class advisor or an instanceadvisor with own data so we need to do all the work
+      TLongObjectHashMap newMethodInfos = new TLongObjectHashMap();
+
+      long[] keys = newMethodInterceptors.keys();
+      for (int i = 0; i < keys.length; i++)
+      {
+         MethodMatchInfo matchInfo = newMethodInterceptors.getMatchInfo(keys[i]);
+         matchInfo.populateBindings();
+
+         MethodInfo info = matchInfo.getInfo();
+         newMethodInfos.put(keys[i], info);
+
+         MethodJoinPointGenerator generator = getJoinPointGenerator(info);
+         finalizeChainAndRebindJoinPoint(oldInfos, info, generator);
+      }
+      methodInterceptors = newMethodInfos;
+      
+      //Handle the overridden methods
+      if (overriddenMethods != null && overriddenMethods.size() > 0)
+      {
+         for (Iterator it = overriddenMethods.iterator() ; it.hasNext() ; )
          {
-            MethodMatchInfo matchInfo = newMethodInterceptors.getMatchInfo(keys[i]);
-            matchInfo.populateBindings();
-   
-            MethodInfo info = matchInfo.getInfo();
-            newMethodInfos.put(keys[i], info);
-   
+            MethodInfo info = (MethodInfo)it.next();
+
             MethodJoinPointGenerator generator = getJoinPointGenerator(info);
             finalizeChainAndRebindJoinPoint(oldInfos, info, generator);
          }
-         methodInterceptors = newMethodInfos;
-         
-         //Handle the overridden methods
-         if (overriddenMethods != null && overriddenMethods.size() > 0)
-         {
-            for (Iterator it = overriddenMethods.iterator() ; it.hasNext() ; )
-            {
-               MethodInfo info = (MethodInfo)it.next();
-   
-               MethodJoinPointGenerator generator = getJoinPointGenerator(info);
-               finalizeChainAndRebindJoinPoint(oldInfos, info, generator);
-            }
-         }      
-      }
+      }      
    }
 
    @Override
    protected void finalizeFieldReadChain(ArrayList newFieldInfos)
    {
-      for (int i = 0; i < newFieldInfos.size(); i++)
+      ClassAdvisor classAdvisor = getClassAdvisorIfInstanceAdvisorWithNoOwnDataWithEffectOnAdvices();
+      if (classAdvisor != null)
       {
-         FieldInfo info = (FieldInfo)newFieldInfos.get(i);
-         FieldJoinPointGenerator generator = getJoinPointGenerator(info);
-         finalizeChainAndRebindJoinPoint(oldFieldReadInfos, info, generator);
+         //We are an instance advisor with no own data influencing the chains, copy these from the parent advisor
+         easyFinalizeFieldChainForInstance(oldFieldReadInfos, classAdvisor.getFieldReadInfos(), newFieldInfos);
       }
+      else
+      {
+         //We are either the class advisor or an instanceadvisor with own data so we need to do all the work
+         fullWorkFinalizeFieldChain(oldFieldReadInfos, newFieldInfos);
+      }
    }
 
    @Override
    protected void finalizeFieldWriteChain(ArrayList newFieldInfos)
    {
+      ClassAdvisor classAdvisor = getClassAdvisorIfInstanceAdvisorWithNoOwnDataWithEffectOnAdvices();
+      if (classAdvisor != null)
+      {
+         //We are an instance advisor with no own data influencing the chains, copy these from the parent advisor
+         easyFinalizeFieldChainForInstance(oldInfos, classAdvisor.getFieldWriteInfos(), newFieldInfos);
+      }
+      else
+      {
+         //We are either the class advisor or an instanceadvisor with own data so we need to do all the work
+         fullWorkFinalizeFieldChain(oldInfos, newFieldInfos);
+      }
+   }
+
+   private void easyFinalizeFieldChainForInstance(Map oldFieldInfos, FieldInfo[] classFieldInfos, ArrayList newFieldInfos)
+   {
+      //We are an instance advisor with no own data influencing the chains, copy these from the parent advisor
+      if (newFieldInfos.size() > 0)
+      {
+         for (int i = 0; i < newFieldInfos.size(); i++)
+         {
+            FieldInfo myInfo = (FieldInfo)newFieldInfos.get(i);
+            myInfo.cloneChains(classFieldInfos[i]);
+            
+            if (updateOldInfo(oldFieldInfos, myInfo))
+            {
+               FieldJoinPointGenerator generator = getJoinPointGenerator(myInfo);
+               generator.rebindJoinpoint(myInfo);
+            }
+         }
+      }
+   }
+   
+   private void fullWorkFinalizeFieldChain(Map oldFieldInfos, ArrayList newFieldInfos)
+   {
+      //We are either the class advisor or an instanceadvisor with own data so we need to do all the work
       for (int i = 0; i < newFieldInfos.size(); i++)
       {
          FieldInfo info = (FieldInfo)newFieldInfos.get(i);
          FieldJoinPointGenerator generator = getJoinPointGenerator(info);
-         finalizeChainAndRebindJoinPoint(oldInfos, info, generator);
+         finalizeChainAndRebindJoinPoint(oldFieldInfos, info, generator);
       }
    }
-
+   
    @Override
    protected void finalizeConstructorChain(ArrayList newConstructorInfos)
    {
-      for (int i = 0; i < newConstructorInfos.size(); i++)
-      {
-         ConstructorInfo info = (ConstructorInfo) newConstructorInfos.get(i);
-         ConstructorJoinPointGenerator generator = getJoinPointGenerator(info);
-         finalizeChainAndRebindJoinPoint(oldInfos, info, generator);
-      }
+      advisorStrategy.finalizeConstructorChain(newConstructorInfos);
    }
 
    @Override
    protected void finalizeConstructionChain(ArrayList newConstructionInfos)
    {
-      for (int i = 0; i < newConstructionInfos.size(); i++)
-      {
-         ConstructionInfo info = (ConstructionInfo) newConstructionInfos.get(i);
-         ConstructionJoinPointGenerator generator = getJoinPointGenerator(info);
-         finalizeChainAndRebindJoinPoint(oldConstructionInfos, info, generator);
-      }
+      advisorStrategy.finalizeConstructionChain(newConstructionInfos);
    }
 
    @Override
@@ -656,14 +726,6 @@
       return PrecedenceSorter.applyPrecedence(interceptors, manager);
    }
 
-//   /**
-//    * Generated instance advisors will override this and return the parent class advisor
-//    */
-//   protected GeneratedClassAdvisor getParentAdvisor()
-//   {
-//      return parent;
-//   }
-
    /**
     * If this is an instance advisor, will check with parent class advisor if the aspect
     * is already registered. If so, we should use the one from the parent advisor
@@ -866,6 +928,10 @@
       Set getPerInstanceAspectDefinitions();
       Map getPerInstanceJoinpointAspectDefinitions();
       void rebuildInterceptors();
+      void resolveConstructorPointcut(ArrayList newConstructorInfos, AdviceBinding binding);
+      void resolveConstructionPointcut(ArrayList newConstructionInfos, AdviceBinding binding);
+      void finalizeConstructorChain(ArrayList newConstructorInfos);
+      void finalizeConstructionChain(ArrayList newConstructionInfos);
    }
    
    private class ClassAdvisorStrategy implements AdvisorStrategy
@@ -1051,6 +1117,36 @@
          version++;
          GeneratedClassAdvisor.super.rebuildInterceptors();
       }
+
+      public void resolveConstructorPointcut(ArrayList newConstructorInfos, AdviceBinding binding)
+      {
+         GeneratedClassAdvisor.this.resolveConstructorPointcut(newConstructorInfos, binding);
+      }
+
+      public void resolveConstructionPointcut(ArrayList newConstructionInfos, AdviceBinding binding)
+      {
+         GeneratedClassAdvisor.this.resolveConstructionPointcut(newConstructionInfos, binding);
+      }
+
+      public void finalizeConstructorChain(ArrayList newConstructorInfos)
+      {
+         for (int i = 0; i < newConstructorInfos.size(); i++)
+         {
+            ConstructorInfo info = (ConstructorInfo) newConstructorInfos.get(i);
+            ConstructorJoinPointGenerator generator = getJoinPointGenerator(info);
+            finalizeChainAndRebindJoinPoint(oldInfos, info, generator);
+         }
+      }
+
+      public void finalizeConstructionChain(ArrayList newConstructionInfos)
+      {
+         for (int i = 0; i < newConstructionInfos.size(); i++)
+         {
+            ConstructionInfo info = (ConstructionInfo) newConstructionInfos.get(i);
+            ConstructionJoinPointGenerator generator = getJoinPointGenerator(info);
+            finalizeChainAndRebindJoinPoint(oldConstructionInfos, info, generator);
+         }
+      }
    }
    
    private class InstanceAdvisorStrategy implements AdvisorStrategy 
@@ -1197,5 +1293,25 @@
             GeneratedClassAdvisor.super.rebuildInterceptors();
          }
       }
+
+      public void resolveConstructorPointcut(ArrayList newConstructorInfos, AdviceBinding binding)
+      {
+         //Since the instance already exists it makes no sense to have bindings for constructors
+      }
+
+      public void resolveConstructionPointcut(ArrayList newConstructionInfos, AdviceBinding binding)
+      {
+         //Since the instance already exists it makes no sense to have bindings for constructors         
+      }
+
+      public void finalizeConstructorChain(ArrayList newConstructorInfos)
+      {
+         //Since the instance already exists it makes no sense to have bindings for constructors
+      }
+
+      public void finalizeConstructionChain(ArrayList newConstructionInfos)
+      {
+         //Since the instance already exists it makes no sense to have bindings for constructors
+      }
    }
 }

Modified: projects/aop/trunk/aop/src/main/org/jboss/aop/JoinPointInfo.java
===================================================================
--- projects/aop/trunk/aop/src/main/org/jboss/aop/JoinPointInfo.java	2007-04-11 17:08:38 UTC (rev 62279)
+++ projects/aop/trunk/aop/src/main/org/jboss/aop/JoinPointInfo.java	2007-04-11 17:11:42 UTC (rev 62280)
@@ -156,4 +156,17 @@
    {
       return null;
    }
+   
+   public void cloneChains(JoinPointInfo other)
+   {
+      interceptorChain = (ArrayList)other.interceptorChain.clone();
+      if (other.interceptors == null)
+      {
+         interceptors = null;
+      }
+      else
+      {
+         interceptors = other.interceptors.clone();
+      }
+   }
 }

Modified: projects/aop/trunk/aop/src/main/org/jboss/aop/util/reference/MethodPersistentReference.java
===================================================================
--- projects/aop/trunk/aop/src/main/org/jboss/aop/util/reference/MethodPersistentReference.java	2007-04-11 17:08:38 UTC (rev 62279)
+++ projects/aop/trunk/aop/src/main/org/jboss/aop/util/reference/MethodPersistentReference.java	2007-04-11 17:11:42 UTC (rev 62280)
@@ -50,7 +50,6 @@
 		if ((returnValue=internalGet())!=null) return returnValue;
 		
 		Method aMethod = getMappedClass().getDeclaredMethod(name,getArguments());
-		aMethod.setAccessible(true);
 		buildReference(aMethod);
 		return aMethod;
 	}

Modified: projects/aop/trunk/aop/src/resources/test/stress/config.properties
===================================================================
--- projects/aop/trunk/aop/src/resources/test/stress/config.properties	2007-04-11 17:08:38 UTC (rev 62279)
+++ projects/aop/trunk/aop/src/resources/test/stress/config.properties	2007-04-11 17:11:42 UTC (rev 62280)
@@ -1,4 +1,4 @@
-warmup=1000
+warmup=10000
 loops=50000
 threads=1
 random_sleep_interval=false

Modified: projects/aop/trunk/aop/src/test/org/jboss/test/aop/dynamicgenadvisor/DynamicTester.java
===================================================================
--- projects/aop/trunk/aop/src/test/org/jboss/test/aop/dynamicgenadvisor/DynamicTester.java	2007-04-11 17:08:38 UTC (rev 62279)
+++ projects/aop/trunk/aop/src/test/org/jboss/test/aop/dynamicgenadvisor/DynamicTester.java	2007-04-11 17:11:42 UTC (rev 62280)
@@ -608,11 +608,14 @@
       assertEquals(Interceptions.getFieldReadName("MyAspect", "POJO", "i"), Interceptions.get(2));
 
 
+      System.out.println("================> removing binding");
       getInstanceDomain(pojo1).removeBinding(nameA);
+
       
       Interceptions.clear();
       pojo1.i = 50;
       assertEquals(50, pojo1.i);
+      Interceptions.printInterceptions();
       assertEquals(2, Interceptions.size());
       assertEquals(Interceptions.getFieldWriteName("YourInterceptor", "POJO", "i"), Interceptions.get(0));
       assertEquals(Interceptions.getFieldReadName("YourInterceptor", "POJO", "i"), Interceptions.get(1));

Modified: projects/aop/trunk/aop/src/test/org/jboss/test/aop/dynamicgenadvisor/Interceptions.java
===================================================================
--- projects/aop/trunk/aop/src/test/org/jboss/test/aop/dynamicgenadvisor/Interceptions.java	2007-04-11 17:08:38 UTC (rev 62279)
+++ projects/aop/trunk/aop/src/test/org/jboss/test/aop/dynamicgenadvisor/Interceptions.java	2007-04-11 17:11:42 UTC (rev 62280)
@@ -30,6 +30,11 @@
 {
    private static ArrayList interceptions = new ArrayList();
    
+   public static void printInterceptions()
+   {
+      System.out.println(interceptions);
+   }
+   
    public static void clear()
    {
       interceptions.clear();




More information about the jboss-cvs-commits mailing list