You are viewing a plain text version of this content. The canonical link for it is here.
Posted to derby-commits@db.apache.org by km...@apache.org on 2007/10/29 18:21:52 UTC

svn commit: r589751 - in /db/derby/code/branches/10.1/java/engine/org/apache/derby: iapi/services/compiler/ iapi/types/ impl/services/bytecode/ impl/sql/compile/

Author: kmarsden
Date: Mon Oct 29 10:21:49 2007
New Revision: 589751

URL: http://svn.apache.org/viewvc?rev=589751&view=rev
Log:
DERBY-742  Use a single result BooleanDataValue for a boolean expression in class generation

merge from trunk. revision 351550 354493


Modified:
    db/derby/code/branches/10.1/java/engine/org/apache/derby/iapi/services/compiler/MethodBuilder.java
    db/derby/code/branches/10.1/java/engine/org/apache/derby/iapi/types/BooleanDataValue.java
    db/derby/code/branches/10.1/java/engine/org/apache/derby/iapi/types/SQLBoolean.java
    db/derby/code/branches/10.1/java/engine/org/apache/derby/impl/services/bytecode/BCMethod.java
    db/derby/code/branches/10.1/java/engine/org/apache/derby/impl/services/bytecode/Conditional.java
    db/derby/code/branches/10.1/java/engine/org/apache/derby/impl/sql/compile/AndNode.java
    db/derby/code/branches/10.1/java/engine/org/apache/derby/impl/sql/compile/BinaryLogicalOperatorNode.java
    db/derby/code/branches/10.1/java/engine/org/apache/derby/impl/sql/compile/IsNode.java
    db/derby/code/branches/10.1/java/engine/org/apache/derby/impl/sql/compile/OrNode.java

Modified: db/derby/code/branches/10.1/java/engine/org/apache/derby/iapi/services/compiler/MethodBuilder.java
URL: http://svn.apache.org/viewvc/db/derby/code/branches/10.1/java/engine/org/apache/derby/iapi/services/compiler/MethodBuilder.java?rev=589751&r1=589750&r2=589751&view=diff
==============================================================================
--- db/derby/code/branches/10.1/java/engine/org/apache/derby/iapi/services/compiler/MethodBuilder.java (original)
+++ db/derby/code/branches/10.1/java/engine/org/apache/derby/iapi/services/compiler/MethodBuilder.java Mon Oct 29 10:21:49 2007
@@ -360,7 +360,7 @@
 	public void methodReturn();
 
 	/**
-		Initiate a sequence that corresponds to the Java language 'ref == null ? ... : ...'.
+		Initiate a conditional sequence.
 		The top value on the stack (a reference) is popped and compared to 'null'.
 		If the value is null then the code following this call until the startElseCode()
 		will be executed at runtime, otherwise the code following startElseCode() until
@@ -380,6 +380,9 @@
 
 		Each path through the ?: statement must leave the stack at the same depth
 		as the other.
+		<BR>
+		If the if or else code pops values from the stack that were before the conditional
+		value, then they must use the same number of values from the stack.
 
 		<PRE>
 		Stack ...,ref =>
@@ -389,12 +392,14 @@
 	*/
 
 	public void conditionalIfNull();
+	
 	/**
-		Initiate a sequence that corresponds to the Java language ' value ? ... : ...'.
+		Initiate a conditional sequence.
 		The top value on the stack must be a boolean and will be popped. If it
 		is true then the code following this call until the startElseCode()
 		will be executed at runtime, otherwise the code following startElseCode() until
-		the completeConditional() is called. See conditionalIfNull() for example.
+		the completeConditional() is called. See conditionalIfNull() for example
+		and restrictions.
 
 		<PRE>
 		Stack ...,boolean_value =>
@@ -404,12 +409,12 @@
 	public void conditionalIf();
 
 	/**
-		Complete the true code path of a ?: operator.
+		Complete the true code path of a conditional.
 	*/
 	public void startElseCode();
 
 	/**
-		Complete the a ?: operator which completes the false code path.
+		Complete a conditional which completes the false code path.
 	*/
 	public void completeConditional();
 

