> On March 4, 2015, 6:55 p.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. > > Jason Altekruse wrote: > 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.
Make sense to me. > On March 4, 2015, 6:55 p.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. > > Jason Altekruse wrote: > 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. Yes, I'm fine that you enhance this later in a new JIRA. - Jinfeng ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31567/#review75278 ----------------------------------------------------------- On March 5, 2015, 5:18 p.m., Jason Altekruse wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/31567/ > ----------------------------------------------------------- > > (Updated March 5, 2015, 5:18 p.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 > >
