fix for 152871: parsing bytecode too often

diff --git a/loadtime/src/org/aspectj/weaver/loadtime/ClassLoaderWeavingAdaptor.java b/loadtime/src/org/aspectj/weaver/loadtime/ClassLoaderWeavingAdaptor.java
index 1ea1397ca862eeab13fcca7ff18bf8726787ff21..75775e5ea612ec5a235c6a912a0fda5f0e3cb17c 100644 (file)
--- a/loadtime/src/org/aspectj/weaver/loadtime/ClassLoaderWeavingAdaptor.java
+++ b/loadtime/src/org/aspectj/weaver/loadtime/ClassLoaderWeavingAdaptor.java
@@ -36,10 +36,7 @@ import org.aspectj.weaver.ResolvedType;
 import org.aspectj.weaver.UnresolvedType;
 import org.aspectj.weaver.World;
 import org.aspectj.weaver.Lint.Kind;
-import org.aspectj.weaver.bcel.BcelObjectType;
 import org.aspectj.weaver.bcel.BcelWeaver;
-import org.aspectj.weaver.bcel.BcelWorld;
-import org.aspectj.weaver.bcel.Utility;
 import org.aspectj.weaver.loadtime.definition.Definition;
 import org.aspectj.weaver.loadtime.definition.DocumentParser;
 import org.aspectj.weaver.ltw.LTWWorld;
