diff options
Diffstat (limited to 'exec/java-exec/src/main/java/org/apache/drill/exec/compile')
4 files changed, 144 insertions, 33 deletions
diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/compile/bytecode/MethodAnalyzer.java b/exec/java-exec/src/main/java/org/apache/drill/exec/compile/bytecode/MethodAnalyzer.java index 78cf16fc1..ae900cf71 100644 --- a/exec/java-exec/src/main/java/org/apache/drill/exec/compile/bytecode/MethodAnalyzer.java +++ b/exec/java-exec/src/main/java/org/apache/drill/exec/compile/bytecode/MethodAnalyzer.java @@ -17,16 +17,26 @@ */ package org.apache.drill.exec.compile.bytecode; +import org.objectweb.asm.tree.AbstractInsnNode; +import org.objectweb.asm.tree.InsnList; +import org.objectweb.asm.tree.LabelNode; +import org.objectweb.asm.tree.MethodNode; import org.objectweb.asm.tree.analysis.Analyzer; +import org.objectweb.asm.tree.analysis.AnalyzerException; import org.objectweb.asm.tree.analysis.Frame; import org.objectweb.asm.tree.analysis.Interpreter; import org.objectweb.asm.tree.analysis.Value; +import java.util.ArrayDeque; +import java.util.Deque; +import java.util.HashSet; +import java.util.Set; + /** * Analyzer that allows us to inject additional functionality into ASMs basic analysis. * * <p>We need to be able to keep track of local variables that are assigned to each other - * so that we can infer their replacability (for scalar replacement). In order to do that, + * so that we can infer their replaceability (for scalar replacement). In order to do that, * we need to know when local variables are assigned (with the old value being overwritten) * so that we can associate them with the new value, and hence determine whether they can * also be replaced, or not. @@ -36,22 +46,71 @@ import org.objectweb.asm.tree.analysis.Value; * as factories that will provide our own derivative of Frame<> which we use to detect */ public class MethodAnalyzer<V extends Value> extends Analyzer <V> { + + // list of method instructions which is analyzed + private InsnList insnList; + + public MethodAnalyzer(Interpreter<V> interpreter) { + super(interpreter); + } + + @Override + protected Frame<V> newFrame(int maxLocals, int maxStack) { + return new AssignmentTrackingFrame<>(maxLocals, maxStack); + } + + @Override + protected Frame<V> newFrame(Frame<? extends V> src) { + return new AssignmentTrackingFrame<>(src); + } + + @Override + protected void newControlFlowEdge(int insnIndex, int successorIndex) { + AssignmentTrackingFrame<V> oldFrame = (AssignmentTrackingFrame<V>) getFrames()[insnIndex]; + AbstractInsnNode insn = insnList.get(insnIndex); + if (insn.getType() == AbstractInsnNode.LABEL) { + // checks whether current label corresponds to the end of conditional block to restore previous + // local variables set + if (insn.equals(oldFrame.labelsStack.peekFirst())) { + oldFrame.localVariablesSet.pop(); + oldFrame.labelsStack.pop(); + } + } + } + + @Override + public Frame<V>[] analyze(String owner, MethodNode method) throws AnalyzerException { + insnList = method.instructions; + return super.analyze(owner, method); + } + /** * Custom Frame<> that captures setLocal() calls in order to associate values - * that are assigned to the same local variable slot. + * that are assigned to the same local variable slot. Also it controls stack to determine whether + * object was assigned to the value declared outside of conditional block. * * <p>Since this is almost a pass-through, the constructors' arguments match * those from Frame<>. */ private static class AssignmentTrackingFrame<V extends Value> extends Frame<V> { + + // represents stack of variable sets declared inside current code block + private final Deque<Set<Integer>> localVariablesSet; + + // stack of LabelNode instances which correspond to the end of conditional block + private final Deque<LabelNode> labelsStack; + /** * Constructor. * * @param nLocals the number of locals the frame should have * @param nStack the maximum size of the stack the frame should have */ - public AssignmentTrackingFrame(final int nLocals, final int nStack) { + public AssignmentTrackingFrame(int nLocals, int nStack) { super(nLocals, nStack); + localVariablesSet = new ArrayDeque<>(); + localVariablesSet.push(new HashSet<>()); + labelsStack = new ArrayDeque<>(); } /** @@ -59,47 +118,70 @@ public class MethodAnalyzer<V extends Value> extends Analyzer <V> { * * @param src the frame being copied */ - public AssignmentTrackingFrame(final Frame<? extends V> src) { + @SuppressWarnings("unchecked") + public AssignmentTrackingFrame(Frame<? extends V> src) { super(src); + AssignmentTrackingFrame trackingFrame = (AssignmentTrackingFrame) src; + localVariablesSet = new ArrayDeque<>(); + for (Set<Integer> integers : (Deque<Set<Integer>>) trackingFrame.localVariablesSet) { + localVariablesSet.addFirst(new HashSet<>(integers)); + } + labelsStack = new ArrayDeque<>(trackingFrame.labelsStack); } @Override - public void setLocal(final int i, final V value) { + public void setLocal(int i, V value) { /* * If we're replacing one ReplacingBasicValue with another, we need to - * associate them together so that they will have the same replacability + * associate them together so that they will have the same replaceability * attributes. We also track the local slot the new value will be stored in. */ if (value instanceof ReplacingBasicValue) { - final ReplacingBasicValue replacingValue = (ReplacingBasicValue) value; + ReplacingBasicValue replacingValue = (ReplacingBasicValue) value; replacingValue.setFrameSlot(i); - final V localValue = getLocal(i); - if ((localValue != null) && (localValue instanceof ReplacingBasicValue)) { - final ReplacingBasicValue localReplacingValue = (ReplacingBasicValue) localValue; + V localValue = getLocal(i); + Set<Integer> currentLocalVars = localVariablesSet.element(); + if (localValue instanceof ReplacingBasicValue) { + if (!currentLocalVars.contains(i)) { + // value is assigned to object declared outside of conditional block + replacingValue.setAssignedInConditionalBlock(); + } + ReplacingBasicValue localReplacingValue = (ReplacingBasicValue) localValue; localReplacingValue.associate(replacingValue); + } else { + currentLocalVars.add(i); } } super.setLocal(i, value); } - } - /** - * Constructor. - * - * @param interpreter the interpreter to use - */ - public MethodAnalyzer(final Interpreter<V> interpreter) { - super(interpreter); - } - - @Override - protected Frame<V> newFrame(final int maxLocals, final int maxStack) { - return new AssignmentTrackingFrame<V>(maxLocals, maxStack); - } - - @Override - protected Frame<V> newFrame(final Frame<? extends V> src) { - return new AssignmentTrackingFrame<V>(src); + @Override + public void initJumpTarget(int opcode, LabelNode target) { + if (target != null) { + switch (opcode) { + case IFEQ: + case IFNE: + case IFLT: + case IFGE: + case IFGT: + case IFLE: + case IF_ICMPEQ: + case IF_ICMPNE: + case IF_ICMPLT: + case IF_ICMPGE: + case IF_ICMPGT: + case IF_ICMPLE: + case IF_ACMPEQ: + case IF_ACMPNE: + case IFNONNULL: + // for the case when conditional block is handled, creates new variables set + // to store local variables declared inside current conditional block and + // stores its target LabelNode to restore previous variables set after conditional block is ended + localVariablesSet.push(new HashSet<>()); + labelsStack.push(target); + } + } + } } } diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/compile/bytecode/ReplacingBasicValue.java b/exec/java-exec/src/main/java/org/apache/drill/exec/compile/bytecode/ReplacingBasicValue.java index c05f5cbbe..42c8685ef 100644 --- a/exec/java-exec/src/main/java/org/apache/drill/exec/compile/bytecode/ReplacingBasicValue.java +++ b/exec/java-exec/src/main/java/org/apache/drill/exec/compile/bytecode/ReplacingBasicValue.java @@ -49,6 +49,7 @@ public class ReplacingBasicValue extends BasicValue { boolean isFunctionReturn = false; boolean isFunctionArgument = false; boolean isAssignedToMember = false; + boolean isAssignedInConditionalBlock = false; boolean isThis = false; /** @@ -67,6 +68,9 @@ public class ReplacingBasicValue extends BasicValue { if (other.isAssignedToMember) { isAssignedToMember = true; } + if (other.isAssignedInConditionalBlock) { + isAssignedInConditionalBlock = true; + } if (other.isThis) { isThis = true; } @@ -78,7 +82,7 @@ public class ReplacingBasicValue extends BasicValue { * @return whether or not the value is replaceable */ public boolean isReplaceable() { - return !(isFunctionReturn || isFunctionArgument || isAssignedToMember || isThis); + return !(isFunctionReturn || isFunctionArgument || isAssignedToMember || isAssignedInConditionalBlock || isThis); } /** @@ -115,6 +119,15 @@ public class ReplacingBasicValue extends BasicValue { needSpace = true; } + if (isAssignedInConditionalBlock) { + if (needSpace) { + sb.append(' '); + } + + sb.append("conditional"); + needSpace = true; + } + if (isThis) { if (needSpace) { sb.append(' '); @@ -397,6 +410,13 @@ public class ReplacingBasicValue extends BasicValue { } /** + * Mark this value as being assigned to a variable inside of conditional block. + */ + public void setAssignedInConditionalBlock() { + flagSet.isAssignedInConditionalBlock = true; + } + + /** * Indicates whether or not this value is assigned to a class member variable. * * @return whether or not this value is assigned to a class member variable @@ -406,6 +426,15 @@ public class ReplacingBasicValue extends BasicValue { } /** + * Indicates whether or not this value is assigned to a variable inside of conditional block. + * + * @return whether or not this value is assigned to a variable inside of conditional block + */ + public boolean isAssignedInConditionalBlock() { + return flagSet.isAssignedInConditionalBlock; + } + + /** * Return the ValueHolder identity for this value. * * @return the ValueHolderIden for this value diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/compile/bytecode/ScalarReplacementNode.java b/exec/java-exec/src/main/java/org/apache/drill/exec/compile/bytecode/ScalarReplacementNode.java index 3963e9b52..bd836e02e 100644 --- a/exec/java-exec/src/main/java/org/apache/drill/exec/compile/bytecode/ScalarReplacementNode.java +++ b/exec/java-exec/src/main/java/org/apache/drill/exec/compile/bytecode/ScalarReplacementNode.java @@ -56,7 +56,7 @@ public class ScalarReplacementNode extends MethodNode { final LinkedList<ReplacingBasicValue> valueList = new LinkedList<>(); final MethodAnalyzer<BasicValue> analyzer = - new MethodAnalyzer<BasicValue>(new ReplacingInterpreter(className, valueList)); + new MethodAnalyzer<>(new ReplacingInterpreter(className, valueList)); Frame<BasicValue>[] frames; try { frames = analyzer.analyze(className, this); diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/compile/bytecode/ValueHolderReplacementVisitor.java b/exec/java-exec/src/main/java/org/apache/drill/exec/compile/bytecode/ValueHolderReplacementVisitor.java index 9094b33a1..1eb1e7948 100644 --- a/exec/java-exec/src/main/java/org/apache/drill/exec/compile/bytecode/ValueHolderReplacementVisitor.java +++ b/exec/java-exec/src/main/java/org/apache/drill/exec/compile/bytecode/ValueHolderReplacementVisitor.java @@ -50,20 +50,20 @@ public class ValueHolderReplacementVisitor extends ClassVisitor { @Override public MethodVisitor visitMethod(int access, String name, String desc, String signature, String[] exceptions) { MethodVisitor innerVisitor = super.visitMethod(access, name, desc, signature, exceptions); -// innerVisitor = new Debugger(access, name, desc, signature, exceptions, innerVisitor); +// innerVisitor = new Debugger(access, name, desc, signature, exceptions, innerVisitor); if (verifyBytecode) { innerVisitor = new CheckMethodVisitorFsm(api, innerVisitor); } return new ScalarReplacementNode(className, access, name, desc, signature, - exceptions,innerVisitor, verifyBytecode); + exceptions, innerVisitor, verifyBytecode); } private static class Debugger extends MethodNode { MethodVisitor inner; public Debugger(int access, String name, String desc, String signature, String[] exceptions, MethodVisitor inner) { - super(access, name, desc, signature, exceptions); + super(CompilationConfig.ASM_API_VERSION, access, name, desc, signature, exceptions); this.inner = inner; } |