You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Andrew Sherman (Code Review)" <ge...@cloudera.org> on 2019/02/05 19:25:42 UTC

[Impala-ASF-CR] IMPALA-7657: Codegen IsNotEmptyPredicate and ValidTupleIdExpr.

Andrew Sherman has uploaded a new patch set (#6). ( http://gerrit.cloudera.org:8080/12068 )

Change subject: IMPALA-7657: Codegen IsNotEmptyPredicate and ValidTupleIdExpr.
......................................................................

IMPALA-7657: Codegen IsNotEmptyPredicate and ValidTupleIdExpr.

These two classes evaluate scalar expressions. Previously codegen was
done by calling ScalarExpr::GetCodegendComputeFnWrapper which generates
a static method that calls the scalar expression evaluation method. Make
this more efficient by generating code which is customized using
information available at codegen time.

Add new cross-compiled files null-literal-ir.cc slot-ref-ir.cc

IsNotEmptyPredicate works by getting a CollectionVal object from the
single child Expr node, and counting its tuples. At codegen time we know
the type and value of the child node. Generate a call to a node-specific
non-virtual cross-compiled method to get the CollectionVal object from
the child. Then generate a code that examines the CollectionVal and
returns an IntVal.

A ValidTupleIdExpr node contains a vector of tuple ids. It works by
probing each row for the tuple ids in the vector to find a non-null
tuple. At codegen time we know the vector of tuple ids. We unroll the
loop through the tuple ids, generating code that evaluates if the tuple
is non-null, and returns the tuple id if/when a non-null tuple is found.

IMPALA-7657 also requests replacing GetCodegendComputeFnWrapper() in
TupleIsNullPredicate. In the current Impala code this method is never
called. This is because TupleIsNullPredicate is always wrapped in an
IfExpr. This is always codegen'd by IfExpr's
GetCodegendComputeFnWrapper() method. There is a separate Jira
IMPALA-7655 to improve codegen of IfExpr.

Minor corrections:
  Correct the link to llvm tutorial in LlvmCodegen.

PERFORMANCE:
  I tested performance on a local mini-cluster. I wrote some
  pathological queries to test the new code. The new codegen'd code is
  very similar in performance. Both ValidTupleIdExpr and
  IsNotEmptyPredicate seem very slightly faster than the old code.
  Overall these changes are not purely for performance but to move away
  from GetCodegendComputeFnWrapper.

TESTING:
  The changed scalar expressions are well exercised by current tests.
  Ran exhaustive end-to-end tests.

Change-Id: Ifb87b9e3b879c278ce8638d97bcb320a7555a6b3
---
M be/src/codegen/gen_ir_descriptions.py
M be/src/codegen/impala-ir.cc
M be/src/codegen/llvm-codegen.h
M be/src/exprs/CMakeLists.txt
M be/src/exprs/is-not-empty-predicate.cc
M be/src/exprs/is-not-empty-predicate.h
A be/src/exprs/null-literal-ir.cc
M be/src/exprs/null-literal.cc
M be/src/exprs/null-literal.h
M be/src/exprs/scalar-expr.h
A be/src/exprs/slot-ref-ir.cc
M be/src/exprs/slot-ref.cc
M be/src/exprs/slot-ref.h
M be/src/exprs/valid-tuple-id.cc
M be/src/exprs/valid-tuple-id.h
15 files changed, 361 insertions(+), 35 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/68/12068/6
-- 
To view, visit http://gerrit.cloudera.org:8080/12068
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ifb87b9e3b879c278ce8638d97bcb320a7555a6b3
Gerrit-Change-Number: 12068
Gerrit-PatchSet: 6
Gerrit-Owner: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Thomas Marshall <th...@cmu.edu>