Modified: db/derby/code/branches/10.1/java/engine/org/apache/derby/iapi/types/BooleanDataValue.java
URL: http://svn.apache.org/viewvc/db/derby/code/branches/10.1/java/engine/org/apache/derby/iapi/types/BooleanDataValue.java?rev=589751&r1=589750&r2=589751&view=diff
==============================================================================
--- db/derby/code/branches/10.1/java/engine/org/apache/derby/iapi/types/BooleanDataValue.java (original)
+++ db/derby/code/branches/10.1/java/engine/org/apache/derby/iapi/types/BooleanDataValue.java Mon Oct 29 10:21:49 2007
@@ -127,4 +127,10 @@
 	 * @return	true if the BooleanDataValue contains the given value.
 	 */
 	public boolean equals(boolean value);
+	
+	/**
+	 * Return an immutable BooleanDataValue with the same value as this.
+	 * @return An immutable BooleanDataValue with the same value as this.
+	 */
+	public BooleanDataValue getImmutable();
 }

Modified: db/derby/code/branches/10.1/java/engine/org/apache/derby/iapi/types/SQLBoolean.java
URL: http://svn.apache.org/viewvc/db/derby/code/branches/10.1/java/engine/org/apache/derby/iapi/types/SQLBoolean.java?rev=589751&r1=589750&r2=589751&view=diff
==============================================================================
--- db/derby/code/branches/10.1/java/engine/org/apache/derby/iapi/types/SQLBoolean.java (original)
+++ db/derby/code/branches/10.1/java/engine/org/apache/derby/iapi/types/SQLBoolean.java Mon Oct 29 10:21:49 2007
@@ -1086,6 +1086,18 @@
 		else
 			return value == val;
 	}
+	
+	/**
+	 * Return an immutable BooleanDataValue with the same value as this.
+	 * @return An immutable BooleanDataValue with the same value as this.
+	 */
+	public BooleanDataValue getImmutable()
+	{
+		if (isNull())
+			return SQLBoolean.UNKNOWN;
+		
+		return value ? SQLBoolean.BOOLEAN_TRUE : SQLBoolean.BOOLEAN_FALSE;
+	}
 
 	/*
 	 * String display of value
@@ -1142,7 +1154,7 @@
 	/*
 	 * object state
 	 */
-	protected boolean value;
-	protected boolean isnull;
-	protected boolean immutable;
+	private boolean value;
+	private boolean isnull;
+	private boolean immutable;
 }

Modified: db/derby/code/branches/10.1/java/engine/org/apache/derby/impl/services/bytecode/BCMethod.java
URL: http://svn.apache.org/viewvc/db/derby/code/branches/10.1/java/engine/org/apache/derby/impl/services/bytecode/BCMethod.java?rev=589751&r1=589750&r2=589751&view=diff
==============================================================================
--- db/derby/code/branches/10.1/java/engine/org/apache/derby/impl/services/bytecode/BCMethod.java (original)
+++ db/derby/code/branches/10.1/java/engine/org/apache/derby/impl/services/bytecode/BCMethod.java Mon Oct 29 10:21:49 2007
@@ -63,7 +63,7 @@
  * Method Builder implementation for generating bytecode.
  *
  */
