You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@commons.apache.org by ko...@apache.org on 2005/09/09 08:24:00 UTC

svn commit: r279714 - in /jakarta/commons/sandbox/javaflow/trunk/src: java/org/apache/commons/javaflow/bytecode/transformation/bcel/ test/org/apache/commons/javaflow/

Author: kohsuke
Date: Thu Sep  8 23:23:52 2005
New Revision: 279714

URL: http://svn.apache.org/viewcvs?rev=279714&view=rev
Log:
changed the way object allocations are instrumented. we now use local variables to store arguments temporarily, to improve the performance and so that we won't require StackRecorder even if the code is running outside the continuation environment. Also added a test case for that.

Modified:
    jakarta/commons/sandbox/javaflow/trunk/src/java/org/apache/commons/javaflow/bytecode/transformation/bcel/BcelClassTransformer.java
    jakarta/commons/sandbox/javaflow/trunk/src/test/org/apache/commons/javaflow/ContinuationTestCase.java
    jakarta/commons/sandbox/javaflow/trunk/src/test/org/apache/commons/javaflow/ContinuationTests.java

Modified: jakarta/commons/sandbox/javaflow/trunk/src/java/org/apache/commons/javaflow/bytecode/transformation/bcel/BcelClassTransformer.java
URL: http://svn.apache.org/viewcvs/jakarta/commons/sandbox/javaflow/trunk/src/java/org/apache/commons/javaflow/bytecode/transformation/bcel/BcelClassTransformer.java?rev=279714&r1=279713&r2=279714&view=diff
==============================================================================
--- jakarta/commons/sandbox/javaflow/trunk/src/java/org/apache/commons/javaflow/bytecode/transformation/bcel/BcelClassTransformer.java (original)
+++ jakarta/commons/sandbox/javaflow/trunk/src/java/org/apache/commons/javaflow/bytecode/transformation/bcel/BcelClassTransformer.java Thu Sep  8 23:23:52 2005
@@ -15,12 +15,6 @@
  */
 package org.apache.commons.javaflow.bytecode.transformation.bcel;
 
-import java.io.ByteArrayInputStream;
-import java.io.FileOutputStream;
-import java.io.IOException;
-import java.io.InputStream;
-import java.util.ArrayList;
-import java.util.Vector;
 import org.apache.bcel.Constants;
 import org.apache.bcel.Repository;
 import org.apache.bcel.classfile.ClassFormatException;
@@ -59,7 +53,6 @@
 import org.apache.bcel.generic.TABLESWITCH;
 import org.apache.bcel.generic.TargetLostException;
 import org.apache.bcel.generic.Type;
-import org.apache.bcel.generic.IFNONNULL;
 import org.apache.bcel.verifier.exc.AssertionViolatedException;
 import org.apache.commons.javaflow.bytecode.Continuable;
 import org.apache.commons.javaflow.bytecode.StackRecorder;
@@ -75,10 +68,19 @@
 import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
 
