[ 
https://issues.apache.org/jira/browse/DRILL-3912?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14986640#comment-14986640
 ] 

ASF GitHub Bot commented on DRILL-3912:
---------------------------------------

Github user jinfengni commented on a diff in the pull request:

    https://github.com/apache/drill/pull/189#discussion_r43714802
  
    --- Diff: 
exec/java-exec/src/main/java/org/apache/drill/exec/expr/EvaluationVisitor.java 
---
    @@ -106,19 +177,30 @@ public HoldingContainer 
visitFunctionCall(FunctionCall call, ClassGenerator<?> g
         @Override
         public HoldingContainer visitBooleanOperator(BooleanOperator op,
             ClassGenerator<?> generator) throws RuntimeException {
    +      HoldingContainer hc = getPrevious(op, generator.getMappingSet());
    --- End diff --
    
    You put the check whether it's common subexpression in EvalVisitor, when 
handling with different expression. 
    
    I feel the code would be much cleaner, if we add a CSEFilter in a similar 
to ConstantFilter.  The logic will go through ConstantFilter -> CSEFilter -> 
EvalVisitor.  The check of common subexpression and adding to the Hashmap would 
be done solely in CSEFilter:visitiUnknow().  In AbstractExrVisitor,  other 
methods will by default come to visitUnknow().  In this way, there is no need 
to repeat the checking of common subexpression for different expression in 
EvalVisitor; actually no change has to be done to EvalVisitor.
    
      


> Common subexpression elimination in code generation
> ---------------------------------------------------
>
>                 Key: DRILL-3912
>                 URL: https://issues.apache.org/jira/browse/DRILL-3912
>             Project: Apache Drill
>          Issue Type: Improvement
>            Reporter: Steven Phillips
>            Assignee: Jinfeng Ni
>
> Drill currently will evaluate the full expression tree, even if there are 
> redundant subtrees. Many of these redundant evaluations can be eliminated by 
> reusing the results from previously evaluated expression trees.
> For example,
> {code}
> select a + 1, (a + 1)* (a - 1) from t
> {code}
> Will compute the entire (a + 1) expression twice. With CSE, it will only be 
> evaluated once.
> The benefit will be reducing the work done when evaluating expressions, as 
> well as reducing the amount of code that is generated, which could also lead 
> to better JIT optimization.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

Reply via email to