-public class BCMethod implements MethodBuilder {
+class BCMethod implements MethodBuilder {
 
 	final BCClass		cb;
 	protected final ClassHolder modClass; // the class it is in (modifiable fmt)
@@ -270,12 +270,32 @@
 	** New push compiler api.
 	*/
 
+	/**
+	 * Array of the current types of the values on the stack.
+	 * A type that types up two words on the stack, e.g. double
+	 * will only occupy one element in this array.
+	 * This array is dynamically re-sized as needed.
+	 */
 	private Type[]	stackTypes = new Type[8];
+	
+	/**
+	 * Points to the next array offset in stackTypes
+	 * to be used. Really it's the number of valid entries
+	 * in stackTypes.
+	 */
 	private int     stackTypeOffset;
 
+	/**
+	 * Maximum stack depth seen in this method, measured in words.
+	 * Corresponds to max_stack in the Code attribute of section 4.7.3
+	 * of the vm spec.
+	 */
 	private int maxStack;
+	
+	/**
+	 * Current stack depth in this method, measured in words.
+	 */
 	private int stackDepth;
-	// public Stack stackTypes = new Stack();
 
 	private void growStack(int size, Type type) {
 		stackDepth += size;
@@ -314,6 +334,13 @@
 		return topType;
 
 	}
+	
+	private Type[] copyStack()
+	{
+		Type[] stack = new Type[stackTypeOffset];
+		System.arraycopy(stackTypes, 0, stack, 0, stackTypeOffset);
+		return stack;
+	}
 
 	public void pushThis() {
 		myCode.addInstr(VMOpcode.ALOAD_0);
@@ -857,27 +884,28 @@
 
 	private void conditionalIf(short opcode) {
 		popStack();
+		
+		// Save the stack upon entry to the 'then' block of the
+		// 'if' so that we can set up the 'else' block with the
+		// correct stack on entry.
 
-
-		int clearTo = stackTypeOffset;
-
-		condition = new Conditional(condition, myCode, opcode, clearTo);
+		condition = new Conditional(condition, myCode, opcode, copyStack());
 	}
 
 	public void startElseCode() {
-		int clearTo = condition.startElse(myCode, stackTypeOffset);
-
-		if (SanityManager.DEBUG) {
-			if ((stackTypeOffset - 1) != clearTo)
-				SanityManager.THROWASSERT(stackTypeOffset + " is not one more than " + clearTo);
+		
+		// start the else code
+		Type[] entryStack = condition.startElse(myCode, copyStack());
+		
+		for (int i = stackDepth = 0; i  < entryStack.length; i++)
+		{
+			stackDepth += (stackTypes[i] = entryStack[i]).width();
 		}
+		this.stackTypeOffset = entryStack.length;
 
-		while (stackTypeOffset > clearTo) {
-			popStack();
-		}
 	}
 	public void completeConditional() {
-		condition = condition.end(myCode, stackTypeOffset);
+		condition = condition.end(myCode, stackTypes, stackTypeOffset);
 	}
 	
 	public void pop() {

Modified: db/derby/code/branches/10.1/java/engine/org/apache/derby/impl/services/bytecode/Conditional.java
URL: http://svn.apache.org/viewvc/db/derby/code/branches/10.1/java/engine/org/apache/derby/impl/services/bytecode/Conditional.java?rev=589751&r1=589750&r2=589751&view=diff
==============================================================================
--- db/derby/code/branches/10.1/java/engine/org/apache/derby/impl/services/bytecode/Conditional.java (original)
+++ db/derby/code/branches/10.1/java/engine/org/apache/derby/impl/services/bytecode/Conditional.java Mon Oct 29 10:21:49 2007
@@ -20,6 +20,7 @@
 
 package org.apache.derby.impl.services.bytecode;
 import org.apache.derby.iapi.services.classfile.VMOpcode;
+import org.apache.derby.iapi.services.sanity.SanityManager;
 
 /**
 	A code chunk that gets pushed to handle if-else blocks.
@@ -43,19 +44,33 @@
 
 	private final Conditional parent;
 	private final int   ifOffset;
-	private /*final*/ int	clearTo;
+	private Type[]	stack;
 	private int thenGotoOffset;
 
-	Conditional(Conditional parent, CodeChunk chunk, short ifOpcode, int clearTo) {
+	/**
+	 * Start a conditional block.
+	 * @param parent Current conditional block, null if no nesting is going on.
+	 * @param chunk CodeChunk this conditional lives in
+	 * @param ifOpcode Opcode for the if check.
+	 * @param entryStack Type stack on entering the conditional then block.
+	 */
+	Conditional(Conditional parent, CodeChunk chunk, short ifOpcode, Type[] entryStack) {
 		this.parent = parent;
 		ifOffset = chunk.getRelativePC();
-		this.clearTo = clearTo;
+		this.stack = entryStack;
 
 		// reserve the space for the branch, will overwrite later
+		// with the correct branch offset.
 		chunk.addInstrU2(ifOpcode, 0);
 	}
 
-	int startElse(CodeChunk chunk, int thenSize) {
+	/**
+	 * Complete the 'then' block and start the 'else' block for this conditional
+	 * @param chunk CodeChunk this conditional lives in
+	 * @param thenStack Type stack on completing the conditional then block.
+	 * @return the type stack on entering the then block
+	 */
+	Type[] startElse(CodeChunk chunk, Type[] thenStack) {
 
 		thenGotoOffset = chunk.getRelativePC();
 
@@ -64,14 +79,22 @@
 
 		// fill in the branch opcode
 		fillIn(chunk, ifOffset);
-
-		int ret = clearTo;
-		clearTo = thenSize;
-		return ret;
+		
+		Type[] entryStack = stack;
+		stack = thenStack;
+		
+		return entryStack;
 	}
 
 
-	Conditional end(CodeChunk chunk, int elseSize) {
+	/**
+	 * Complete the conditional and patch up any jump instructions.
+	 * @param chunk CodeChunk this conditional lives in
+	 * @param elseStack Current stack, which is the stack at the end of the else
+	 * @param stackNumber Current number of valid elements in elseStack
+	 * @return The conditional this conditional was nested in, if any.
+	 */
+	Conditional end(CodeChunk chunk, Type[] elseStack, int stackNumber) {
 
 		if (thenGotoOffset == 0) {
 			// no else condition
@@ -79,10 +102,22 @@
 		} else {
 			fillIn(chunk, thenGotoOffset);
 		}
-		if (clearTo != elseSize) {
-			throw new RuntimeException("mismatched sizes then " + clearTo + " else " + elseSize);
+		
+		if (SanityManager.DEBUG)
+		{
+			if (stackNumber != stack.length)
+				SanityManager.THROWASSERT("ByteCode Conditional then/else stack depths differ then:"
+						+ stack.length + " else: " + stackNumber);
+			
+			for (int i = 0; i < stackNumber; i++)
+			{
+				if (!stack[i].vmName().equals(elseStack[i].vmName()))
+					SanityManager.THROWASSERT("ByteCode Conditional then/else stack mismatch: then: "
+							+ stack[i].vmName() + 
+							" else: " + elseStack[i].vmName());
+			}
 		}
-
+		
 		return parent;
 	}
 

Modified: db/derby/code/branches/10.1/java/engine/org/apache/derby/impl/sql/compile/AndNode.java
URL: http://svn.apache.org/viewvc/db/derby/code/branches/10.1/java/engine/org/apache/derby/impl/sql/compile/AndNode.java?rev=589751&r1=589750&r2=589751&view=diff
==============================================================================
--- db/derby/code/branches/10.1/java/engine/org/apache/derby/impl/sql/compile/AndNode.java (original)
+++ db/derby/code/branches/10.1/java/engine/org/apache/derby/impl/sql/compile/AndNode.java Mon Oct 29 10:21:49 2007
@@ -41,7 +41,8 @@
 
 	public void init(Object leftOperand, Object rightOperand)
 	{
-		super.init(leftOperand, rightOperand, Boolean.FALSE, "and");
+		super.init(leftOperand, rightOperand, "and");
+		this.shortCircuitValue = false;
 	}
 
 	/**

Modified: db/derby/code/branches/10.1/java/engine/org/apache/derby/impl/sql/compile/BinaryLogicalOperatorNode.java
URL: http://svn.apache.org/viewvc/db/derby/code/branches/10.1/java/engine/org/apache/derby/impl/sql/compile/BinaryLogicalOperatorNode.java?rev=589751&r1=589750&r2=589751&view=diff
==============================================================================
--- db/derby/code/branches/10.1/java/engine/org/apache/derby/impl/sql/compile/BinaryLogicalOperatorNode.java (original)
+++ db/derby/code/branches/10.1/java/engine/org/apache/derby/impl/sql/compile/BinaryLogicalOperatorNode.java Mon Oct 29 10:21:49 2007
@@ -43,7 +43,7 @@
 
 import java.util.Vector;
 
-public abstract class BinaryLogicalOperatorNode extends BinaryOperatorNode
+abstract class BinaryLogicalOperatorNode extends BinaryOperatorNode
 {
 	boolean	shortCircuitValue;
 
@@ -62,13 +62,11 @@
 	public void init(
 				Object	leftOperand,
 				Object	rightOperand,
-				Object		shortCircuitValue,
 				Object		methodName)
 	{
 		/* For logical operators, the operator and method names are the same */
 		super.init(leftOperand, rightOperand, methodName, methodName,
 				ClassName.BooleanDataValue, ClassName.BooleanDataValue);
-		this.shortCircuitValue = ((Boolean) shortCircuitValue).booleanValue();
 	}
 
 	/**
@@ -121,6 +119,8 @@
 
 	/**
 	 * Do code generation for this logical binary operator.
+	 * This is used for AND and OR. the IsNode extends this class but
+	 * overrides generateExpression.
 	 *
 	 * @param acb	The ExpressionClassBuilder for the class we're generating
 	 * @param mb	The method the code to place the code
@@ -132,19 +132,21 @@
 	public void generateExpression(ExpressionClassBuilder acb,
 											MethodBuilder mb)
 		throws StandardException
-	{
+	{		
 		/*
 		** This generates the following code:
 		**
-		** (<fieldx> = <leftOperand>).equals(shortCircuitValue) ?
-		**	 <fieldy> = <shortCircuitValue, nullability> :
-		**   fieldx.<and/or>(<rightOperand>, nullability)
+		** (<leftOperand>.equals(shortCircuitValue) ?
+		**	 <leftOperand> :
+		**   <leftOperand>.<and/or>(<rightOperand>)
 		**
 		** The ?: operator accomplishes the short-circuiting.  We save the
-		** value of the left operand in a field so we don't have to evaluate
-		** it twice.  We save the return value of the getBoolean() call so
-		** we can re-use that object rather than allocate a new one every
-		** time this method is called.
+		** value of the left operand on the stack so we don't have to evaluate
+		** it twice.
+		**
+		** The BooleanDataValue.{and,or} methods return an immutable BooleanDataValue
+		** and an immutable BooleanDataValue is returned by this generated code in
+		** the short circuit case.
 		*/
 
 		/*
@@ -172,28 +174,28 @@
 		** Generated code is:
 		**
 		**		<test for short circuiting> ?
-		**			<call to getBooleanDataValue> : <call to operator method>
+		**			<call to BooleanDataValue.getImmutable> : <call to operator method>
+		**
+		** For AND short circuiting shortcircuit value will be false, so that
+		** if left is false, no need to evaluate the right and the result will be false.
+		**
+		** For OR short circuiting shortcircuit value will be true, so that
+		** if left is true, no need to to evaluate the right and the result will be true.
+		**
+		** In both cases the result is the same as the left operand.
+		**
+		** TODO: Could short circuit when the left value is NULL as well. Then
+		** the result would be NULL in either case and still equal to the left value.
+		** This would require a different check on the conditional.
 		*/
 
 		mb.conditionalIf();
 		
 		// stack: left
+		mb.callMethod(VMOpcode.INVOKEINTERFACE, (String) null, "getImmutable",
+				ClassName.BooleanDataValue, 0);
 		
-		/*
-		** Generate the return value if the left operand equals the short-
-		** circuit value.  Generated code calls a static method in the
-		** boolean datatype implementation that allocates a new object
-		** if necessary, and re-uses the object if it already exists.
-		*/
-		LocalField reusableBoolean = acb.newFieldDeclaration(Modifier.PRIVATE,
-												ClassName.BooleanDataValue);
-
-
-		mb.push(shortCircuitValue);
-		// stack: left, shortcircuit
-		acb.generateDataValue(mb, getTypeCompiler(), reusableBoolean);
-		// stack: left, dvf
-
+		// stack: result (matching left)
 
 		mb.startElseCode();
 
@@ -207,33 +209,18 @@
 		*/
 
 		// stack: left
-		
-		// we duplicate left here rather than just pop'ing left
-		// in the 'then' clause. pop'ing in the then clause
-		// breaks the current conditional implementation
-		// which is modeling a simple ? : operator.
-		// note that this will leave through either path
-		// an extra left on the stack after the conditional
-		mb.dup();
-		// stack left, left
 
 		rightOperand.generateExpression(acb, mb);
-		// stack: left, left, right
+
+		// stack: left, right
 		mb.upCast(ClassName.BooleanDataValue);
 
 		mb.callMethod(VMOpcode.INVOKEINTERFACE, (String) null, methodName, ClassName.BooleanDataValue, 1);
-		// stack: left, result(left op right)
+		// stack: result(left op right)
 
 		mb.completeConditional();
-		//	 stack: left, result
-		
-		// remove the extra left on the stack, see the
-		// comments in the else clause.
-		mb.swap();
-		// stack: result, left
-		mb.pop();
-		
-		// stack: result
+		//	 stack: result
+
 	}
 
 	DataTypeDescriptor resolveLogicalBinaryOperator(

Modified: db/derby/code/branches/10.1/java/engine/org/apache/derby/impl/sql/compile/IsNode.java
URL: http://svn.apache.org/viewvc/db/derby/code/branches/10.1/java/engine/org/apache/derby/impl/sql/compile/IsNode.java?rev=589751&r1=589750&r2=589751&view=diff
==============================================================================
--- db/derby/code/branches/10.1/java/engine/org/apache/derby/impl/sql/compile/IsNode.java (original)
+++ db/derby/code/branches/10.1/java/engine/org/apache/derby/impl/sql/compile/IsNode.java Mon Oct 29 10:21:49 2007
@@ -41,7 +41,7 @@
 
 public class IsNode extends BinaryLogicalOperatorNode
 {
-	boolean		notMe;	// set to true if we're to negate the sense of this node
+	private boolean		notMe;	// set to true if we're to negate the sense of this node
 
 	/**
 	 * Initializer for an IsNode
@@ -56,7 +56,8 @@
 							Object rightOperand,
 							Object notMe )
 	{
-		super.init(leftOperand, rightOperand, Boolean.FALSE, "is" );
+		// the false for shortCir
+		super.init(leftOperand, rightOperand, "is" );
 		this.notMe = ((Boolean) notMe).booleanValue();
 	}
 

Modified: db/derby/code/branches/10.1/java/engine/org/apache/derby/impl/sql/compile/OrNode.java
URL: http://svn.apache.org/viewvc/db/derby/code/branches/10.1/java/engine/org/apache/derby/impl/sql/compile/OrNode.java?rev=589751&r1=589750&r2=589751&view=diff
==============================================================================
--- db/derby/code/branches/10.1/java/engine/org/apache/derby/impl/sql/compile/OrNode.java (original)
+++ db/derby/code/branches/10.1/java/engine/org/apache/derby/impl/sql/compile/OrNode.java Mon Oct 29 10:21:49 2007
@@ -42,7 +42,8 @@
 
 	public void init(Object leftOperand, Object rightOperand)
 	{
-		super.init(leftOperand, rightOperand, Boolean.TRUE, "or");
+		super.init(leftOperand, rightOperand, "or");
+		this.shortCircuitValue = true;
 	}
 
 	/**