> On March 5, 2015, 2:55 a.m., Jinfeng Ni wrote: > > exec/java-exec/src/test/java/org/apache/drill/exec/fn/interp/ExpressionInterpreterTest.java, > > line 64 > > <https://reviews.apache.org/r/31567/diff/2/?file=885228#file885228line64> > > > > Do we have a testcase which use "like", "similar_to" function? Those > > functions are a bit special, in that it requires some input to be constant.
Will add a test case for this. > On March 5, 2015, 2:55 a.m., Jinfeng Ni wrote: > > exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/interpreter/InterpreterEvaluator.java, > > line 298 > > <https://reviews.apache.org/r/31567/diff/2/?file=885225#file885225line298> > > > > I assume that we are aware that there is performance overhead to use > > reflection here. So, in case if we want to use interpreter to evaluate many > > batches, there would be performance impact. On the other hand, we may argue > > that using interpreter already means performance is not as good as the > > current run-time compiling + evaluation model, and therefore, adding > > additional performance overhead does not change a lot. While there are no instances of evaluating a full batch of records, much less a series of batches in the current code with the interpreter, I am not too concerned about this. If we need a little more perforamnce we could modify the DrillFuncHolderExpr class to hold hard references to the Fields for the inputs extracted out of the DrillFuncCalsses with reflection once during a setup step. We would still be using some reflection to set the values, but we can remove the overhead of finding the fields by name each time. Overall if we need performance for lots of prepeatitive expression evaluations, we already have a mechanism for it. I will admit we could probably improve the situation for a medium number of evaluations (maybe in the 10,000 range) with these improvements. > On March 5, 2015, 2:55 a.m., Jinfeng Ni wrote: > > exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/interpreter/InterpreterEvaluator.java, > > line 320 > > <https://reviews.apache.org/r/31567/diff/2/?file=885225#file885225line320> > > > > Do you have a plan to move setup() call into places such that setup() > > will be called once for each VectorAccessible input? > > > > In the code compile + evaluation model, doSetup() will be called per > > batch, in stead of per row. I have started working on a fix for this. Its a little complicated with setting constant inputs before the setup method is called. I'm trying to figure out the best way to share code with the rest of the input passing used in the EvalVisitor. Would you be okay with this being opened as an enhancement request to be merged a little later? Considering the current use of the interpreter this won't have an impact on any actual queries today. - Jason ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31567/#review75278 ----------------------------------------------------------- On March 5, 2015, 12:25 a.m., Jason Altekruse wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/31567/ > ----------------------------------------------------------- > > (Updated March 5, 2015, 12:25 a.m.) > > > Review request for drill, Aman Sinha, Jacques Nadeau, Jinfeng Ni, and Mehant > Baid. > > > Bugs: DRILL-2060 > https://issues.apache.org/jira/browse/DRILL-2060 > > > Repository: drill-git > > > Description > ------- > > The interpreter was previously not used in normal execution, it was added > with unit tests but never hooked up to an execution component. When trying to > use it in the new constant folding issues I ran into build issues that are > described in detail on the 2060 JIRA. > > I have created this to isolate the changes from the others in 2060 for > review, but they are intended to be committed together. > > > Diffs > ----- > > exec/interpreter/pom.xml 20539a8 > > exec/interpreter/src/test/java/org/apache/drill/exec/expr/ExpressionInterpreterTest.java > a94ef94 > > exec/java-exec/src/main/java/org/apache/drill/exec/expr/DrillFuncHolderExpr.java > bc631b8 > > exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillBooleanOPHolder.java > 743598a > > exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillDecimalAddFuncHolder.java > 3871cd7 > > exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillDecimalCastFuncHolder.java > db49173 > > exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillDecimalDivScaleFuncHolder.java > 683a04f > > exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillDecimalMaxScaleFuncHolder.java > aa8e2b5 > > exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillDecimalModScaleFuncHolder.java > b5e754e > > exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillDecimalSetScaleFuncHolder.java > 47b8507 > > exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillDecimalSumScaleFuncHolder.java > cb8bfed > > exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillDecimalZeroScaleFuncHolder.java > 674fc87 > > exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillSimpleFuncHolder.java > ec284a7 > > exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/FunctionConverter.java > 0127e6e > > exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/interpreter/DrillFuncInterpreter.java > 3a83542 > > exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/interpreter/DrillSimpleFuncInterpreter.java > e3696f0 > > exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/interpreter/InterpreterBuilder.java > 3dac818 > > exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/interpreter/InterpreterEvaluator.java > 0fe36cb > > exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/interpreter/InterpreterGenerator.java > 6cede33 > > exec/java-exec/src/main/java/org/apache/drill/exec/vector/ValueHolderHelper.java > 2f5bf6a > > exec/java-exec/src/test/java/org/apache/drill/exec/fn/interp/ExpressionInterpreterTest.java > PRE-CREATION > exec/java-exec/src/test/resources/functions/interp/test_input.csv > PRE-CREATION > exec/pom.xml e27e50b > > Diff: https://reviews.apache.org/r/31567/diff/ > > > Testing > ------- > > Ran the previous interpreter test, with this and part 2 I am still finishing > up some last debugging on a few type-specific test cases. > > > Thanks, > > Jason Altekruse > >
