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

jboss-cvs-commits at lists.jboss.org jboss-cvs-commits at lists.jboss.org
Wed Jun 18 13:19:56 EDT 2008


Author: stalep
Date: 2008-06-18 13:19:56 -0400 (Wed, 18 Jun 2008)
New Revision: 74807

Modified:
   projects/aop/trunk/aop/src/main/org/jboss/aop/ClassAdvisor.java
   projects/aop/trunk/aop/src/main/org/jboss/aop/JoinPointInfo.java
   projects/aop/trunk/aop/src/main/org/jboss/aop/pointcut/PointcutMethodMatch.java
   projects/aop/trunk/aop/src/test/org/jboss/test/aop/benchmark/DynamicAddRemoveBenchTester.java
   projects/aop/trunk/aop/src/test/org/jboss/test/aop/dynamic/SimpleDynamicTester.java
Log:
[JBAOP-578]
More optimal removal of advicebindings.


Modified: projects/aop/trunk/aop/src/main/org/jboss/aop/ClassAdvisor.java
===================================================================
--- projects/aop/trunk/aop/src/main/org/jboss/aop/ClassAdvisor.java	2008-06-18 16:51:56 UTC (rev 74806)
+++ projects/aop/trunk/aop/src/main/org/jboss/aop/ClassAdvisor.java	2008-06-18 17:19:56 UTC (rev 74807)
@@ -41,6 +41,7 @@
 import org.jboss.aop.advice.AdviceBinding;
 import org.jboss.aop.advice.AspectDefinition;
 import org.jboss.aop.advice.Interceptor;
+import org.jboss.aop.instrument.ConstructionJoinPointGenerator;
 import org.jboss.aop.instrument.ConstructorExecutionTransformer;
 import org.jboss.aop.instrument.FieldAccessTransformer;
 import org.jboss.aop.instrument.MethodExecutionTransformer;
@@ -740,60 +741,23 @@
       
       if (BindingClassifier.isExecution(removedBinding))
       {
-         //TODO: this method is more optimal, but needs further testing...
-//         updateMethodPointcutAfterRemove(removedBinding);
-         resetChain(methodInfos);
-         synchronized (manager.getBindings())
-         {
-            for(AdviceBinding ab : manager.getBindings().values())
-            {
-               if(BindingClassifier.isMethodExecution(ab))
-                  resolveMethodPointcut(ab);
-            }
-         }
+         updateMethodPointcutAfterRemove(removedBinding);
       }
       if (BindingClassifier.isGet(removedBinding) || BindingClassifier.isSet(removedBinding))
       {
-         resetChain(fieldReadInfos);
-         resetChain(fieldWriteInfos);
-         synchronized (manager.getBindings())
-         {
-            for(AdviceBinding ab : manager.getBindings().values())
-            {
-               if(BindingClassifier.isGet(ab))
-                  resolveFieldPointcut(fieldReadInfos, ab, false);
-               if(BindingClassifier.isSet(ab))
-                  resolveFieldPointcut(fieldWriteInfos, ab, true);
-            }
-         }
+         updateFieldPointcutAfterRemove(fieldReadInfos, removedBinding, false);
+         updateFieldPointcutAfterRemove(fieldWriteInfos, removedBinding, true);
       }
       if (BindingClassifier.isConstructorExecution(removedBinding))
       {
-         resetChain(constructorInfos);
-         synchronized (manager.getBindings())
-         {
-            for(AdviceBinding ab : manager.getBindings().values())
-            {
-               if(BindingClassifier.isConstructorExecution(ab))
-                  resolveConstructorPointcut(ab);
-            }
-         }
+         updateConstructorPointcutAfterRemove(removedBinding);
       }
       if (BindingClassifier.isConstruction(removedBinding))
       {
-         resetChain(constructionInfos);
-         synchronized (manager.getBindings())
-         {
-            for(AdviceBinding ab : manager.getBindings().values())
-            {
-               if(BindingClassifier.isConstruction(ab))
-                  resolveConstructionPointcut(ab);
-            }
-         }
+         updateConstructionPointcutAfterRemove(removedBinding);
       }
 
       finalizeChains();
-
       unlockWriteChains();
       
       // Notify observer about this change
@@ -812,7 +776,7 @@
       {
 
       }
-
+      
    }
    
    protected void updateFieldPointcutAfterRemove(FieldInfo[] fieldInfos, AdviceBinding binding, boolean write)