+import java.io.ByteArrayInputStream;
+import java.io.FileOutputStream;
+import java.io.IOException;
+import java.io.InputStream;
+import java.util.ArrayList;
+import java.util.Vector;
+
 
 /**
+ * {@link ClassTransformer} that uses BCEL.
+ *
  * @author tcurdt
- *  
+ * @author Kohsuke Kawaguchi
  */
 public final class BcelClassTransformer implements ClassTransformer {
 
@@ -339,7 +341,12 @@
         InstructionHandle ins = insList.getStart();
         final InstructionList restorer = new InstructionList();
         int count = 0;  // count # of breakpoints
-        
+
+        // we need to expand the local variable size to store arguments for the constructor invocation.
+        // compute the necessary size in this variable.
+        int[] localVarsSize = new int[1];
+        localVarsSize[0] = method.getMaxLocals();
+
         while (ins != null) {
             InstructionHandle next = ins.getNext();
 
@@ -401,7 +408,7 @@
                     final Type type = os.peek(arguments.length);
                     if (type instanceof UninitializedObjectType) {
                         final ObjectType objecttype = ((UninitializedObjectType) type).getInitialized();
-                        final InstructionList duplicator = duplicateStack(method, invoke, objecttype);
+                        final InstructionList duplicator = duplicateStack(method, invoke, objecttype, localVarsSize);
                         final InstructionTargeter[] targeter = ins.getTargeters();
 
                         if (targeter != null) {
@@ -450,77 +457,48 @@
             insList.insert(InstructionConstants.DUP);
             insList.insert(insFactory.createInvoke(STACK_RECORDER_CLASS, STACK_METHOD, STACK_RECORDER_TYPE, Type.NO_ARGS, Constants.INVOKESTATIC));
 
+            // we need one more variable for storing StackRecorder
+            localVarsSize[0] = Math.max( localVarsSize[0], method.getMaxLocals()+1 );
+
             // make room for additional object (StackRecorder)
-            method.setMaxLocals(method.getMaxLocals() + 1);
             method.setMaxStack(method.getMaxStack() + 2);
         }
+
+        method.setMaxLocals(localVarsSize[0]);
     }
 
-    private InstructionList duplicateStack(MethodGen method, InvokeInstruction invoke, ObjectType objecttype) {
+    private InstructionList duplicateStack(MethodGen method, InvokeInstruction invoke, ObjectType objecttype, int[] localVarsSize) {
         // reconstruction of an uninitialed object to call the constructor.
         final InstructionFactory insFactory = new InstructionFactory(method.getConstantPool());
         final InstructionList insList = new InstructionList();
 
         final Type[] arguments = invoke.getArgumentTypes(method.getConstantPool());
-        Instruction loadStackRecorder = InstructionFactory.createLoad(STACK_RECORDER_TYPE, method.getMaxLocals());
 
-        {
-            // make sure that the stack recorder exists, if not create a temporary one.
-            // this is rather wasteful when executing the instrumented bytecode outside
-            // the continuation environment (because it creates a dummy object just to save/load few
-            // variables.), but at least it lets the code run.
-            //
-            // the alternative is to store arguments in local variables. If you do so,
-            // make sure to erase them to null when you finish restoring them onto the stack,
-            // or otherwise they'll be left reachable.
-            IFNONNULL x;
-            insList.append(loadStackRecorder);
-            insList.append(x=new IFNONNULL(null));
-            insList.append(insFactory.createNew(STACK_RECORDER_TYPE));
-            insList.append(InstructionFactory.DUP);
-            insList.append(insFactory.createInvoke(STACK_RECORDER_CLASS,"<init>",Type.VOID,new Type[0],Constants.INVOKESPECIAL));
-            insList.append(InstructionFactory.createStore(STACK_RECORDER_TYPE, method.getMaxLocals()));
-            x.setTarget(
-                insList.append(InstructionFactory.NOP));
-        }
+        int localVarOffset = method.getMaxLocals()+1;   // +1 for stack recorder
 
-        // pop all arguments for the constructor from the stack
+        // move all arguments for the constructor from the operand stack into local variables
         for (int i = arguments.length - 1; i >= 0; i--) {
             Type type = arguments[i];
-            insList.append(loadStackRecorder);
-            if(type.getSize()==1)
-                insList.append(InstructionFactory.SWAP);
-            else {
-                // TODO: is there more efficient way to do this?
-                insList.append(InstructionFactory.DUP_X2);
-                insList.append(InstructionFactory.POP);
-            }
-            if (type instanceof BasicType) {
-                if (type.getSize() < 2 && !type.equals(Type.FLOAT)) {
-                    type = Type.INT;
-                }
-                insList.append(insFactory.createInvoke(STACK_RECORDER_CLASS, getPushMethod(type), Type.VOID, new Type[] { type }, Constants.INVOKEVIRTUAL));
-            } else if (type instanceof ReferenceType) {
-                insList.append(insFactory.createInvoke(STACK_RECORDER_CLASS, getPushMethod(Type.OBJECT), Type.VOID, new Type[] { Type.OBJECT }, Constants.INVOKEVIRTUAL));
-            }
+            insList.append(InstructionFactory.createStore(type,localVarOffset));
+            localVarOffset += type.getSize();
         }
+
+        localVarsSize[0] = Math.max(localVarsSize[0],localVarOffset);
+
         // create uninitialzed object
         insList.append(insFactory.createNew(objecttype));
-        insList.append(InstructionFactory.createDup(objecttype.getSize()));
-        // return the arguments into the stack
+        insList.append(InstructionConstants.DUP);
+
+        // move the arguments back into the operand stack
         for (int i = 0; i < arguments.length; i++) {
             Type type = arguments[i];
-            insList.append(loadStackRecorder);
-            if (type instanceof BasicType) {
-                if (type.getSize() < 2 && !type.equals(Type.FLOAT)) {
-                    type = Type.INT;
-                }
-                insList.append(insFactory.createInvoke(STACK_RECORDER_CLASS, getPopMethod(type), type, Type.NO_ARGS, Constants.INVOKEVIRTUAL));
-            } else if (type instanceof ReferenceType) {
-                insList.append(insFactory.createInvoke(STACK_RECORDER_CLASS, getPopMethod(Type.OBJECT), Type.OBJECT, Type.NO_ARGS, Constants.INVOKEVIRTUAL));
-                if (!type.equals(Type.OBJECT)) {
-                    insList.append(insFactory.createCast(Type.OBJECT, type));
-                }
+
+            localVarOffset -= type.getSize();
+            insList.append(InstructionFactory.createLoad(type,localVarOffset));
+            if(type instanceof ReferenceType) {
+                // erase the local variable, or else we have a potential for memory leak
+                insList.append(InstructionConstants.ACONST_NULL);
+                insList.append(InstructionFactory.createStore(type,localVarOffset));
             }
         }
         return insList;
@@ -538,12 +516,12 @@
 
         if (opcode == Constants.INVOKEVIRTUAL || opcode == Constants.INVOKESTATIC || opcode == Constants.INVOKEINTERFACE || invokeSpecialSuper) {
 
-            final int index = ((InvokeInstruction) handle.getInstruction()).getIndex();
-            final String classname = getObjectType(method.getConstantPool().getConstantPool(), index).getClassName();
+            // final int index = ((InvokeInstruction) handle.getInstruction()).getIndex();
+            // final String classname = getObjectType(method.getConstantPool().getConstantPool(), index).getClassName();
 
             // rewrite invocation if object is continuable or a continuation
             // object
-            
+
             // FIXME:
             //return Repository.implementationOf(classname, CONTINUABLE_CLASS) || Repository.instanceOf(classname, CONTINUATION_CLASS);
             return true;
@@ -577,7 +555,7 @@
                 if (type.getSize() < 2 && !type.equals(Type.FLOAT)) {
                     type = Type.INT;
                 }
-                
+
                 // check for types with two words on stack
                 if (type.equals(Type.LONG) || type.equals(Type.DOUBLE)) {
                     insList.append(new ACONST_NULL()); // create dummy stack entry
@@ -605,22 +583,22 @@
         // add isCapturing test
         if (debug) {
             insList.insert(insFactory.createPrintln("capturing invocation "+method));
-        }        
+        }
         insList.insert(new IFEQ(handle.getNext()));
-        
+
         // test if the continuation should be captured after the invocation
         insList.insert(insFactory.createInvoke(STACK_RECORDER_CLASS, CAPURING_METHOD, Type.BOOLEAN, Type.NO_ARGS, Constants.INVOKEVIRTUAL));
         insList.insert(loadStackRecorder);
-        
+
         // test if continuation exists
         insList.insert(new IFNULL(handle.getNext()));
         insList.insert(loadStackRecorder);
 
-        // save local variables        
+        // save local variables
         //if (debug) {
         //    insList.append(insFactory.createPrintln("save local variables"));
         //}
-        
+
         final LocalVariables lvs = frame.getLocals();
         for (int i = 0; i < lvs.maxLocals(); i++) {
             Type type = lvs.get(i);
@@ -662,7 +640,7 @@
         //if (debug) {
         //    insList.append(insFactory.createPrintln("restore local variables"));
         //}
-        
+
         final LocalVariables lvs = frame.getLocals();
         Instruction loadStackRecorder = InstructionFactory.createLoad(STACK_RECORDER_TYPE, method.getMaxLocals());
 

Modified: jakarta/commons/sandbox/javaflow/trunk/src/test/org/apache/commons/javaflow/ContinuationTestCase.java
URL: http://svn.apache.org/viewcvs/jakarta/commons/sandbox/javaflow/trunk/src/test/org/apache/commons/javaflow/ContinuationTestCase.java?rev=279714&r1=279713&r2=279714&view=diff
==============================================================================
--- jakarta/commons/sandbox/javaflow/trunk/src/test/org/apache/commons/javaflow/ContinuationTestCase.java (original)
+++ jakarta/commons/sandbox/javaflow/trunk/src/test/org/apache/commons/javaflow/ContinuationTestCase.java Thu Sep  8 23:23:52 2005
@@ -39,4 +39,7 @@
         call("testCounter");
     }
 
+    public void testNewObject() throws Exception {
+        call("testNewObject");
+    }
 }

Modified: jakarta/commons/sandbox/javaflow/trunk/src/test/org/apache/commons/javaflow/ContinuationTests.java
URL: http://svn.apache.org/viewcvs/jakarta/commons/sandbox/javaflow/trunk/src/test/org/apache/commons/javaflow/ContinuationTests.java?rev=279714&r1=279713&r2=279714&view=diff
==============================================================================
--- jakarta/commons/sandbox/javaflow/trunk/src/test/org/apache/commons/javaflow/ContinuationTests.java (original)
+++ jakarta/commons/sandbox/javaflow/trunk/src/test/org/apache/commons/javaflow/ContinuationTests.java Thu Sep  8 23:23:52 2005
@@ -19,9 +19,11 @@
 import java.util.Map;
 
 import junit.framework.TestCase;
+import junit.framework.Assert;
 
 import org.apache.commons.javaflow.testcode.Calculator;
 import org.apache.commons.javaflow.testcode.Counter;
+import org.apache.commons.javaflow.testcode.NewObject;
 import org.apache.commons.javaflow.utils.ReflectionUtils;
 import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
@@ -30,7 +32,7 @@
  * @author tcurdt
  *
  */
-public final class ContinuationTests {
+public final class ContinuationTests extends Assert {
     
     private final static Log log = LogFactory.getLog(ContinuationTests.class);
 
@@ -89,5 +91,10 @@
         Counter runner = new Counter(5);
         for(Continuation c=Continuation.startWith(runner); c!=null; c=Continuation.continueWith(c))
             System.out.println("resuming");
+    }
+
+    public void testNewObject() throws Exception {
+        Continuation c = Continuation.startWith(new NewObject());
+        assertNull(c);
     }
 }



---------------------------------------------------------------------
To unsubscribe, e-mail: commons-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-dev-help@jakarta.apache.org