diff options
-rw-r--r-- | org.aspectj.ajdt.core/src/org/aspectj/ajdt/internal/compiler/AjCompilerAdapter.java | 9 | ||||
-rw-r--r-- | tests/cflow/CounterTest01.java | 11 | ||||
-rw-r--r-- | tests/cflow/CounterTest02.java | 85 | ||||
-rw-r--r-- | tests/cflow/CounterTest03.java | 75 | ||||
-rw-r--r-- | tests/cflow/CounterTest04.java | 76 | ||||
-rw-r--r-- | tests/cflow/CounterTest05.java | 51 | ||||
-rw-r--r-- | tests/src/org/aspectj/systemtest/ajc121/Ajc121Tests.java | 20 | ||||
-rw-r--r-- | tests/src/org/aspectj/systemtest/ajc121/ajc121-tests.xml | 27 | ||||
-rw-r--r-- | weaver/src/org/aspectj/weaver/bcel/BcelWeaver.java | 2 | ||||
-rw-r--r-- | weaver/src/org/aspectj/weaver/patterns/CflowPointcut.java | 101 |
10 files changed, 424 insertions, 33 deletions
diff --git a/org.aspectj.ajdt.core/src/org/aspectj/ajdt/internal/compiler/AjCompilerAdapter.java b/org.aspectj.ajdt.core/src/org/aspectj/ajdt/internal/compiler/AjCompilerAdapter.java index d7c1b9c2a..ccb9821b9 100644 --- a/org.aspectj.ajdt.core/src/org/aspectj/ajdt/internal/compiler/AjCompilerAdapter.java +++ b/org.aspectj.ajdt.core/src/org/aspectj/ajdt/internal/compiler/AjCompilerAdapter.java @@ -25,6 +25,7 @@ import org.aspectj.bridge.IMessageHandler; import org.aspectj.bridge.IProgressListener; import org.aspectj.weaver.bcel.BcelWeaver; import org.aspectj.weaver.bcel.BcelWorld; +import org.aspectj.weaver.patterns.CflowPointcut; import org.aspectj.org.eclipse.jdt.internal.compiler.CompilationResult; import org.aspectj.org.eclipse.jdt.internal.compiler.Compiler; import org.aspectj.org.eclipse.jdt.internal.compiler.ICompilerAdapter; @@ -235,8 +236,12 @@ public class AjCompilerAdapter implements ICompilerAdapter { // addAllKnownClassesToWeaveList(); // resultsPendingWeave.addAll(getBinarySourcesFrom(binarySourceSetForFullWeave)); // } - - weaver.weave(new WeaverAdapter(this,weaverMessageHandler,progressListener)); + try { + weaver.weave(new WeaverAdapter(this,weaverMessageHandler,progressListener)); + } finally { + // ???: is this the right point for this? After weaving has finished clear the caches. + CflowPointcut.clearCaches(); + } } private void addAllKnownClassesToWeaveList() { diff --git a/tests/cflow/CounterTest01.java b/tests/cflow/CounterTest01.java index 1578ea0f8..0240d6056 100644 --- a/tests/cflow/CounterTest01.java +++ b/tests/cflow/CounterTest01.java @@ -4,13 +4,20 @@ import java.util.ArrayList; import java.util.Iterator; import java.util.List; +/** + * In this testcase we are using cflow() with a pointcut that doesn't include state - + * this should be managed by our new CflowCounter rather than a CflowStack. + * + * Because the two cflow pointcuts are identical (both are 'cflow(execution(* main(..))' it also + * means we can share a single counter for both of them ! + */ public class CounterTest01 { public static void main(String []argv) { new CounterTest01().sayMessage(); int ctrs = ReflectionHelper.howManyCflowCounterFields(Cflow1.aspectOf()); - if (ctrs!=2) { - throw new RuntimeException("Should be two cflow counters, but found: "+ctrs); + if (ctrs!=1) { + throw new RuntimeException("Should be one cflow counter, but found: "+ctrs); } int stacks = ReflectionHelper.howManyCflowStackFields(Cflow1.aspectOf()); if (stacks!=1) { diff --git a/tests/cflow/CounterTest02.java b/tests/cflow/CounterTest02.java new file mode 100644 index 000000000..e596d7bfa --- /dev/null +++ b/tests/cflow/CounterTest02.java @@ -0,0 +1,85 @@ +import java.lang.reflect.Field; +import java.lang.reflect.Modifier; +import java.util.ArrayList; +import java.util.Iterator; +import java.util.List; + +/** + * In this testcase we create a pointcut p1() which uses cflow and then we reference + * it in two other anonymous pointcuts attached to advice. The cflow() should be managed + * by a counter (as no state is maintained) and the reused pointcut should not mean two + * counters are created. One counter should be created and shared. + */ +public class CounterTest02 { + + public static void main(String []argv) { + new CounterTest02().sayMessage(); + int ctrs = ReflectionHelper.howManyCflowCounterFields(Cflow1.aspectOf()); + int stacks = ReflectionHelper.howManyCflowStackFields(Cflow1.aspectOf()); + if (ctrs!=1) + throw new RuntimeException("Should be one cflow counter, but found: "+ctrs); + if (stacks!=1) + throw new RuntimeException("Should be one cflow stacks, but found: "+stacks); + if (Cflow1.stackAdvice!=2) + throw new RuntimeException("Expected two piece of stack advice to run: "+Cflow1.stackAdvice); + if (Cflow1.counterAdvice!=4) + throw new RuntimeException("Expected four pieces of counter advice to run: "+Cflow1.counterAdvice); + + } + + public void sayMessage() { + printmsg("Hello "); printmsg("World\n"); + } + + public void printmsg(String msg) { + System.out.print(msg); + } +} + +aspect Cflow1 { + public static int stackAdvice = 0; + public static int counterAdvice = 0; + + // CflowCounter created for this pointcut should be shared below! + pointcut p1(): cflow(execution(* main(..))); + + before(): call(* print(..)) && p1() { + // Managed by a CflowCounter + Cflow1.counterAdvice++; + } + + before(): call(* print(..)) && p1() { + // Managed by a CflowCounter + Cflow1.counterAdvice++; + } + + before(Object o): call(* print(..)) && cflow(execution(* main(..)) && args(o)) { + // Managed by a CflowStack - since state is exposed + Cflow1.stackAdvice++; + } +} + +class ReflectionHelper { + public static List getCflowfields(Object o,boolean includeCounters,boolean includeStacks) { + List res = new ArrayList(); + Class clazz = o.getClass(); + Field[] fs = clazz.getDeclaredFields(); + for (int i = 0; i < fs.length; i++) { + Field f = fs[i]; + if ((f.getType().getName().endsWith("CFlowCounter") && includeCounters) || + (f.getType().getName().endsWith("CFlowStack") && includeStacks)) { + res.add(f.getType().getName()+":"+f.getName()); + } + } + return res; + } + + public static int howManyCflowCounterFields(Object o) { + return getCflowfields(o,true,false).size(); + } + + public static int howManyCflowStackFields(Object o) { + return getCflowfields(o,false,true).size(); + } + +} diff --git a/tests/cflow/CounterTest03.java b/tests/cflow/CounterTest03.java new file mode 100644 index 000000000..048f2fe86 --- /dev/null +++ b/tests/cflow/CounterTest03.java @@ -0,0 +1,75 @@ +import java.lang.reflect.Field; +import java.lang.reflect.Modifier; +import java.util.ArrayList; +import java.util.Iterator; +import java.util.List; + +/** + * In this test, a cflow() pointcut is named and then reused. It refers to state and so + * we must manage it with a CFlowStack - can we share the stacks? + */ +public class CounterTest03 { + + public static void main(String []argv) { + new CounterTest03().sayMessage(); + int ctrs = ReflectionHelper.howManyCflowCounterFields(Cflow1.aspectOf()); + if (ctrs!=0) { + throw new RuntimeException("Should be zero cflow counters, but found: "+ctrs); + } + int stacks = ReflectionHelper.howManyCflowStackFields(Cflow1.aspectOf()); + if (stacks!=1) { + throw new RuntimeException("Should be one cflow stacks, but found: "+stacks); + } + } + + public void sayMessage() { + printmsg("Hello "); printmsg("World\n"); + } + + public void printmsg(String msg) { + System.out.print(msg); + } +} + +aspect Cflow1 { + + // CflowCounter created for this pointcut should be shared below! + pointcut p1(Object o): cflow(execution(* main(..)) && args(o)); + + before(Object o): call(* print(..)) && p1(o) { + // Managed by a CflowCounter + } + + before(Object o): call(* print(..)) && p1(o) { + // Managed by a CflowCounter + } + +// before(Object o): execution(* print(..)) && cflow(execution(* main(..)) && target(o)) { +// // Managed by a CflowStack - since state is exposed +// } +} + +class ReflectionHelper { + public static List getCflowfields(Object o,boolean includeCounters,boolean includeStacks) { + List res = new ArrayList(); + Class clazz = o.getClass(); + Field[] fs = clazz.getDeclaredFields(); + for (int i = 0; i < fs.length; i++) { + Field f = fs[i]; + if ((f.getType().getName().endsWith("CFlowCounter") && includeCounters) || + (f.getType().getName().endsWith("CFlowStack") && includeStacks)) { + res.add(f.getType().getName()+":"+f.getName()); + } + } + return res; + } + + public static int howManyCflowCounterFields(Object o) { + return getCflowfields(o,true,false).size(); + } + + public static int howManyCflowStackFields(Object o) { + return getCflowfields(o,false,true).size(); + } + +} diff --git a/tests/cflow/CounterTest04.java b/tests/cflow/CounterTest04.java new file mode 100644 index 000000000..9f20e940b --- /dev/null +++ b/tests/cflow/CounterTest04.java @@ -0,0 +1,76 @@ +import java.lang.reflect.Field; +import java.lang.reflect.Modifier; +import java.util.ArrayList; +import java.util.Iterator; +import java.util.List; + +/** + * In this test, we have multiple identical cflow() pointcut 'pieces' used in a number of + * pointcuts. We are not naming a cflow() and reusing it, we are just duplicating the + * pointcut in multiple places - can we share counters? + */ +public class CounterTest04 { + + public static void main(String []argv) { + new CounterTest04().sayMessage(); + int ctrs = ReflectionHelper.howManyCflowCounterFields(Cflow1.aspectOf()); +// if (ctrs!=2) { +// throw new RuntimeException("Should be two cflow counters, but found: "+ctrs); +// } + int stacks = ReflectionHelper.howManyCflowStackFields(Cflow1.aspectOf()); + if (stacks!=2) { + throw new RuntimeException("Should be two cflow stacks, but found: "+stacks); + } + } + + public void sayMessage() { + printmsg("Hello "); printmsg("World\n"); + } + + public void printmsg(String msg) { + System.out.print(msg); + } +} + +aspect Cflow1 { + + // CflowCounter created for this pointcut should be shared below! + pointcut p1(Object o): cflow(execution(* main(..)) && args(o)); + + before(Object o): call(* print(..)) && p1(o) { + // Managed by a CflowCounter + } + + before(Object o): call(* print(..)) && p1(o) { + // Managed by a CflowCounter + } + + before(Object o): execution(* print(..)) && cflow(execution(* main(..)) && target(o)) { + // Managed by a CflowStack - since state is exposed + } +} + +class ReflectionHelper { + public static List getCflowfields(Object o,boolean includeCounters,boolean includeStacks) { + List res = new ArrayList(); + Class clazz = o.getClass(); + Field[] fs = clazz.getDeclaredFields(); + for (int i = 0; i < fs.length; i++) { + Field f = fs[i]; + if ((f.getType().getName().endsWith("CFlowCounter") && includeCounters) || + (f.getType().getName().endsWith("CFlowStack") && includeStacks)) { + res.add(f.getType().getName()+":"+f.getName()); + } + } + return res; + } + + public static int howManyCflowCounterFields(Object o) { + return getCflowfields(o,true,false).size(); + } + + public static int howManyCflowStackFields(Object o) { + return getCflowfields(o,false,true).size(); + } + +} diff --git a/tests/cflow/CounterTest05.java b/tests/cflow/CounterTest05.java new file mode 100644 index 000000000..44c4b3ecc --- /dev/null +++ b/tests/cflow/CounterTest05.java @@ -0,0 +1,51 @@ +public class CounterTest05 { + +/* + * Here we have an abstract pointcut that is used within a cflow. In the two concrete sub-aspects + * we make the abstract pointcut concrete. The aim of the test is to ensure we do not share + * the cflow counter objects, since the pointcut within the cflow() in each case points at a + * different 'entry' point. The count should be 10 when we finish. If it is 8 we have shared + * a counter. + */ + public static void main(String []argv) { + print(); + print(); + below1(); + System.err.println("ctr="+A.ctr); + if (A.ctr!=10) + throw new RuntimeException("Counter should be 10 but is "+A.ctr); + } + + public static void below1() { + print(); + print(); + below2(); + } + + public static void below2() { + print(); + print(); + } + +public static void print() {} +} + +abstract aspect A { + public static int ctr = 0; + + abstract pointcut abs(); + + pointcut p(): call(* print(..)) && cflow(abs()); + + before(): p() { + A.ctr++; + } +} + +aspect B extends A { + pointcut abs(): execution(* main(..)); // ctr increases by 6 +} + +aspect C extends A { + pointcut abs(): execution(* below1(..)); // ctr increases by 4 +} diff --git a/tests/src/org/aspectj/systemtest/ajc121/Ajc121Tests.java b/tests/src/org/aspectj/systemtest/ajc121/Ajc121Tests.java index 82574a339..4e0cca88e 100644 --- a/tests/src/org/aspectj/systemtest/ajc121/Ajc121Tests.java +++ b/tests/src/org/aspectj/systemtest/ajc121/Ajc121Tests.java @@ -308,14 +308,30 @@ public class Ajc121Tests extends org.aspectj.testing.XMLBasedAjcTestCase { public void test057_decSoftWithSuper() { runTest("declare soft can cause programs with invalid exception behaviour to be generated"); - } + } public void test058_npeOnTJPerror() { runTest("NPE on thisJoinPoint mistake"); } public void test059_cflowOptimization_counters() { - runTest("Optimization of cflow - counters"); + runTest("Optimization of cflow - counters (1)"); + } + + public void test060_cflowOptimization_counters() { + runTest("Optimization of cflow - shared counters (2)"); + } + + public void test061_cflowOptimization_counters() { + runTest("Optimization of cflow - shared stacks (3)"); + } + + public void test062_cflowOptimization_counters() { + runTest("Optimization of cflow - counters (4)"); + } + + public void test063_cflowOptimization_countersWithAbstractPcuts() { + runTest("Optimization of cflow - counters with abstract pointcuts (5)"); } }
\ No newline at end of file diff --git a/tests/src/org/aspectj/systemtest/ajc121/ajc121-tests.xml b/tests/src/org/aspectj/systemtest/ajc121/ajc121-tests.xml index 2d443d55c..a2684804d 100644 --- a/tests/src/org/aspectj/systemtest/ajc121/ajc121-tests.xml +++ b/tests/src/org/aspectj/systemtest/ajc121/ajc121-tests.xml @@ -466,10 +466,27 @@ </compile> </ajc-test> - <ajc-test - dir="cflow" - pr="76030" - title="Optimization of cflow - counters"> + <ajc-test dir="cflow" pr="76030" title="Optimization of cflow - counters (1)"> <compile files="CounterTest01.java"/> <run class="CounterTest01"/> - </ajc-test>
\ No newline at end of file + </ajc-test> + + <ajc-test dir="cflow" pr="76030" title="Optimization of cflow - shared counters (2)"> + <compile files="CounterTest02.java"/> + <run class="CounterTest02"/> + </ajc-test> + + <ajc-test dir="cflow" pr="76030" title="Optimization of cflow - shared stacks (3)"> + <compile files="CounterTest03.java"/> + <run class="CounterTest03"/> + </ajc-test> + + <ajc-test dir="cflow" pr="76030" title="Optimization of cflow - counters (4)"> + <compile files="CounterTest04.java"/> + <run class="CounterTest04"/> + </ajc-test> + + <ajc-test dir="cflow" pr="76030" title="Optimization of cflow - counters with abstract pointcuts (5)"> + <compile files="CounterTest05.java"/> + <run class="CounterTest05"/> + </ajc-test> diff --git a/weaver/src/org/aspectj/weaver/bcel/BcelWeaver.java b/weaver/src/org/aspectj/weaver/bcel/BcelWeaver.java index daa113db2..3e497642d 100644 --- a/weaver/src/org/aspectj/weaver/bcel/BcelWeaver.java +++ b/weaver/src/org/aspectj/weaver/bcel/BcelWeaver.java @@ -61,6 +61,7 @@ import org.aspectj.weaver.TypeX; import org.aspectj.weaver.WeaverMessages; import org.aspectj.weaver.WeaverMetrics; import org.aspectj.weaver.WeaverStateInfo; +import org.aspectj.weaver.patterns.CflowPointcut; import org.aspectj.weaver.patterns.DeclareParents; import org.aspectj.weaver.patterns.FastMatchInfo; @@ -320,6 +321,7 @@ public class BcelWeaver implements IWeaver { public void prepareForWeave() { needToReweaveWorld = false; + CflowPointcut.clearCaches(); // update mungers for (Iterator i = addedClasses.iterator(); i.hasNext(); ) { diff --git a/weaver/src/org/aspectj/weaver/patterns/CflowPointcut.java b/weaver/src/org/aspectj/weaver/patterns/CflowPointcut.java index adbb5839d..422fda50e 100644 --- a/weaver/src/org/aspectj/weaver/patterns/CflowPointcut.java +++ b/weaver/src/org/aspectj/weaver/patterns/CflowPointcut.java @@ -19,6 +19,7 @@ import java.io.IOException; import java.lang.reflect.Modifier; import java.util.ArrayList; import java.util.Collection; +import java.util.Hashtable; import java.util.List; import org.aspectj.bridge.IMessage; @@ -41,10 +42,13 @@ import org.aspectj.weaver.ast.Test; public class CflowPointcut extends Pointcut { - private Pointcut entry; - private boolean isBelow; + private Pointcut entry; // The pointcut inside the cflow() that represents the 'entry' point + private boolean isBelow;// Is this cflowbelow? private int[] freeVars; + private static Hashtable cflowFields = new Hashtable(); + private static Hashtable cflowBelowFields = new Hashtable(); + /** * Used to indicate that we're in the context of a cflow when concretizing if's * @@ -56,6 +60,7 @@ public class CflowPointcut extends Pointcut { public CflowPointcut(Pointcut entry, boolean isBelow, int[] freeVars) { + // System.err.println("Building cflow pointcut "+entry.toString()); this.entry = entry; this.isBelow = isBelow; this.freeVars = freeVars; @@ -167,21 +172,47 @@ public class CflowPointcut extends Pointcut { List innerCflowEntries = new ArrayList(xcut.getCflowEntries()); innerCflowEntries.removeAll(previousCflowEntries); + + + Object field = getCflowfield(concreteEntry); + + // Four routes of interest through this code (did I hear someone say refactor??) + // 1) no state in the cflow - we can use a counter *and* we have seen this pointcut + // before - so use the same counter as before. + // 2) no state in the cflow - we can use a counter, but this is the first time + // we have seen this pointcut, so build the infrastructure. + // 3) state in the cflow - we need to use a stack *and* we have seen this pointcut + // before - so share the stack. + // 4) state in the cflow - we need to use a stack, but this is the first time + // we have seen this pointcut, so build the infrastructure. if (freeVars.length == 0) { // No state, so don't use a stack, use a counter. - ResolvedMember cflowField = new ResolvedMember(Member.FIELD,concreteAspect,Modifier.STATIC | Modifier.PUBLIC | Modifier.FINAL, + ResolvedMember localCflowField = null; + + // Check if we have already got a counter for this cflow pointcut + if (field != null) { + localCflowField = (ResolvedMember)field; // Use the one we already have + + } else { + + // Create a counter field in the aspect + localCflowField = new ResolvedMember(Member.FIELD,concreteAspect,Modifier.STATIC | Modifier.PUBLIC | Modifier.FINAL, NameMangler.cflowCounter(xcut),TypeX.forName(NameMangler.CFLOW_COUNTER_TYPE).getSignature()); - // Create type munger to add field to the aspect - concreteAspect.crosscuttingMembers.addTypeMunger(world.makeCflowCounterFieldAdder(cflowField)); + // Create type munger to add field to the aspect + concreteAspect.crosscuttingMembers.addTypeMunger(world.makeCflowCounterFieldAdder(localCflowField)); - // Create shadow munger to push stuff onto the stack - concreteAspect.crosscuttingMembers.addConcreteShadowMunger( - Advice.makeCflowEntry(world,concreteEntry,isBelow,cflowField,freeVars.length,innerCflowEntries,inAspect)); - return new ConcreteCflowPointcut(cflowField, null,true); + // Create shadow munger to push stuff onto the stack + concreteAspect.crosscuttingMembers.addConcreteShadowMunger( + Advice.makeCflowEntry(world,concreteEntry,isBelow,localCflowField,freeVars.length,innerCflowEntries,inAspect)); + + putCflowfield(concreteEntry,localCflowField); // Remember it + } + return new ConcreteCflowPointcut(localCflowField, null,true); } else { - + List slots = new ArrayList(); + for (int i=0, len=freeVars.length; i < len; i++) { int freeVar = freeVars[i]; @@ -197,25 +228,51 @@ public class CflowPointcut extends Pointcut { new ConcreteCflowPointcut.Slot(formalIndex, formalType, i); slots.add(slot); } - - ResolvedMember cflowField = new ResolvedMember( + ResolvedMember localCflowField = null; + if (field != null) { + localCflowField = (ResolvedMember)field; + } else { + + localCflowField = new ResolvedMember( Member.FIELD, concreteAspect, Modifier.STATIC | Modifier.PUBLIC | Modifier.FINAL, NameMangler.cflowStack(xcut), TypeX.forName(NameMangler.CFLOW_STACK_TYPE).getSignature()); - //System.out.println("adding field to: " + inAspect + " field " + cflowField); + //System.out.println("adding field to: " + inAspect + " field " + cflowField); - // add field and initializer to inAspect - //XXX and then that info above needs to be mapped down here to help with - //XXX getting the exposed state right - concreteAspect.crosscuttingMembers.addConcreteShadowMunger( - Advice.makeCflowEntry(world, concreteEntry, isBelow, cflowField, freeVars.length, innerCflowEntries,inAspect)); + // add field and initializer to inAspect + //XXX and then that info above needs to be mapped down here to help with + //XXX getting the exposed state right + concreteAspect.crosscuttingMembers.addConcreteShadowMunger( + Advice.makeCflowEntry(world, concreteEntry, isBelow, localCflowField, freeVars.length, innerCflowEntries,inAspect)); - concreteAspect.crosscuttingMembers.addTypeMunger( - world.makeCflowStackFieldAdder(cflowField)); - - return new ConcreteCflowPointcut(cflowField, slots,false); + concreteAspect.crosscuttingMembers.addTypeMunger( + world.makeCflowStackFieldAdder(localCflowField)); + putCflowfield(concreteEntry,localCflowField); + } + return new ConcreteCflowPointcut(localCflowField, slots,false); } } + + public static void clearCaches() { + cflowFields.clear(); + cflowBelowFields.clear(); + } + + private Object getCflowfield(Pointcut pcutkey) { + if (isBelow) { + return cflowBelowFields.get(pcutkey); + } else { + return cflowFields.get(pcutkey); + } + } + + private void putCflowfield(Pointcut pcutkey,Object o) { + if (isBelow) { + cflowBelowFields.put(pcutkey,o); + } else { + cflowFields.put(pcutkey,o); + } + } } |