@@ -820,25 +784,46 @@
       for (int i = 0; i < fieldInfos.length; i++)
       {
          Field field = fieldInfos[i].getField();
+         fieldInfos[i].resetInterceptors();
 
          if ((!write && binding.getPointcut().matchesGet(this, field))
          || (write && binding.getPointcut().matchesSet(this, field)))
          {
             if (AspectManager.verbose) System.err.println("[debug] Removing field, matched " + ((write) ? "write" : "read") + " binding: " + field);
             fieldInfos[i].clear();
+            
+            for(AdviceBinding ab : manager.getBindings().values())
+            {
+               if ((!write && ab.getPointcut().matchesGet(this, field))
+                     || (write && ab.getPointcut().matchesSet(this, field)))
+               {    
+                  pointcutResolved(fieldInfos[i], ab, new FieldJoinpoint(field));
+               }
+            }
          }
       }
    }
    
    protected void updateConstructorPointcutAfterRemove(AdviceBinding binding)
    {
-      for (int i = 0; i < constructors.length; i++)
+      if(constructorInfos != null && constructorInfos.length > 0)
       {
-         Constructor<?> constructor = constructors[i];
-         if (binding.getPointcut().matchesExecution(this, constructor))
+         for (int i = 0; i < constructors.length; i++)
          {
-            if (AspectManager.verbose) System.err.println("[debug] Removing constructor, matched binding: " + constructor);
-            constructorInfos[i].clear();
+            constructorInfos[i].resetInterceptors();
+            Constructor<?> constructor = constructors[i];
+            if (binding.getPointcut().matchesExecution(this, constructor))
+            {
+               if (AspectManager.verbose) System.err.println("[debug] Removing constructor, matched binding: " + constructor);
+               constructorInfos[i].clear();
+               for(AdviceBinding ab : manager.getBindings().values())
+               {
+                  if (ab.getPointcut().matchesExecution(this, constructor))
+                  {
+                     pointcutResolved(constructorInfos[i], ab, new ConstructorJoinpoint(constructor));
+                  }
+               }
+            }
          }
       }
    }
@@ -849,12 +834,20 @@
       {
          for (int i = 0; i < constructionInfos.length ;i++)
          {
+            constructionInfos[i].resetInterceptors();
             ConstructionInfo info = constructionInfos[i];
             Constructor<?> constructor = info.getConstructor();
             if (binding.getPointcut().matchesConstruction(this, constructor))
             {
                if (AspectManager.verbose) System.err.println("[debug] Removing construction, matched binding: " + constructor);
                constructionInfos[i].clear();
+               for(AdviceBinding ab : manager.getBindings().values())
+               {
+                  if (binding.getPointcut().matchesConstruction(this, constructor))
+                  {
+                     pointcutResolved(constructionInfos[i], ab, new ConstructorJoinpoint(constructor));
+                  }
+               }
             }
          }
       }

Modified: projects/aop/trunk/aop/src/main/org/jboss/aop/JoinPointInfo.java
===================================================================
--- projects/aop/trunk/aop/src/main/org/jboss/aop/JoinPointInfo.java	2008-06-18 16:51:56 UTC (rev 74806)
+++ projects/aop/trunk/aop/src/main/org/jboss/aop/JoinPointInfo.java	2008-06-18 17:19:56 UTC (rev 74807)
@@ -88,6 +88,11 @@
       interceptorChain.clear();
    }
    
+   protected void resetInterceptors()
+   {
+      interceptors = new Interceptor[0];
+   }
+   
    public Advisor getAdvisor() 
    {
       if (advisor == null)

Modified: projects/aop/trunk/aop/src/main/org/jboss/aop/pointcut/PointcutMethodMatch.java
===================================================================
--- projects/aop/trunk/aop/src/main/org/jboss/aop/pointcut/PointcutMethodMatch.java	2008-06-18 16:51:56 UTC (rev 74806)
+++ projects/aop/trunk/aop/src/main/org/jboss/aop/pointcut/PointcutMethodMatch.java	2008-06-18 17:19:56 UTC (rev 74807)
@@ -66,6 +66,7 @@
    {
       if(o != null && o instanceof PointcutMethodMatch &&
             this.getMatchLevel() == ((PointcutMethodMatch) o).getMatchLevel() &&
+            this.getMatchedClass() != null && ((PointcutMethodMatch) o).getMatchedClass() != null &&
             this.getMatchedClass().getName().equals(((PointcutMethodMatch) o).getMatchedClass().getName()) &&
             this.isInstanceOf() == ((PointcutMethodMatch) o).isInstanceOf())
          return true;

Modified: projects/aop/trunk/aop/src/test/org/jboss/test/aop/benchmark/DynamicAddRemoveBenchTester.java
===================================================================
--- projects/aop/trunk/aop/src/test/org/jboss/test/aop/benchmark/DynamicAddRemoveBenchTester.java	2008-06-18 16:51:56 UTC (rev 74806)
+++ projects/aop/trunk/aop/src/test/org/jboss/test/aop/benchmark/DynamicAddRemoveBenchTester.java	2008-06-18 17:19:56 UTC (rev 74807)
@@ -52,7 +52,7 @@
       super(name);
    }
    
-   public void testPerformance() throws ParseException
+   public void testMethodPerformance() throws ParseException
    {
       int size = 500;
       ArrayList<AdviceBinding> bindings = new ArrayList<AdviceBinding>(size);
@@ -65,7 +65,7 @@
          binding.addInterceptor(Interceptor1.class);
          bindings.add(binding);
       }
-      System.out.println("Creating the bindings took: "+(System.currentTimeMillis()-time));
+      System.out.println("Creating the methodbindings took: "+(System.currentTimeMillis()-time));
       
       POJO p = new POJO();
       p.foo();
@@ -75,18 +75,61 @@
       {
          AspectManager.instance().addBinding(ab);
       }
-      System.out.println("Adding the bindings took: "+(System.currentTimeMillis()-time));
+      System.out.println("Adding the methodbindings took: "+(System.currentTimeMillis()-time));
       
       time = System.currentTimeMillis();
       for(AdviceBinding ab : bindings)
       {
          AspectManager.instance().removeBinding(ab.getName());
       }
-      System.out.println("Removing the bindings took: "+(System.currentTimeMillis()-time));
+      System.out.println("Removing the methodbindings took: "+(System.currentTimeMillis()-time));
       
       
       assertTrue(true);
    }
