From c779c96eff482f8b99fe5d7f85505f1792780281 Mon Sep 17 00:00:00 2001 From: aclement Date: Mon, 7 Aug 2006 13:04:28 +0000 Subject: [PATCH] fix for 152871: parsing bytecode too often --- .../loadtime/ClassLoaderWeavingAdaptor.java | 11 ++-- .../systemtest/ajc153/Ajc153Tests.java | 12 +++- .../org/aspectj/systemtest/ajc153/ajc153.xml | 13 +++++ .../aspectj/weaver/bcel/UnwovenClassFile.java | 7 +++ .../src/org/aspectj/weaver/bcel/Utility.java | 4 ++ .../aspectj/weaver/tools/WeavingAdaptor.java | 56 +++++++++++-------- 6 files changed, 73 insertions(+), 30 deletions(-) diff --git a/loadtime/src/org/aspectj/weaver/loadtime/ClassLoaderWeavingAdaptor.java b/loadtime/src/org/aspectj/weaver/loadtime/ClassLoaderWeavingAdaptor.java index 1ea1397ca..75775e5ea 100644 --- 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 bca2c1443..e2e1e2582 100644 --- 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 d0e994614..5b221f8d9 100644 --- a/tests/src/org/aspectj/systemtest/ajc153/ajc153.xml +++ b/tests/src/org/aspectj/systemtest/ajc153/ajc153.xml @@ -3,6 +3,19 @@ + + + + + + + + + + + + + diff --git a/weaver/src/org/aspectj/weaver/bcel/UnwovenClassFile.java b/weaver/src/org/aspectj/weaver/bcel/UnwovenClassFile.java index 52f3aa7e4..09e5f48c3 100644 --- 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 1f99c526f..c6e735cf8 100644 --- 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 f62c7305e..b6d386e02 100644 --- 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 */ 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 } }; } -- 2.39.5