optimization: use tags not gens. improves memory (create less garbage), faster (dont need to transform the garbage): see pr128650, optimization #6.

9 files changed, 107 insertions, 175 deletions

diff --git a/weaver/src/org/aspectj/weaver/bcel/BcelClassWeaver.java b/weaver/src/org/aspectj/weaver/bcel/BcelClassWeaver.java
index d080ec18f..527e7369b 100644
--- a/weaver/src/org/aspectj/weaver/bcel/BcelClassWeaver.java
+++ b/weaver/src/org/aspectj/weaver/bcel/BcelClassWeaver.java
@@ -55,6 +55,7 @@ import org.aspectj.apache.bcel.generic.PUTSTATIC;
 import org.aspectj.apache.bcel.generic.RET;
 import org.aspectj.apache.bcel.generic.ReturnInstruction;
 import org.aspectj.apache.bcel.generic.Select;
+import org.aspectj.apache.bcel.generic.Tag;
 import org.aspectj.apache.bcel.generic.Type;
 import org.aspectj.apache.bcel.generic.annotation.AnnotationGen;
 import org.aspectj.bridge.IMessage;
@@ -769,7 +770,7 @@ class BcelClassWeaver implements IClassWeaver {
             			Annotation a = decaM.getAnnotationX().getBcelAnnotation();
             			AnnotationGen ag = new AnnotationGen(a,clazz.getConstantPoolGen(),true);
             			Method oldMethod = mg.getMethod();
-            			MethodGen myGen = new MethodGen(oldMethod,clazz.getClassName(),clazz.getConstantPoolGen());
+            			MethodGen myGen = new MethodGen(oldMethod,clazz.getClassName(),clazz.getConstantPoolGen(),false);// dont use tags, they won't get repaired like for woven methods.
             			myGen.addAnnotation(ag);
             			Method newMethod = myGen.getMethod();
             			mg.addAnnotation(decaM.getAnnotationX());
diff --git a/weaver/src/org/aspectj/weaver/bcel/BcelShadow.java b/weaver/src/org/aspectj/weaver/bcel/BcelShadow.java
index a5dd60910..b6b661154 100644
--- a/weaver/src/org/aspectj/weaver/bcel/BcelShadow.java
+++ b/weaver/src/org/aspectj/weaver/bcel/BcelShadow.java
@@ -43,6 +43,7 @@ import org.aspectj.apache.bcel.generic.InstructionList;
 import org.aspectj.apache.bcel.generic.InstructionTargeter;
 import org.aspectj.apache.bcel.generic.InvokeInstruction;
 import org.aspectj.apache.bcel.generic.LoadInstruction;
+import org.aspectj.apache.bcel.generic.LocalVariableTag;
 import org.aspectj.apache.bcel.generic.MULTIANEWARRAY;
 import org.aspectj.apache.bcel.generic.NEW;
 import org.aspectj.apache.bcel.generic.ObjectType;
@@ -1344,12 +1345,13 @@ public class BcelShadow extends Shadow {
      */
     public UnresolvedType ensureTargetTypeIsCorrect(UnresolvedType tx) {
     	
-    	
-    	if (tx.equals(ResolvedType.OBJECT) && getKind() == MethodCall && 
-    	    getSignature().getReturnType().equals(ResolvedType.OBJECT) && 
-			getSignature().getArity()==0 && 
-			getSignature().getName().charAt(0) == 'c' &&
-			getSignature().getName().equals("clone")) {
+    	Member msig = getSignature();
+    	if (msig.getArity()==0 && 
+    		getKind() == MethodCall && 
+        	msig.getName().charAt(0) == 'c' &&
+    		tx.equals(ResolvedType.OBJECT) && 
+    		msig.getReturnType().equals(ResolvedType.OBJECT) && 
+			msig.getName().equals("clone")) {
     		
     		// Lets go back through the code from the start of the shadow
             InstructionHandle searchPtr = range.getStart().getPrev();
@@ -1363,7 +1365,7 @@ public class BcelShadow extends Shadow {
             	LoadInstruction li = (LoadInstruction)searchPtr.getInstruction();
             	li.getIndex();
             	LocalVariableTag lvt = LazyMethodGen.getLocalVariableTag(searchPtr,li.getIndex());
-            	if (lvt!=null) 	return lvt.getType();
+            	if (lvt!=null) 	return UnresolvedType.forSignature(lvt.getType());
             }
             // A field access instruction may tell us the real type of what the clone() call is on
             if (searchPtr.getInstruction() instanceof FieldInstruction) {
diff --git a/weaver/src/org/aspectj/weaver/bcel/LazyMethodGen.java b/weaver/src/org/aspectj/weaver/bcel/LazyMethodGen.java
index 809d41225..043537de1 100644
--- a/weaver/src/org/aspectj/weaver/bcel/LazyMethodGen.java
+++ b/weaver/src/org/aspectj/weaver/bcel/LazyMethodGen.java
@@ -24,6 +24,7 @@ import java.util.LinkedList;
 import java.util.List;
 import java.util.ListIterator;
 import java.util.Map;
+import java.util.Properties;
 import java.util.Set;
 import java.util.Stack;
 
@@ -44,15 +45,19 @@ import org.aspectj.apache.bcel.generic.InstructionHandle;
 import org.aspectj.apache.bcel.generic.InstructionList;
 import org.aspectj.apache.bcel.generic.InstructionTargeter;
 import org.aspectj.apache.bcel.generic.LineNumberGen;
+import org.aspectj.apache.bcel.generic.LineNumberTag;
 import org.aspectj.apache.bcel.generic.LocalVariableGen;
 import org.aspectj.apache.bcel.generic.LocalVariableInstruction;
+import org.aspectj.apache.bcel.generic.LocalVariableTag;
 import org.aspectj.apache.bcel.generic.MethodGen;
 import org.aspectj.apache.bcel.generic.ObjectType;
 import org.aspectj.apache.bcel.generic.Select;
+import org.aspectj.apache.bcel.generic.Tag;
 import org.aspectj.apache.bcel.generic.Type;
 import org.aspectj.apache.bcel.generic.annotation.AnnotationGen;
 import org.aspectj.bridge.IMessage;
 import org.aspectj.bridge.ISourceLocation;
+import org.aspectj.bridge.MessageUtil;
 import org.aspectj.weaver.AjAttribute;
 import org.aspectj.weaver.AnnotationX;
 import org.aspectj.weaver.BCException;
@@ -94,6 +99,27 @@ public final class LazyMethodGen {
     private BcelMethod memberView;
     private AjAttribute.EffectiveSignatureAttribute effectiveSignature;
     int highestLineNumber = 0;
+    
+
+    /*
+     * This option specifies whether we let the BCEL classes create LineNumberGens and LocalVariableGens
+     * or if we make it create LineNumberTags and LocalVariableTags.  Up until 1.5.1 we always created
+     * Gens - then on return from the MethodGen ctor we took them apart, reprocessed them all and 
+     * created Tags. (see unpackLocals/unpackLineNumbers).  As we have our own copy of Bcel, why not create
+     * the right thing straightaway?  So setting this to true will call the MethodGen ctor() in such
+     * a way that it creates Tags - removing the need for unpackLocals/unpackLineNumbers - HOWEVER see
+     * the ensureAllLineNumberSetup() method for some other relevant info.
+     * 
+     * Whats the difference between a Tag and a Gen?  A Tag is more lightweight, it doesn't know which
+     * instructions it targets, it relies on the instructions targetting *it* - this reduces the amount
+     * of targeter manipulation we have to do.
+     * 
+     * Because this *could* go wrong - it passes all our tests, but you never know, the option:
+     * -Xset:optimizeWithTags=false
+     * will turn it *OFF*
+     */
+	public static boolean avoidUseOfBcelGenObjects = true;
+	public static boolean checkedXsetOption = false;
 
 	/** This is nonnull if this method is the result of an "inlining".  We currently
 	 * copy methods into other classes for around advice.  We add this field so
@@ -248,9 +274,20 @@ public final class LazyMethodGen {
     private void initialize() {
     	if (returnType != null) return; 
     	
+    	// Check whether we need to configure the optimization
+    	if (!checkedXsetOption) {
+        	Properties p = enclosingClass.getWorld().getExtraConfiguration();
+        	if (p!=null) {
+        		avoidUseOfBcelGenObjects = Boolean.parseBoolean(p.getProperty("optimizeWithTags","true"));
+        		if (!avoidUseOfBcelGenObjects) 
+        			enclosingClass.getWorld().getMessageHandler().handleMessage(MessageUtil.info("[optimizeWithTags=false] Disabling optimization to use Tags rather than Gens"));
+        	}
+        	checkedXsetOption=true;
+        }
+    	
     	//System.err.println("initializing: " + getName() + ", " + enclosingClass.getName() + ", " + returnType + ", " + savedMethod);
     	
-		MethodGen gen = new MethodGen(savedMethod, enclosingClass.getName(), enclosingClass.getConstantPoolGen());
+		MethodGen gen = new MethodGen(savedMethod, enclosingClass.getName(), enclosingClass.getConstantPoolGen(),avoidUseOfBcelGenObjects);
         
 		this.returnType = gen.getReturnType();
 		this.argumentTypes = gen.getArgumentTypes();
@@ -273,10 +310,16 @@ public final class LazyMethodGen {
         } else {
         	//body = new InstructionList(savedMethod.getCode().getCode());
             body = gen.getInstructionList();
-            
+
             unpackHandlers(gen);
-            unpackLineNumbers(gen);
-            unpackLocals(gen);
+            
+			if (avoidUseOfBcelGenObjects) {
+				ensureAllLineNumberSetup(gen);
+			    highestLineNumber = gen.getHighestlinenumber();
+			} else {
+				unpackLineNumbers(gen);
+			    unpackLocals(gen);
+			}
         }
         assertGoodBody();
         
@@ -363,6 +406,33 @@ public final class LazyMethodGen {
         gen.removeLineNumbers();
     }
 
+    /**
+     * On entry to this method we have a method whose instruction stream contains a few instructions 
+     * that have line numbers assigned to them (LineNumberTags).  The aim is to ensure every instruction
+     * has the right line number. This is necessary because some of them may be extracted out into other
+     * methods - and it'd be useful for them to maintain the source line number for debugging.
+     */
+    private void ensureAllLineNumberSetup(MethodGen gen) {
+        LineNumberTag lr = null;
+        boolean skip = false;
+        for (InstructionHandle ih = body.getStart(); ih != null; ih = ih.getNext()) {
+            InstructionTargeter[] targeters = ih.getTargeters();
+            skip = false;
+            if (targeters != null) {
+                for (int i = targeters.length - 1; i >= 0; i--) {
+                    InstructionTargeter targeter = targeters[i];
+                    if (targeter instanceof LineNumberTag) {
+                        lr = (LineNumberTag) targeter;
+                        skip=true;
+                    }
+                }
+            }
+            if (lr != null && !skip) {
+                ih.addTargeter(lr);
+            }
+        }
+    }
+
     private void unpackLocals(MethodGen gen) {
         Set locals = new HashSet();
         for (InstructionHandle ih = body.getStart(); ih != null; ih = ih.getNext()) {
@@ -373,7 +443,7 @@ public final class LazyMethodGen {
                     InstructionTargeter targeter = targeters[i];
                     if (targeter instanceof LocalVariableGen) {
                         LocalVariableGen lng = (LocalVariableGen) targeter;
-                        LocalVariableTag lr = new LocalVariableTag(BcelWorld.fromBcel(lng.getType()), lng.getName(), lng.getIndex(), lng.getStart().getPosition());
+                        LocalVariableTag lr = new LocalVariableTag(lng.getType().getSignature()/*BcelWorld.fromBcel(lng.getType())*/, lng.getName(), lng.getIndex(), lng.getStart().getPosition());
                         if (lng.getStart() == ih) {
                             locals.add(lr);
                         } else {
@@ -1006,9 +1076,13 @@ public final class LazyMethodGen {
 	       		continue;
 	       	}
 	       	slots.add(new Integer(tag.getSlot()));
-
-            gen.addLocalVariable(tag.getName(), 
-                BcelWorld.makeBcelType(tag.getType()),
+	       	Type t = tag.getRealType();
+	       	if (t==null) {
+	       		t = BcelWorld.makeBcelType(UnresolvedType.forSignature(tag.getType()));
+	       	}
+            gen.addLocalVariable(
+            		tag.getName(),
+            		t,
                 tag.getSlot(),(InstructionHandle) start,(InstructionHandle) lvpos.end);
         }
 	}
diff --git a/weaver/src/org/aspectj/weaver/bcel/LineNumberTag.java b/weaver/src/org/aspectj/weaver/bcel/LineNumberTag.java
deleted file mode 100644
index 9dc5b8a15..000000000
--- a/weaver/src/org/aspectj/weaver/bcel/LineNumberTag.java
+++ /dev/null
@@ -1,39 +0,0 @@
-/* *******************************************************************
- * Copyright (c) 2002 Palo Alto Research Center, Incorporated (PARC).
- * All rights reserved. 
- * This program and the accompanying materials are made available 
- * under the terms of the Common Public License v1.0 
- * which accompanies this distribution and is available at 
- * http://www.eclipse.org/legal/cpl-v10.html 
- *  
- * Contributors: 
- *     PARC     initial implementation 
- * ******************************************************************/
-
-
-package org.aspectj.weaver.bcel;
-
-/** we don't actually target instructions, but instructions target us. */
-public class LineNumberTag extends Tag {
-
-    private final int lineNumber;
-
-    public LineNumberTag(int lineNumber) {
-        this.lineNumber = lineNumber;
-    }
-      
-    public int getLineNumber() { return lineNumber; }
-    
-    // ---- from Object
-
-    public String toString() {
-        return "line " + lineNumber;
-    }  
-    public boolean equals(Object other) {
-        if (! (other instanceof LineNumberTag)) return false;
-        return lineNumber == ((LineNumberTag)other).lineNumber;
-    }
-    public int hashCode() {
-        return lineNumber;
-    }
-}
diff --git a/weaver/src/org/aspectj/weaver/bcel/LocalVariableTag.java b/weaver/src/org/aspectj/weaver/bcel/LocalVariableTag.java
deleted file mode 100644
index 14bb30acf..000000000
--- a/weaver/src/org/aspectj/weaver/bcel/LocalVariableTag.java
+++ /dev/null
@@ -1,73 +0,0 @@
-/* *******************************************************************
- * Copyright (c) 2002 Palo Alto Research Center, Incorporated (PARC).
- * All rights reserved. 
- * This program and the accompanying materials are made available 
- * under the terms of the Common Public License v1.0 
- * which accompanies this distribution and is available at 
- * http://www.eclipse.org/legal/cpl-v10.html 
- *  
- * Contributors: 
- *     PARC     initial implementation 
- * ******************************************************************/
-
-
-package org.aspectj.weaver.bcel;
-
-import org.aspectj.weaver.UnresolvedType;
-
-public final class LocalVariableTag extends Tag {
-    private final UnresolvedType type;
-    private final String name;
-    private int slot;
-    private final int startPos;
-    boolean remapped = false;
-
-    // AMC - pr101047, two local vars with the same name can share the same slot, but must in that case
-    // have different start positions.
-    public LocalVariableTag(UnresolvedType type, String name, int slot, int startPosition) {
-        this.type = type;
-        this.name = name;
-        this.slot = slot;
-        this.startPos = startPosition;
-    }
-
-    public String getName() {
-        return name;
-    }
-    public int getSlot() {
-        return slot;
-    }
-    public UnresolvedType getType() {
-        return type;
-    }
-    
-    public void updateSlot(int newSlot) {
-    	this.slot = newSlot;
-    	this.remapped = true;
-    }
-    
-    public boolean isRemapped() { return this.remapped; }
-    
-    // ---- from Object
-    
-    public String toString() {
-        return "local " + slot + ": " + type + " " + name;
-    }
-    public boolean equals(Object other) {
-        if (!(other instanceof LocalVariableTag)) return false;
-        LocalVariableTag o = (LocalVariableTag)other;
-        return o.type.equals(type) && o.name.equals(name) && o.slot == slot && o.startPos == startPos;
-    }
-    private volatile int hashCode = 0;
-    public int hashCode() {
-        if (hashCode == 0) {
-            int ret = 17;
-            ret = 37*ret + type.hashCode();
-            ret = 37*ret + name.hashCode();
-            ret = 37*ret + slot;
-            ret = 37*ret + startPos;
-            hashCode = ret;
-        }
-        return hashCode;
-    }
-}
diff --git a/weaver/src/org/aspectj/weaver/bcel/Range.java b/weaver/src/org/aspectj/weaver/bcel/Range.java
index 60702912b..f32335dd4 100644
--- a/weaver/src/org/aspectj/weaver/bcel/Range.java
+++ b/weaver/src/org/aspectj/weaver/bcel/Range.java
@@ -197,6 +197,7 @@ abstract class Range implements InstructionTargeter {
         // assert isRangeHandle(ih)
 		Range ret = null;
         InstructionTargeter[] targeters = ih.getTargeters();
+        if (targeters!=null) {
         for (int i = targeters.length - 1; i >= 0; i--) {
             if (targeters[i] instanceof Range) {
             	Range r = (Range) targeters[i];
@@ -205,7 +206,10 @@ abstract class Range implements InstructionTargeter {
             	ret = (Range) targeters[i];
             }
         }
-		if (ret == null) throw new BCException("shouldn't happen");
+        }
+		if (ret == null) {
+			throw new BCException("shouldn't happen");
+		}
 		return ret;
     }
 
diff --git a/weaver/src/org/aspectj/weaver/bcel/ShadowRange.java b/weaver/src/org/aspectj/weaver/bcel/ShadowRange.java
index 7632c3e16..cf46cbf68 100644
--- a/weaver/src/org/aspectj/weaver/bcel/ShadowRange.java
+++ b/weaver/src/org/aspectj/weaver/bcel/ShadowRange.java
@@ -21,6 +21,7 @@ import org.aspectj.apache.bcel.generic.InstructionHandle;
 import org.aspectj.apache.bcel.generic.InstructionList;
 import org.aspectj.apache.bcel.generic.InstructionTargeter;
 import org.aspectj.apache.bcel.generic.LocalVariableInstruction;
+import org.aspectj.apache.bcel.generic.LocalVariableTag;
 import org.aspectj.apache.bcel.generic.RET;
 import org.aspectj.apache.bcel.generic.Select;
 import org.aspectj.apache.bcel.generic.TargetLostException;
diff --git a/weaver/src/org/aspectj/weaver/bcel/Tag.java b/weaver/src/org/aspectj/weaver/bcel/Tag.java
deleted file mode 100644
index 427fc3f94..000000000
--- a/weaver/src/org/aspectj/weaver/bcel/Tag.java
+++ /dev/null
@@ -1,43 +0,0 @@
-/* *******************************************************************
- * Copyright (c) 2002 Palo Alto Research Center, Incorporated (PARC).
- * All rights reserved. 
- * This program and the accompanying materials are made available 
- * under the terms of the Common Public License v1.0 
- * which accompanies this distribution and is available at 
- * http://www.eclipse.org/legal/cpl-v10.html 
- *  
- * Contributors: 
- *     PARC     initial implementation 
- * ******************************************************************/
-
-
-package org.aspectj.weaver.bcel;
-
-import org.aspectj.apache.bcel.generic.InstructionHandle;
-import org.aspectj.apache.bcel.generic.InstructionTargeter;
-
-/** A tag is an instruction-targeter that doesn't bother remembering its target(s) */
-abstract class Tag implements InstructionTargeter, Cloneable {
-
-    public Tag() {
-    }
-
-    // ---- from InstructionTargeter
-    public boolean containsTarget(InstructionHandle ih) {
-        return false;
-    }
-
-    public void updateTarget(InstructionHandle old_ih, InstructionHandle new_ih) {
-        old_ih.removeTargeter(this);
-        if (new_ih != null)
-            new_ih.addTargeter(this);
-    }
-    
-    public Tag copy() {
-    	try {
-    		return (Tag)clone();
-    	} catch (CloneNotSupportedException e) {
-    		throw new RuntimeException("Sanity check, can't clone me");
-    	}
-    }
-}
diff --git a/weaver/src/org/aspectj/weaver/bcel/Utility.java b/weaver/src/org/aspectj/weaver/bcel/Utility.java
index 7eec5ef41..79571fcf7 100644
--- a/weaver/src/org/aspectj/weaver/bcel/Utility.java
+++ b/weaver/src/org/aspectj/weaver/bcel/Utility.java
@@ -44,6 +44,7 @@ import org.aspectj.apache.bcel.generic.InstructionHandle;
 import org.aspectj.apache.bcel.generic.InstructionList;
 import org.aspectj.apache.bcel.generic.InstructionTargeter;
 import org.aspectj.apache.bcel.generic.LDC;
+import org.aspectj.apache.bcel.generic.LineNumberTag;
 import org.aspectj.apache.bcel.generic.ObjectType;
 import org.aspectj.apache.bcel.generic.ReferenceType;
 import org.aspectj.apache.bcel.generic.SIPUSH;
@@ -655,9 +656,8 @@ public class Utility {
 //		}
 //		return getSourceLine(ih.getNext());
 //	}
-	
 
-	public static int getSourceLine(InstructionHandle ih) {
+	public static int getSourceLine(InstructionHandle ih) {//,boolean goforwards) {
 		int lookahead=0;
 		// arbitrary rule that we will never lookahead more than 100 instructions for a line #
 		while (lookahead++ < 100) {
@@ -672,11 +672,16 @@ public class Utility {
 	                }
 	            }
 	        }
-	        ih = ih.getNext();
+//	        if (goforwards) ih=ih.getNext(); else 
+	        	ih=ih.getPrev();
 		}
 		//System.err.println("no line information available for: " + ih);
         return -1;
 	}
+
+//	public static int getSourceLine(InstructionHandle ih) {
+//		return getSourceLine(ih,false);
+//	}
 	
 	// assumes that there is no already extant source line tag.  Otherwise we'll have to be better.
 	public static void setSourceLine(InstructionHandle ih, int lineNumber) {