76030 - cflow optimizations. Part 2 fix - share counters and stacks when we can.

7 files changed, 336 insertions, 9 deletions

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>