[
https://issues.apache.org/jira/browse/DRILL-6868?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16699195#comment-16699195
]
ASF GitHub Bot commented on DRILL-6868:
---------------------------------------
asfgit closed pull request #1553: DRILL-6868: Upgrade Janino compiler to 3.0.11
URL: https://github.com/apache/drill/pull/1553
This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:
As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):
diff --git
a/exec/java-exec/src/main/java/org/apache/drill/exec/compile/bytecode/AloadPopRemover.java
b/exec/java-exec/src/main/java/org/apache/drill/exec/compile/bytecode/AloadPopRemover.java
deleted file mode 100644
index a6df261d3d0..00000000000
---
a/exec/java-exec/src/main/java/org/apache/drill/exec/compile/bytecode/AloadPopRemover.java
+++ /dev/null
@@ -1,333 +0,0 @@
-/*
- * Licensed to the Apache Software Foundation (ASF) under one
- * or more contributor license agreements. See the NOTICE file
- * distributed with this work for additional information
- * regarding copyright ownership. The ASF licenses this file
- * to you under the Apache License, Version 2.0 (the
- * "License"); you may not use this file except in compliance
- * with the License. You may obtain a copy of the License at
- *
- * http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-package org.apache.drill.exec.compile.bytecode;
-
-import org.objectweb.asm.AnnotationVisitor;
-import org.objectweb.asm.Attribute;
-import org.objectweb.asm.Handle;
-import org.objectweb.asm.Label;
-import org.objectweb.asm.MethodVisitor;
-import org.objectweb.asm.Opcodes;
-import org.objectweb.asm.TypePath;
-
-
-/**
- * Remove any adjacent ALOAD-POP instruction pairs.
- *
- * The Janino compiler generates an instruction stream where it will ALOAD a
- * holder's objectref, and then immediately POP it because the compiler has
- * recognized that the method call that it loaded the objectref for is static
- * (i.e., no this pointer is required). This causes a problem with our scalar
- * replacement strategy, because once we remove the holders, we simply
eliminate
- * the ALOAD instructions. In this case, the POP gets left behind, and breaks
- * the program stack.
- *
- * This class looks for ALOADs that are immediately followed by a POP, and
removes
- * the pair of instructions altogether.
- *
- * It is unknown if other compilers besides Janino generate this instruction
sequence,
- * but to be on the safe side, we'll use this class unconditionally to filter
bytecode.
- *
- * TODO: this might be easier to do by going off an InsnList; the state
machine would
- * be in the loop that visits the instructions then.
- */
-public class AloadPopRemover extends MethodVisitor {
- private final static int NONE = -1; // var value to indicate no captured
ALOAD
- private int var = NONE;
-
- /**
- * Constructor.
- *
- * See {@link org.objectweb.asm.MethodVisitor#MethodVisitor(int)}.
- */
- public AloadPopRemover(final int api) {
- super(api);
- }
-
- /**
- * Constructor.
- *
- * See {@link org.objectweb.asm.MethodVisitor#MethodVisitor(int,
MethodVisitor)}.
- */
- public AloadPopRemover(final int api, final MethodVisitor mv) {
- super(api, mv);
- }
-
- /**
- * Process a deferred ALOAD instruction, if there is one.
- *
- * If there is no deferred ALOAD, does nothing, and returns false.
- *
- * If there is a deferred ALOAD, and we're on a POP instruction
- * (indicated by onPop), does nothing (the ALOAD is not forwarded),
- * and returns true.
- *
- * If there is a deferred ALOAD and we're not on a POP instruction,
- * forwards the deferred ALOAD, and returns false
- *
- * @param onPop true if the current instruction is POP
- * @return true if onPop and there was a deferred ALOAD, false otherwise;
- * basically, returns true if the ALOAD-POP optimization is required
- */
- private boolean processDeferredAload(final boolean onPop) {
- // if the previous instruction wasn't an ALOAD, then there's nothing to do
- if (var == NONE) {
- return false;
- }
-
- // clear the variable index, but save it for local use
- final int localVar = var;
- var = NONE;
-
- // if the next instruction is a POP, don't emit the deferred ALOAD
- if (onPop) {
- return true;
- }
-
- // if we got here, we're not on a POP, so emit the deferred ALOAD
- super.visitVarInsn(Opcodes.ALOAD, localVar);
- return false;
- }
-
- @Override
- public AnnotationVisitor visitAnnotation(final String desc, final boolean
visible) {
- processDeferredAload(false);
- final AnnotationVisitor annotationVisitor = super.visitAnnotation(desc,
visible);
- return annotationVisitor;
- }
-
- @Override
- public AnnotationVisitor visitAnnotationDefault() {
- processDeferredAload(false);
- final AnnotationVisitor annotationVisitor = super.visitAnnotationDefault();
- return annotationVisitor;
- }
-
- @Override
- public void visitAttribute(final Attribute attr) {
- processDeferredAload(false);
- super.visitAttribute(attr);
- }
-
- @Override
- public void visitCode() {
- processDeferredAload(false);
- super.visitCode();
- }
-
- @Override
- public void visitEnd() {
- processDeferredAload(false);
- super.visitEnd();
- }
-
- @Override
- public void visitFieldInsn(final int opcode, final String owner, final
String name,
- final String desc) {
- processDeferredAload(false);
- super.visitFieldInsn(opcode, owner, name, desc);
- }
-
- @Override
- public void visitFrame(final int type, final int nLocal,
- final Object[] local, final int nStack, final Object[] stack) {
- processDeferredAload(false);
- super.visitFrame(type, nLocal, local, nStack, stack);
- }
-
- @Override
- public void visitIincInsn(final int var, final int increment) {
- processDeferredAload(false);
- super.visitIincInsn(var, increment);
- }
-
- @Override
- public void visitInsn(final int opcode) {
- /*
- * If we don't omit an ALOAD with a following POP, then forward this.
- * In other words, if we omit an ALOAD because we're on the following POP,
- * don't forward this POP, which omits the ALOAD-POP pair.
- */
- if (!processDeferredAload(Opcodes.POP == opcode)) {
- super.visitInsn(opcode);
- }
- }
-
- @Override
- public AnnotationVisitor visitInsnAnnotation(final int typeRef,
- final TypePath typePath, final String desc, final boolean visible) {
- processDeferredAload(false);
- final AnnotationVisitor annotationVisitor = super.visitInsnAnnotation(
- typeRef, typePath, desc, visible);
- return annotationVisitor;
- }
-
- @Override
- public void visitIntInsn(final int opcode, final int operand) {
- processDeferredAload(false);
- super.visitIntInsn(opcode, operand);
- }
-
- @Override
- public void visitInvokeDynamicInsn(final String name, final String desc,
- final Handle bsm, final Object... bsmArgs) {
- processDeferredAload(false);
- super.visitInvokeDynamicInsn(name, desc, bsm, bsmArgs);
- }
-
- @Override
- public void visitJumpInsn(final int opcode, final Label label) {
- processDeferredAload(false);
- super.visitJumpInsn(opcode, label);
- }
-
- @Override
- public void visitLabel(final Label label) {
- processDeferredAload(false);
- super.visitLabel(label);
- }
-
- @Override
- public void visitLdcInsn(final Object cst) {
- processDeferredAload(false);
- super.visitLdcInsn(cst);
- }
-
- @Override
- public void visitLineNumber(final int line, final Label start) {
- processDeferredAload(false);
- super.visitLineNumber(line, start);
- }
-
- @Override
- public void visitLocalVariable(final String name, final String desc,
- final String signature, final Label start, final Label end, final int
index) {
- processDeferredAload(false);
- super.visitLocalVariable(name, desc, signature, start, end, index);
- }
-
- @Override
- public AnnotationVisitor visitLocalVariableAnnotation(final int typeRef,
- final TypePath typePath, final Label[] start, final Label[] end,
- final int[] index, final String desc, final boolean visible) {
- processDeferredAload(false);
- final AnnotationVisitor annotationVisitor =
- super.visitLocalVariableAnnotation(typeRef, typePath, start, end,
index,
- desc, visible);
- return annotationVisitor;
- }
-
- @Override
- public void visitLookupSwitchInsn(final Label dflt, final int[] keys,
- final Label[] labels) {
- processDeferredAload(false);
- super.visitLookupSwitchInsn(dflt, keys, labels);
- }
-
- @Override
- public void visitMaxs(final int maxStack, final int maxLocals) {
- processDeferredAload(false);
- super.visitMaxs(maxStack, maxLocals);
- }
-
- @Deprecated
- @Override
- public void visitMethodInsn(final int opcode, final String owner,
- final String name, final String desc) {
- processDeferredAload(false);
- super.visitMethodInsn(opcode, owner, name, desc);
- }
-
- @Override
- public void visitMethodInsn(final int opcode, final String owner,
- final String name, final String desc, final boolean itf) {
- processDeferredAload(false);
- super.visitMethodInsn(opcode, owner, name, desc, itf);
- }
-
- @Override
- public void visitMultiANewArrayInsn(final String desc, final int dims) {
- processDeferredAload(false);
- super.visitMultiANewArrayInsn(desc, dims);
- }
-
- @Override
- public void visitParameter(final String name, final int access) {
- processDeferredAload(false);
- super.visitParameter(name, access);
- }
-
- @Override
- public AnnotationVisitor visitParameterAnnotation(final int parameter,
- final String desc, final boolean visible) {
- processDeferredAload(false);
- final AnnotationVisitor annotationVisitor =
- super.visitParameterAnnotation(parameter, desc, visible);
- return annotationVisitor;
- }
-
- @Override
- public void visitTableSwitchInsn(final int min, final int max,
- final Label dflt, final Label... labels) {
- processDeferredAload(false);
- super.visitTableSwitchInsn(min, max, dflt, labels);
- }
-
- @Override
- public AnnotationVisitor visitTryCatchAnnotation(final int typeRef,
- final TypePath typePath, final String desc, final boolean visible) {
- processDeferredAload(false);
- final AnnotationVisitor annotationVisitor =
- super.visitTryCatchAnnotation(typeRef, typePath, desc, visible);
- return annotationVisitor;
- }
-
- @Override
- public void visitTryCatchBlock(final Label start, final Label end,
- final Label handler, final String type) {
- processDeferredAload(false);
- super.visitTryCatchBlock(start, end, handler, type);
- }
-
- @Override
- public AnnotationVisitor visitTypeAnnotation(final int typeRef,
- final TypePath typePath, final String desc, final boolean visible) {
- processDeferredAload(false);
- final AnnotationVisitor annotationVisitor =
- super.visitTypeAnnotation(typeRef, typePath, desc, visible);
- return annotationVisitor;
- }
-
- @Override
- public void visitTypeInsn(final int opcode, final String type) {
- processDeferredAload(false);
- super.visitTypeInsn(opcode, type);
- }
-
- @Override
- public void visitVarInsn(final int opcode, final int var) {
- processDeferredAload(false);
-
- // if we see an ALOAD, defer forwarding it until we know what the next
instruction is
- if (Opcodes.ALOAD == opcode) {
- this.var = var;
- } else {
- super.visitVarInsn(opcode, var);
- }
- }
-}
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 1ed9ea592ee..9094b33a1f4 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
@@ -55,14 +55,8 @@ public MethodVisitor visitMethod(int access, String name,
String desc, String si
innerVisitor = new CheckMethodVisitorFsm(api, innerVisitor);
}
- /*
- * Before using the ScalarReplacementNode to rewrite method code, use the
- * AloadPopRemover to eliminate unnecessary ALOAD-POP pairs; see the
- * AloadPopRemover javadoc for a detailed explanation.
- */
- return new AloadPopRemover(api,
- new ScalarReplacementNode(
- className, access, name, desc, signature, exceptions,
innerVisitor, verifyBytecode));
+ return new ScalarReplacementNode(className, access, name, desc, signature,
+ exceptions,innerVisitor, verifyBytecode);
}
private static class Debugger extends MethodNode {
diff --git
a/exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/MethodGrabbingVisitor.java
b/exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/MethodGrabbingVisitor.java
index 782e1e2b82e..18cc4e47cb6 100644
---
a/exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/MethodGrabbingVisitor.java
+++
b/exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/MethodGrabbingVisitor.java
@@ -21,10 +21,13 @@
import java.util.HashMap;
import java.util.Map;
+import org.codehaus.commons.compiler.CompileException;
import org.codehaus.janino.Java;
import org.codehaus.janino.Java.AbstractClassDeclaration;
import org.codehaus.janino.Java.MethodDeclarator;
+import org.codehaus.janino.Unparser;
import org.codehaus.janino.util.AbstractTraverser;
+import org.codehaus.janino.util.DeepCopier;
public class MethodGrabbingVisitor {
@@ -64,9 +67,59 @@ public void
traverseClassDeclaration(AbstractClassDeclaration classDeclaration)
@Override
public void traverseMethodDeclarator(MethodDeclarator methodDeclarator) {
if (captureMethods) {
+ // Generates a "labeled statement".
+ // This code takes code from the method body, wraps it into the
labeled statement
+ // and replaces all the return statements by break command with label.
+ //
+ // For example, the following method
+ // public void foo(int a) {
+ // if (a < 0) {
+ // return;
+ // } else {
+ // do something;
+ // }
+ // }
+ //
+ // will be converted to
+ // MethodClassName_foo: {
+ // if (a < 0) {
+ // break MethodClassName_foo;
+ // } else {
+ // do something;
+ // }
+ // }
+
+ // Constructs a name of the resulting label
+ // using methods class name and method name itself.
+ String[] fQCN =
methodDeclarator.getDeclaringType().getClassName().split("\\.");
+ String returnLabel = fQCN[fQCN.length - 1] + "_" +
methodDeclarator.name;
+ Java.Block methodBodyBlock = new
Java.Block(methodDeclarator.getLocation());
+
+ // DeepCopier implementation which returns break statement with label
+ // instead if return statement.
+ DeepCopier returnStatementReplacer = new DeepCopier() {
+ @Override
+ public Java.BlockStatement copyReturnStatement(Java.ReturnStatement
subject) {
+ return new Java.BreakStatement(subject.getLocation(), returnLabel);
+ }
+ };
+ try {
+ // replaces return statements and stores the result into
methodBodyBlock
+ methodBodyBlock.addStatements(
+
returnStatementReplacer.copyBlockStatements(methodDeclarator.optionalStatements));
+ } catch (CompileException e) {
+ throw new RuntimeException(e);
+ }
+
+ // wraps method code with replaced return statements into label
statement.
+ Java.LabeledStatement labeledStatement =
+ new Java.LabeledStatement(methodDeclarator.getLocation(),
returnLabel, methodBodyBlock);
+
+ // Unparse the labeled statement.
StringWriter writer = new StringWriter();
- ModifiedUnparser unparser = new ModifiedUnparser(writer);
- unparser.visitMethodDeclarator(methodDeclarator);
+ Unparser unparser = new Unparser(writer);
+ // unparses labeledStatement and stores unparsed code into writer
+ unparser.unparseBlockStatement(labeledStatement);
unparser.close();
writer.flush();
methods.put(methodDeclarator.name, writer.getBuffer().toString());
diff --git
a/exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/ModifiedUnparser.java
b/exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/ModifiedUnparser.java
deleted file mode 100644
index e7b22447c6e..00000000000
---
a/exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/ModifiedUnparser.java
+++ /dev/null
@@ -1,110 +0,0 @@
-/*
- * Licensed to the Apache Software Foundation (ASF) under one
- * or more contributor license agreements. See the NOTICE file
- * distributed with this work for additional information
- * regarding copyright ownership. The ASF licenses this file
- * to you under the Apache License, Version 2.0 (the
- * "License"); you may not use this file except in compliance
- * with the License. You may obtain a copy of the License at
- *
- * http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-package org.apache.drill.exec.expr.fn;
-
-import java.io.Writer;
-import java.util.List;
-
-import org.codehaus.janino.Java;
-import org.codehaus.janino.Unparser;
-import org.codehaus.janino.util.AutoIndentWriter;
-
-/**
- * This is a modified version of {@link Unparser} so that we can avoid
- * rendering few things.
- */
-public class ModifiedUnparser extends Unparser {
-
- private String returnLabel;
-
- public ModifiedUnparser(Writer writer) {
- super(writer);
- }
-
- @Override
- public void unparseBlockStatement(Java.BlockStatement blockStatement) {
- // Unparser uses anonymous classes for visiting statements,
- // therefore added this check for customizing of handling ReturnStatement.
- if (blockStatement instanceof Java.ReturnStatement) {
- visitReturnStatement((Java.ReturnStatement) blockStatement);
- } else {
- super.unparseBlockStatement(blockStatement);
- }
- }
-
- /**
- * Parses specified {@link Java.MethodDeclarator}, wraps its content
- * with replaced {@code return} statements by {@code break} ones into the
- * block with label and stores it into {@link java.io.PrintWriter}.
- *
- * @param methodDeclarator method to parse
- */
- public void visitMethodDeclarator(Java.MethodDeclarator methodDeclarator) {
- if (methodDeclarator.optionalStatements == null) {
- pw.print(';');
- } else if (methodDeclarator.optionalStatements.isEmpty()) {
- pw.print(" {}");
- } else {
- pw.println(' ');
- // Add labels to handle return statements within function templates
- String[] fQCN =
methodDeclarator.getDeclaringType().getClassName().split("\\.");
- returnLabel = fQCN[fQCN.length - 1] + "_" + methodDeclarator.name;
- pw.print(returnLabel);
- pw.println(": {");
- pw.print(AutoIndentWriter.INDENT);
- unparseStatements(methodDeclarator.optionalStatements);
- pw.print(AutoIndentWriter.UNINDENT);
- pw.println("}");
- pw.print(' ');
- }
- }
-
- private void visitReturnStatement(Java.ReturnStatement returnStatement) {
- pw.print("break ");
- pw.print(returnLabel);
- if (returnStatement.optionalReturnValue != null) {
- pw.print(' ');
- unparseAtom(returnStatement.optionalReturnValue);
- }
- pw.print(';');
- }
-
- /**
- * The following helper method is copied from the parent class since it
- * is declared as private in the parent class and can not be used in the
child
- * class (this).
- */
- private void unparseStatements(List<? extends Java.BlockStatement>
statements) {
- int state = -1;
- for (Java.BlockStatement bs : statements) {
- int x = (
- bs instanceof Java.Block ? 1 :
- bs instanceof Java.LocalClassDeclarationStatement ? 2 :
- bs instanceof Java.LocalVariableDeclarationStatement ? 3 :
- bs instanceof Java.SynchronizedStatement ? 4 : 99
- );
- if (state != -1 && state != x) {
- pw.println(AutoIndentWriter.CLEAR_TABULATORS);
- }
- state = x;
-
- unparseBlockStatement(bs);
- pw.println();
- }
- }
-}
diff --git a/pom.xml b/pom.xml
index d69c67993b9..30ea179c75e 100644
--- a/pom.xml
+++ b/pom.xml
@@ -48,7 +48,7 @@
<parquet.version>1.10.0</parquet.version>
<calcite.version>1.17.0-drill-r1</calcite.version>
<avatica.version>1.12.0</avatica.version>
- <janino.version>3.0.10</janino.version>
+ <janino.version>3.0.11</janino.version>
<sqlline.version>1.5.0</sqlline.version>
<jackson.version>2.9.5</jackson.version>
<jackson.databind.version>2.9.5</jackson.databind.version>
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
> Upgrade Janino compiler to 3.0.11
> ---------------------------------
>
> Key: DRILL-6868
> URL: https://issues.apache.org/jira/browse/DRILL-6868
> Project: Apache Drill
> Issue Type: Task
> Affects Versions: 1.14.0
> Reporter: Volodymyr Vysotskyi
> Assignee: Volodymyr Vysotskyi
> Priority: Major
> Labels: ready-to-commit
> Fix For: 1.15.0
>
>
> Recently new version 3.0.11 of Janino compiler was released which contains
> fixes for [https://github.com/janino-compiler/janino/issues/61] and
> [https://github.com/janino-compiler/janino/issues/62]. So after the update,
> workarounds for these issues may be removed.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
