231396: refactoring AspectJ: optimized packing change integrated, can be turned off

2 files changed, 173 insertions, 3 deletions

diff --git a/weaver/src/org/aspectj/weaver/World.java b/weaver/src/org/aspectj/weaver/World.java
index db895576a..7bf7b6f8b 100644
--- a/weaver/src/org/aspectj/weaver/World.java
+++ b/weaver/src/org/aspectj/weaver/World.java
@@ -117,6 +117,7 @@ public abstract class World implements Dump.INode {
 	private boolean runMinimalMemory = false;
 	private boolean shouldPipelineCompilation = true;
 	protected boolean bcelRepositoryCaching = xsetBCEL_REPOSITORY_CACHING_DEFAULT.equalsIgnoreCase("true");
+	private boolean fastMethodPacking = false; 
 	private boolean completeBinaryTypes = false;
 	public boolean forDEBUG_structuralChangesCode = false;
 	public boolean forDEBUG_bridgingCode = false;
@@ -810,6 +811,7 @@ public abstract class World implements Dump.INode {
 	public final static String xsetCOMPLETE_BINARY_TYPES = "completeBinaryTypes";
 	public final static String xsetCOMPLETE_BINARY_TYPES_DEFAULT = "false"; 
 	public final static String xsetBCEL_REPOSITORY_CACHING_DEFAULT = "true"; 
+	public final static String xsetFAST_PACK_METHODS = "fastPackMethods";  // default TRUE
 	
 	public boolean isInJava5Mode() {
 		return behaveInJava5Way;
@@ -1218,6 +1220,9 @@ public abstract class World implements Dump.INode {
 				if (!bcelRepositoryCaching) {
 					getMessageHandler().handleMessage(MessageUtil.info("[bcelRepositoryCaching=false] AspectJ will not use a bcel cache for class information"));
 				}
+
+				s = p.getProperty(xsetFAST_PACK_METHODS,"true");
+				fastMethodPacking = s.equalsIgnoreCase("true");
 				
 				s = p.getProperty(xsetPIPELINE_COMPILATION,xsetPIPELINE_COMPILATION_DEFAULT);
 				shouldPipelineCompilation = s.equalsIgnoreCase("true");
@@ -1249,6 +1254,11 @@ public abstract class World implements Dump.INode {
 	      ensureAdvancedConfigurationProcessed();
 	    	  return runMinimalMemory;
 	    }
+
+    public boolean shouldFastPackMethods() {
+        ensureAdvancedConfigurationProcessed();
+        return fastMethodPacking;
+    }
 	    
 	    public boolean shouldPipelineCompilation() {
 	    	ensureAdvancedConfigurationProcessed();
diff --git a/weaver/src/org/aspectj/weaver/bcel/LazyMethodGen.java b/weaver/src/org/aspectj/weaver/bcel/LazyMethodGen.java
index e22781934..5d3998858 100644
--- a/weaver/src/org/aspectj/weaver/bcel/LazyMethodGen.java
+++ b/weaver/src/org/aspectj/weaver/bcel/LazyMethodGen.java
@@ -47,6 +47,7 @@ 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.Tag;
+import org.aspectj.apache.bcel.generic.TargetLostException;
 import org.aspectj.apache.bcel.generic.Type;
 import org.aspectj.bridge.IMessage;
 import org.aspectj.bridge.ISourceLocation;
@@ -96,8 +97,8 @@ public final class LazyMethodGen implements Traceable {
     private BcelMethod memberView;
     private AjAttribute.EffectiveSignatureAttribute effectiveSignature;
     int highestLineNumber = 0;
+    boolean wasPackedOptimally = false;
     
-
     /*
      * We use LineNumberTags and not Gens.
      * 
@@ -433,7 +434,8 @@ public final class LazyMethodGen implements Traceable {
     	
     	try {
 			MethodGen gen = pack();
-			return gen.getMethod();
+			savedMethod = gen.getMethod();
+			return savedMethod;
     	} catch (ClassGenException e) {
     		enclosingClass.getBcelObjectType().getResolvedTypeX().getWorld().showMessage(
     			IMessage.ERROR, 
@@ -450,6 +452,9 @@ public final class LazyMethodGen implements Traceable {
     }
     
     public void markAsChanged() {
+    	if (wasPackedOptimally) {
+    		throw new RuntimeException("Already packed method is being re-modified: "+getClassName()+" "+toShortString());
+    	}
     	initialize();
     	savedMethod = null;
     }
@@ -933,7 +938,15 @@ public final class LazyMethodGen implements Traceable {
         }
         
         if (hasBody()) {
-            packBody(gen);
+        	if (this.enclosingClass.getWorld().shouldFastPackMethods()) {
+	            if (isAdviceMethod() || getName().equals("<clinit>")) {
+	        		packBody(gen);
+	        	} else {
+	        		optimizedPackBody(gen); 
+	        	}
+        	} else {
+        		packBody(gen);
+        	}
             gen.setMaxLocals();
             gen.setMaxStack();
         } else {
@@ -1046,6 +1059,108 @@ public final class LazyMethodGen implements Traceable {
         }
     }
     
+
+    /*
+     * Optimized packing that does a 'local packing' of the code rather than building a brand new method
+     * and packing into it.  Only usable when the packing is going to be done just once.
+     */
+    public void optimizedPackBody(MethodGen gen) {
+    	InstructionList theBody = getBody();
+        InstructionHandle iHandle = theBody.getStart();
+
+        int currLine = -1;
+		int lineNumberOffset = (fromFilename == null) ? 0: getEnclosingClass().getSourceDebugExtensionOffset(fromFilename);
+        Map localVariables = new HashMap();
+        LinkedList exceptionList = new LinkedList();   
+        Set forDeletion = new HashSet();
+        Set branchInstructions = new HashSet();
+        // OPTIMIZE sort out in here: getRange()/insertHandler() and type of exceptionList
+        while (iHandle != null) {
+        	Instruction inst = iHandle.getInstruction();
+        	InstructionHandle nextInst = iHandle.getNext();
+        	// OPTIMIZE remove this instructionhandle as it now points to nowhere?
+        	if (inst == Range.RANGEINSTRUCTION) {
+        		Range r = Range.getRange(iHandle);
+	    		if (r instanceof ExceptionRange) {
+	    		    ExceptionRange er = (ExceptionRange) r;
+	    		    if (er.getStart() == iHandle) {
+	    		    	if (!er.isEmpty()){
+	    		        	// order is important, insert handlers in order of start
+	    		        	insertHandler(er, exceptionList);	    	      
+	    		    	}
+	    		    }
+	    		} 
+    			forDeletion.add(iHandle);
+    		} else {
+    			if (inst instanceof InstructionBranch) {
+    				branchInstructions.add(iHandle);
+    			}
+
+              InstructionTargeter[] targeters = iHandle.getTargeters();
+              if (targeters != null) {
+                  for (int k = targeters.length - 1; k >= 0; k--) {
+                      InstructionTargeter targeter = targeters[k];
+                      if (targeter instanceof LineNumberTag) {
+                          int line = ((LineNumberTag)targeter).getLineNumber();
+                          if (line != currLine) {
+                              gen.addLineNumber(iHandle, line + lineNumberOffset);
+                              currLine = line;
+                          }
+                      } else if (targeter instanceof LocalVariableTag) {
+                          LocalVariableTag lvt = (LocalVariableTag) targeter;
+                          LVPosition p = (LVPosition)localVariables.get(lvt);
+                          // If we don't know about it, create a new position and store
+                          // If we do know about it - update its end position
+                          if (p==null) {
+                          	LVPosition newp = new LVPosition();
+                          	newp.start=newp.end=iHandle;
+                          	localVariables.put(lvt,newp);
+                          } else {
+                          	p.end = iHandle;
+                          }
+                      }
+                  }
+              }
+    		}
+        	iHandle = iHandle.getNext();
+        }
+        for (Iterator iterator = branchInstructions.iterator(); iterator.hasNext();) {
+			BranchHandle iBranch = (BranchHandle) iterator.next();
+			handleBranchInstruction(iBranch,forDeletion);
+		}
+    	// now add exception handlers
+        for (Iterator iter = exceptionList.iterator(); iter.hasNext();) {
+            ExceptionRange r = (ExceptionRange) iter.next();
+            if (r.isEmpty()) continue;
+            gen.addExceptionHandler(
+                jumpForward(r.getRealStart(),forDeletion), 
+                jumpForward(r.getRealEnd(),forDeletion),
+                jumpForward(r.getHandler(),forDeletion),
+                (r.getCatchType() == null)
+                ? null 
+                : (ObjectType) BcelWorld.makeBcelType(r.getCatchType()));
+        }
+        
+        for (Iterator iterator = forDeletion.iterator(); iterator.hasNext();) {
+        	try {
+				theBody.delete((InstructionHandle)iterator.next());
+			} catch (TargetLostException e) {
+				e.printStackTrace();
+			}
+        }
+        gen.setInstructionList(theBody);
+        addLocalVariables(gen,localVariables);
+        
+        // JAVAC adds line number tables (with just one entry) to generated accessor methods - this
+        // keeps some tools that rely on finding at least some form of linenumbertable happy.
+        // Let's check if we have one - if we don't then let's add one.
+        // TODO Could be made conditional on whether line debug info is being produced
+        if (gen.getLineNumbers().length==0) { 
+        	gen.addLineNumber(gen.getInstructionList().getStart(),1);
+        }
+        wasPackedOptimally = true;
+    }
+    
     private void addLocalVariables(MethodGen gen, Map localVariables) {
 		// now add local variables
         gen.removeLocalVariables();
@@ -1118,6 +1233,44 @@ public final class LazyMethodGen implements Traceable {
 	        }
 	    }
 	}
+	
+	private InstructionHandle jumpForward(InstructionHandle t,Set handlesForDeletion) {
+		
+		InstructionHandle target = t;
+		  if (handlesForDeletion.contains(target)) {
+		    	do {
+		    		target = target.getNext();
+		    	} while (handlesForDeletion.contains(target));
+		    }
+		  return target;
+	}
+	
+	private void handleBranchInstruction(BranchHandle branchHandle, Set handlesForDeletion) {
+		InstructionBranch branchInstruction = (InstructionBranch) branchHandle.getInstruction();
+	    InstructionHandle target = branchInstruction.getTarget(); // old target
+   
+	    if (handlesForDeletion.contains(target)) {
+	    	do {
+	    		target = target.getNext();
+	    	} while (handlesForDeletion.contains(target));
+	    	branchInstruction.setTarget(target);
+	    }
+   
+	    if (branchInstruction instanceof InstructionSelect) { 
+	        // Either LOOKUPSWITCH or TABLESWITCH
+	        InstructionHandle[] targets = ((InstructionSelect)branchInstruction).getTargets();
+	        for (int k = targets.length - 1; k >= 0; k--) { 
+	        	InstructionHandle oneTarget = targets[k];
+	        	if (handlesForDeletion.contains(oneTarget)) {
+	    	    	do {
+	    	    		oneTarget = oneTarget.getNext();
+	    	    	} while (handlesForDeletion.contains(oneTarget));
+	    	    	branchInstruction.setTarget(oneTarget);
+	    	    	oneTarget.addTargeter(branchInstruction);
+	    	    }
+	        }
+	    }
+	}
 
 	private void handleRangeInstruction(InstructionHandle ih, LinkedList exnList) {
 		// we're a range instruction
@@ -1181,6 +1334,13 @@ public final class LazyMethodGen implements Traceable {
 //			curr = next;
 //    	}
 //	}
+   
+   private static InstructionHandle fNext(InstructionHandle ih) {
+	   while (true) {
+		   if (ih.getInstruction()==Range.RANGEINSTRUCTION) ih = ih.getNext();
+		   else return ih;
+	   }
+   }
 
     private static InstructionHandle remap(InstructionHandle ih, Map map) {
         while (true) {