@@ -542,8 +539,9 @@ public class ClassLoaderWeavingAdaptor extends WeavingAdaptor {
         // does returns null or some other info for getResourceAsStream (f.e. WLS 9 CR248491)
         // Instead I parse the given bytecode. But this also means it will be parsed again in
         // new WeavingClassFileProvider() from WeavingAdaptor.getWovenBytes()...
-        BcelObjectType bct = ((BcelWorld)weaver.getWorld()).addSourceObjectType(Utility.makeJavaClass(null, bytes));
-        ResolvedType classInfo = bct.getResolvedTypeX();//BAD: weaver.getWorld().resolve(UnresolvedType.forName(className), true);
+        
+        ensureDelegateInitialized(className,bytes);
+        ResolvedType classInfo = delegateForCurrentClass.getResolvedTypeX();//BAD: weaver.getWorld().resolve(UnresolvedType.forName(className), true);
 
         //exclude are "AND"ed
         for (Iterator iterator = m_excludeTypePattern.iterator(); iterator.hasNext();) {
@@ -566,7 +564,8 @@ public class ClassLoaderWeavingAdaptor extends WeavingAdaptor {
         return accept;
     }
 
-    //FIXME we don't use include/exclude of others aop.xml
+
+       //FIXME we don't use include/exclude of others aop.xml
     //this can be nice but very dangerous as well to change that
     private boolean acceptAspect(String aspectClassName) {
         // avoid ResolvedType if not needed

diff --git a/tests/src/org/aspectj/systemtest/ajc153/Ajc153Tests.java b/tests/src/org/aspectj/systemtest/ajc153/Ajc153Tests.java
index bca2c144348d686638ceab4a9e11b94536b17c0e..e2e1e25822f7f1a156ff1d2e3cf1036f87bee3b8 100644 (file)
--- a/tests/src/org/aspectj/systemtest/ajc153/Ajc153Tests.java
+++ b/tests/src/org/aspectj/systemtest/ajc153/Ajc153Tests.java
@@ -16,6 +16,7 @@ import junit.framework.Test;
 
 import org.aspectj.testing.Utils;
 import org.aspectj.testing.XMLBasedAjcTestCase;
+import org.aspectj.weaver.bcel.Utility;
 
 public class Ajc153Tests extends org.aspectj.testing.XMLBasedAjcTestCase {
 
@@ -47,9 +48,16 @@ public class Ajc153Tests extends org.aspectj.testing.XMLBasedAjcTestCase {
   public void testCantFindType_pr149322_01() {runTest("can't find type on interface call 1");}
   public void testCantFindType_pr149322_02() {runTest("can't find type on interface call 2");}
   public void testCantFindType_pr149322_03() {runTest("can't find type on interface call 3");}
-
+  public void testParsingBytecodeLess_pr152871() { 
+         Utility.testingParseCounter=0;
+         runTest("parsing bytecode less"); 
+         assertTrue("Should have called parse 5 times, not "+Utility.testingParseCounter+" times",Utility.testingParseCounter==5);
+         // 5 means:   
+         // (1)=registerAspect   
+         // (2,3)=checkingIfShouldWeave,AcceptingResult for class
+         // (4,5)=checkingIfShouldWeave,AcceptingResult for aspect
+  }
   public void testMatchVolatileField_pr150671() {runTest("match volatile field");};
-
   public void testDuplicateJVMTIAgents_pr151938() {runTest("Duplicate JVMTI agents");};
 
   /////////////////////////////////////////

diff --git a/tests/src/org/aspectj/systemtest/ajc153/ajc153.xml b/tests/src/org/aspectj/systemtest/ajc153/ajc153.xml
index d0e9946140f35d67ea8231c0dcd997f1c2becc37..5b221f8d9ec3b20c8d2946c5ebc231ce29bcabaf 100644 (file)
--- a/tests/src/org/aspectj/systemtest/ajc153/ajc153.xml
+++ b/tests/src/org/aspectj/systemtest/ajc153/ajc153.xml
@@ -3,6 +3,19 @@
 <!-- AspectJ v1.5.3 Tests -->
 <suite>
 
+    <ajc-test dir="bugs153/pr152871" title="parsing bytecode less">
+      <compile files="MyClass.java" options="-1.5"/>
+      <compile files="MyAspect.java" options="-1.5 -Xlint:ignore"/>
+      <run class="a.MyClass" ltw="aop.xml">
+        <stdout>
+          <line text="advice running"/>
+          <line text="hello"/>
+          <line text="advice running"/>
+          <line text="world"/>
+           </stdout>
+      </run>
+    </ajc-test>
+    
     <ajc-test dir="bugs153/pr148381" title="argNames and javac">
       <!--compile files="C.java" options="-1.5"/>
       <compile files="A.java" options="-1.5"/-->

diff --git a/weaver/src/org/aspectj/weaver/bcel/UnwovenClassFile.java b/weaver/src/org/aspectj/weaver/bcel/UnwovenClassFile.java
index 52f3aa7e409b2423208d0b3154d5466beaf9aa52..09e5f48c390e66bfba7b3fbbe1d1a95bf57d62a1 100644 (file)
--- a/weaver/src/org/aspectj/weaver/bcel/UnwovenClassFile.java
+++ b/weaver/src/org/aspectj/weaver/bcel/UnwovenClassFile.java
@@ -37,6 +37,13 @@ public class UnwovenClassFile {
                this.filename = filename;
                this.bytes = bytes;
        }
+
+    /** Use if the classname is known, saves a bytecode parse */
+       public UnwovenClassFile(String filename, String classname,byte[] bytes) {
+               this.filename = filename;
+               this.className = classname;
+               this.bytes = bytes;
+       }
        
        public String getFilename() {
                return filename;

diff --git a/weaver/src/org/aspectj/weaver/bcel/Utility.java b/weaver/src/org/aspectj/weaver/bcel/Utility.java
index 1f99c526fb2b31f670ffcd48632ea6333553bb2d..c6e735cf80fa30bf16bbfd1a0fe0e347bcecc798 100644 (file)
--- a/weaver/src/org/aspectj/weaver/bcel/Utility.java
+++ b/weaver/src/org/aspectj/weaver/bcel/Utility.java
@@ -521,9 +521,13 @@ public class Utility {
                }
                return inst;
        }
+       
+       /** For testing purposes: bit clunky but does work */
+       public static int testingParseCounter=0;
 
        public static JavaClass makeJavaClass(String filename, byte[] bytes) {
                try {
+                       testingParseCounter++;
                    ClassParser parser = new ClassParser(new ByteArrayInputStream(bytes), filename);
             return parser.parse();
                } catch (IOException e) {

diff --git a/weaver/src/org/aspectj/weaver/tools/WeavingAdaptor.java b/weaver/src/org/aspectj/weaver/tools/WeavingAdaptor.java
index f62c7305e4f010e314ac518edffd6968c7f7db2b..b6d386e02c38a6e0bb5d7092f5116aee11b00334 100644 (file)
--- a/weaver/src/org/aspectj/weaver/tools/WeavingAdaptor.java
+++ b/weaver/src/org/aspectj/weaver/tools/WeavingAdaptor.java
@@ -46,6 +46,7 @@ import org.aspectj.weaver.bcel.BcelObjectType;
 import org.aspectj.weaver.bcel.BcelWeaver;
 import org.aspectj.weaver.bcel.BcelWorld;
 import org.aspectj.weaver.bcel.UnwovenClassFile;
+import org.aspectj.weaver.bcel.Utility;
 
 /**
  * This adaptor allows the AspectJ compiler to be embedded in an existing
@@ -77,6 +78,7 @@ public class WeavingAdaptor {
        private WeavingAdaptorMessageHandler messageHolder;
        protected GeneratedClassHandler generatedClassHandler;
        protected Map generatedClasses = new HashMap(); /* String -> UnwovenClassFile */
+       protected BcelObjectType delegateForCurrentClass; // lazily initialized, should be used to prevent parsing bytecode multiple times
 
        private static Trace trace = TraceFactory.getTraceFactory().getTrace(WeavingAdaptor.class);
 
@@ -207,21 +209,25 @@ public class WeavingAdaptor {
         */
        public byte[] weaveClass (String name, byte[] bytes) throws IOException {
                if (enabled) {
-               if (trace.isTraceEnabled()) trace.enter("weaveClass",this,new Object[] {name,bytes});
-
-                       if (shouldWeave(name, bytes)) {
-                               info("weaving '" + name + "'");
-                               bytes = getWovenBytes(name, bytes);
-                       } else if (shouldWeaveAnnotationStyleAspect(name, bytes)) {
-                   // an @AspectJ aspect needs to be at least munged by the aspectOf munger
-                   info("weaving '" + name + "'");
-                   bytes = getAtAspectJAspectBytes(name, bytes);
-               }
-                       else {
-                               info("not weaving '" + name + "'");
+                       try {
+                               delegateForCurrentClass=null; // TODO will need stack if going recursive...
+                       if (trace.isTraceEnabled()) trace.enter("weaveClass",this,new Object[] {name,bytes});
+                               if (shouldWeave(name, bytes)) {
+                                       info("weaving '" + name + "'");
+                                       bytes = getWovenBytes(name, bytes);
+                               } else if (shouldWeaveAnnotationStyleAspect(name, bytes)) {
+                           // an @AspectJ aspect needs to be at least munged by the aspectOf munger
+                           info("weaving '" + name + "'");
+                           bytes = getAtAspectJAspectBytes(name, bytes);
+                       }
+                               else {
+                                       info("not weaving '" + name + "'");
+                               }
+       
+                               if (trace.isTraceEnabled()) trace.exit("weaveClass",bytes);
+                       } finally {
+                               delegateForCurrentClass=null;
                        }
-
-                       if (trace.isTraceEnabled()) trace.exit("weaveClass",bytes);
                }
 
         return bytes;
@@ -253,8 +259,7 @@ public class WeavingAdaptor {
 
        private boolean shouldWeaveName (String name) {
                boolean should =
-                      !((/*(name.startsWith("org.apache.bcel.")//FIXME AV why ? bcel is wrapped in org.aspectj.
-                ||*/ name.startsWith("org.aspectj.")
+                      !((name.startsWith("org.aspectj.")
                 || name.startsWith("java.")
                 || name.startsWith("javax."))
                 //|| name.startsWith("$Proxy")//JDK proxies//FIXME AV is that 1.3 proxy ? fe. ataspect.$Proxy0 is a java5 proxy...
@@ -273,7 +278,13 @@ public class WeavingAdaptor {
        private boolean shouldWeaveAnnotationStyleAspect(String name, byte[] bytes) {
                // AV: instead of doing resolve that would lookup stuff on disk thru BCEL ClassLoaderRepository
         // we reuse bytes[] here to do a fast lookup for @Aspect annotation
-        return bcelWorld.isAnnotationStyleAspect(name, bytes);
+               ensureDelegateInitialized(name,bytes);
+               return (delegateForCurrentClass.isAnnotationStyleAspect());
+       }
+
+    protected void ensureDelegateInitialized(String name,byte[] bytes) {
+       if (delegateForCurrentClass==null)
+         delegateForCurrentClass =  ((BcelWorld)weaver.getWorld()).addSourceObjectType(Utility.makeJavaClass(name, bytes));
        }
 
        /**
@@ -478,17 +489,16 @@ public class WeavingAdaptor {
                private List unwovenClasses = new ArrayList(); /* List<UnovenClassFile> */
                private UnwovenClassFile wovenClass;
         private boolean isApplyAtAspectJMungersOnly = false;
-        private BcelObjectType delegate;
 
         public WeavingClassFileProvider (String name, byte[] bytes) {
-                       this.unwovenClass = new UnwovenClassFile(name,bytes);
+               ensureDelegateInitialized(name, bytes);
+                       this.unwovenClass = new UnwovenClassFile(name,delegateForCurrentClass.getResolvedTypeX().getName(),bytes);
                        this.unwovenClasses.add(unwovenClass);
                        
                        if (shouldDump(name.replace('/', '.'),true)) {
                                dump(name, bytes, true);
                        }
-
-                       delegate = bcelWorld.addSourceObjectType(unwovenClass.getJavaClass());
+                       
                }
 
         public void setApplyAtAspectJMungersOnly() {
@@ -539,8 +549,10 @@ public class WeavingAdaptor {
                                public void weavingClasses() {}
 
                                public void weaveCompleted() {
-                                       if (delegate!=null) delegate.weavingCompleted();
                                        ResolvedType.resetPrimitives();
+                                       if (delegateForCurrentClass!=null) delegateForCurrentClass.weavingCompleted();
+                                       ResolvedType.resetPrimitives();
+                                       //bcelWorld.discardType(typeBeingProcessed.getResolvedTypeX()); // work in progress
                                }
                        };                              
                }