+   
+   public void testFieldPerformance() throws ParseException
+   {
+      int size = 500;
+      ArrayList<AdviceBinding> bindings = new ArrayList<AdviceBinding>(size);
+      long time = System.currentTimeMillis();
+      for(int i = 0; i < size/2; i++)
+      {
+         AdviceBinding binding = new AdviceBinding( 
+               "set(* org.jboss.test.aop.benchmark.POJO->i)", null); 
+         binding.setName("set"+i);
+         binding.addInterceptor(Interceptor1.class);
+         bindings.add(binding);
+         
+         AdviceBinding binding2 = new AdviceBinding( 
+               "get(* org.jboss.test.aop.benchmark.POJO->i)", null); 
+         binding2.setName("get"+i);
+         binding2.addInterceptor(Interceptor1.class);
+         bindings.add(binding2);
+         
+      }
+      System.out.println("Creating the fieldbindings took: "+(System.currentTimeMillis()-time));
+      
+      POJO p = new POJO();
+      p.foo();
+      
+      time = System.currentTimeMillis();
+      for(AdviceBinding ab : bindings)
+      {
+         AspectManager.instance().addBinding(ab);
+      }
+      System.out.println("Adding the fieldbindings took: "+(System.currentTimeMillis()-time));
+      
+      time = System.currentTimeMillis();
+      for(AdviceBinding ab : bindings)
+      {
+         AspectManager.instance().removeBinding(ab.getName());
+      }
+      System.out.println("Removing the fieldbindings took: "+(System.currentTimeMillis()-time));
+      
+      
+      assertTrue(true);
+   }
 
 
 }

Modified: projects/aop/trunk/aop/src/test/org/jboss/test/aop/dynamic/SimpleDynamicTester.java
===================================================================
--- projects/aop/trunk/aop/src/test/org/jboss/test/aop/dynamic/SimpleDynamicTester.java	2008-06-18 16:51:56 UTC (rev 74806)
+++ projects/aop/trunk/aop/src/test/org/jboss/test/aop/dynamic/SimpleDynamicTester.java	2008-06-18 17:19:56 UTC (rev 74807)
@@ -285,22 +285,19 @@
       POJO p = new POJO();
       p.field = 7;
       assertTrue("POJO.field was not intercepted", Interceptor1.intercepted);
-      System.out.println("NumberOfInterceptions is: "+Interceptor1.numberOfInterceptions);
+      assertEquals("POJO.field should have been intercepted 2 times", 2, Interceptor1.numberOfInterceptions);
 
       Interceptor2.intercepted = false;
       int i = p.field;
       assertTrue("POJO.field was not intercepted", Interceptor2.intercepted);
-      System.out.println("NumberOfInterceptions is: "+Interceptor1.numberOfInterceptions);
-
+      assertEquals("POJO.field should have been intercepted 3 times", 3, Interceptor1.numberOfInterceptions);
+      
       Interceptor1.intercepted = false;
       AspectManager.instance().removeBinding("set1");
 
       p.field = 4;
-      System.out.println("NumberOfInterceptions is: "+Interceptor1.numberOfInterceptions);
       assertEquals("POJO.field should have been intercepted 4 times", 4, Interceptor1.numberOfInterceptions);
-      //      assertFalse("POJO.field was intercepted, shouldnt be.", Interceptor1.intercepted);
 
-
       Interceptor2.intercepted = false;
       AspectManager.instance().removeBinding("get1");
       AspectManager.instance().removeBinding("field1");




More information about the jboss-cvs-commits mailing list