This is an automated email from the ASF dual-hosted git repository. volodymyr pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/drill.git
commit aba268ae70dd5d4012fe2ccbed89e88dc4f81b60 Author: Volodymyr Vysotskyi <[email protected]> AuthorDate: Wed Oct 30 16:49:45 2019 +0200 DRILL-7372: MethodAnalyzer consumes too much memory closes #1887 --- .../exec/compile/bytecode/MethodAnalyzer.java | 35 ++++++++++++---------- .../exec/compile/TestClassTransformation.java | 2 +- .../exec/compile/TestLargeFileCompilation.java | 22 +++++++++++++- 3 files changed, 42 insertions(+), 17 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 ae900cf..1640835 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 @@ -28,9 +28,8 @@ import org.objectweb.asm.tree.analysis.Interpreter; import org.objectweb.asm.tree.analysis.Value; import java.util.ArrayDeque; +import java.util.BitSet; import java.util.Deque; -import java.util.HashSet; -import java.util.Set; /** * Analyzer that allows us to inject additional functionality into ASMs basic analysis. @@ -95,10 +94,10 @@ public class MethodAnalyzer<V extends Value> extends Analyzer <V> { 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; + private Deque<BitSet> localVariablesSet; // stack of LabelNode instances which correspond to the end of conditional block - private final Deque<LabelNode> labelsStack; + private Deque<LabelNode> labelsStack; /** * Constructor. @@ -109,7 +108,7 @@ public class MethodAnalyzer<V extends Value> extends Analyzer <V> { public AssignmentTrackingFrame(int nLocals, int nStack) { super(nLocals, nStack); localVariablesSet = new ArrayDeque<>(); - localVariablesSet.push(new HashSet<>()); + localVariablesSet.push(new BitSet()); labelsStack = new ArrayDeque<>(); } @@ -118,15 +117,13 @@ public class MethodAnalyzer<V extends Value> extends Analyzer <V> { * * @param src the frame being copied */ - @SuppressWarnings("unchecked") public AssignmentTrackingFrame(Frame<? extends V> src) { super(src); - AssignmentTrackingFrame trackingFrame = (AssignmentTrackingFrame) src; + // localVariablesSet and labelsStack aren't copied from the src frame + // since the branch-sensitive analysis was already done for src frame localVariablesSet = new ArrayDeque<>(); - for (Set<Integer> integers : (Deque<Set<Integer>>) trackingFrame.localVariablesSet) { - localVariablesSet.addFirst(new HashSet<>(integers)); - } - labelsStack = new ArrayDeque<>(trackingFrame.labelsStack); + localVariablesSet.push(new BitSet()); + labelsStack = new ArrayDeque<>(); } @Override @@ -140,16 +137,16 @@ public class MethodAnalyzer<V extends Value> extends Analyzer <V> { ReplacingBasicValue replacingValue = (ReplacingBasicValue) value; replacingValue.setFrameSlot(i); V localValue = getLocal(i); - Set<Integer> currentLocalVars = localVariablesSet.element(); + BitSet currentLocalVars = localVariablesSet.element(); if (localValue instanceof ReplacingBasicValue) { - if (!currentLocalVars.contains(i)) { + if (!currentLocalVars.get(i)) { // value is assigned to object declared outside of conditional block replacingValue.setAssignedInConditionalBlock(); } ReplacingBasicValue localReplacingValue = (ReplacingBasicValue) localValue; localReplacingValue.associate(replacingValue); } else { - currentLocalVars.add(i); + currentLocalVars.set(i); } } @@ -174,14 +171,22 @@ public class MethodAnalyzer<V extends Value> extends Analyzer <V> { case IF_ICMPLE: case IF_ACMPEQ: case IF_ACMPNE: + case IFNULL: 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<>()); + localVariablesSet.push(new BitSet()); labelsStack.push(target); } } } + + @Override + public void clearStack() { + super.clearStack(); + localVariablesSet = null; + labelsStack = null; + } } } diff --git a/exec/java-exec/src/test/java/org/apache/drill/exec/compile/TestClassTransformation.java b/exec/java-exec/src/test/java/org/apache/drill/exec/compile/TestClassTransformation.java index ba2feb2..de12fd7 100644 --- a/exec/java-exec/src/test/java/org/apache/drill/exec/compile/TestClassTransformation.java +++ b/exec/java-exec/src/test/java/org/apache/drill/exec/compile/TestClassTransformation.java @@ -44,7 +44,7 @@ import org.objectweb.asm.tree.ClassNode; public class TestClassTransformation extends BaseTestQuery { private static final org.slf4j.Logger logger = org.slf4j.LoggerFactory.getLogger(TestClassTransformation.class); - private static final int ITERATION_COUNT = Integer.valueOf(System.getProperty("TestClassTransformation.iteration", "1")); + private static final int ITERATION_COUNT = Integer.parseInt(System.getProperty("TestClassTransformation.iteration", "1")); private static SessionOptionManager sessionOptions; diff --git a/exec/java-exec/src/test/java/org/apache/drill/exec/compile/TestLargeFileCompilation.java b/exec/java-exec/src/test/java/org/apache/drill/exec/compile/TestLargeFileCompilation.java index ecff4e1..a2b696a 100644 --- a/exec/java-exec/src/test/java/org/apache/drill/exec/compile/TestLargeFileCompilation.java +++ b/exec/java-exec/src/test/java/org/apache/drill/exec/compile/TestLargeFileCompilation.java @@ -41,6 +41,8 @@ public class TestLargeFileCompilation extends BaseTestQuery { private static final String LARGE_QUERY_FILTER; + private static final String QUERY_FILTER; + private static final String HUGE_STRING_CONST_QUERY; private static final String LARGE_QUERY_WRITER; @@ -51,7 +53,7 @@ public class TestLargeFileCompilation extends BaseTestQuery { private static final String LARGE_TABLE_WRITER; - private static final int ITERATION_COUNT = Integer.valueOf(System.getProperty("TestLargeFileCompilation.iteration", "1")); + private static final int ITERATION_COUNT = Integer.parseInt(System.getProperty("TestLargeFileCompilation.iteration", "1")); private static final int NUM_PROJECT_COLUMNS = 2500; @@ -63,6 +65,8 @@ public class TestLargeFileCompilation extends BaseTestQuery { private static final int NUM_FILTER_COLUMNS = 150; + private static final int SMALL_NUM_FILTER_COLUMNS = 50; + private static final int NUM_JOIN_TABLE_COLUMNS = 500; static { @@ -114,6 +118,16 @@ public class TestLargeFileCompilation extends BaseTestQuery { } static { + StringBuilder sb = new StringBuilder("select *\n") + .append("from cp.`employee.json`\n") + .append("where"); + for (int i = 0; i < SMALL_NUM_FILTER_COLUMNS; i++) { + sb.append(" employee_id+").append(i).append(" < employee_id ").append(i % 2 == 0 ? "OR" : "AND"); + } + QUERY_FILTER = sb.append(" true").toString(); + } + + static { final char[] alphabet = "abcdefghijklmnopqrstuvwxyz".toCharArray(); int len = 1 << 18; char[] longText = new char[len]; @@ -186,6 +200,12 @@ public class TestLargeFileCompilation extends BaseTestQuery { } @Test + public void testClassTransformationOOM() throws Exception { + testNoResult("alter session set `%s`='JDK'", ClassCompilerSelector.JAVA_COMPILER_OPTION); + testNoResult(ITERATION_COUNT, QUERY_FILTER); + } + + @Test public void testProject() throws Exception { testNoResult("alter session set `%s`='JDK'", ClassCompilerSelector.JAVA_COMPILER_OPTION); testNoResult(ITERATION_COUNT, LARGE_QUERY_SELECT_LIST);
