You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@impala.apache.org by cs...@apache.org on 2020/03/05 13:48:54 UTC

[impala] branch master updated: IMPALA-6506: Codegen in ORC scanner for primitives and struct

This is an automated email from the ASF dual-hosted git repository.

csringhofer pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/impala.git


The following commit(s) were added to refs/heads/master by this push:
     new 0b081ae  IMPALA-6506: Codegen in ORC scanner for primitives and struct
0b081ae is described below

commit 0b081aef3fc681b40c9cc45e0387bf7dd84358a9
Author: Gabor Kaszab <ga...@cloudera.com>
AuthorDate: Mon Mar 2 16:18:26 2020 +0100

    IMPALA-6506: Codegen in ORC scanner for primitives and struct
    
    IMPALA-9228 introduced scratch batch handling for struct and
    primitive types in the ORC scanner and the existing scratch batch
    logic already supports Codegen for ProcessScratchBatch() function.
    This change turns on this Codegen logic for primitives types and
    structs in the ORC scanner.
    
    Note, if the query involves collection types then
    ProcessScratchBatch() is still codegend but the codegend function
    isn't used as the regular row-by-row approach is followed in this
    case without using a scratch batch.
    
    Testing:
      - Re-run the whole test suite to check for regressions.
      - Checked the performance on a scale 25 TPCH workload in ORC format
        using single_node_perf_run.py. Comparing the query runtimes it
        seems that codegen brings a 1-21% improvement for most of the
        queries. There is a slight decrease in 3 queries that are not
        scan-heavy where codegen doesn't provide any help for scanning.
        However, these are short queries where the size of the
        degradation is in subseconds so I'd say the decrease is
        negligible.
      - Did a manual check for a table that contains both Parquet and ORC
        partitions. Verified that in this case ProcessScratchBatch() is
        codegend for both formats and the query results are as expected.
    
    Change-Id: I2352d0c8fc75ff722e931bc8c866b3e43d3636f4
    Reviewed-on: http://gerrit.cloudera.org:8080/15350
    Reviewed-by: Impala Public Jenkins <im...@cloudera.com>
    Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
 be/src/exec/hdfs-columnar-scanner.cc        | 38 +++++++++++++++++++++++++++++
 be/src/exec/hdfs-columnar-scanner.h         |  9 +++++++
 be/src/exec/hdfs-orc-scanner.cc             |  9 ++++++-
 be/src/exec/hdfs-scan-node-base.cc          |  4 ++-
 be/src/exec/parquet/hdfs-parquet-scanner.cc | 36 ---------------------------
 be/src/exec/parquet/hdfs-parquet-scanner.h  |  9 -------
 6 files changed, 58 insertions(+), 47 deletions(-)

diff --git a/be/src/exec/hdfs-columnar-scanner.cc b/be/src/exec/hdfs-columnar-scanner.cc
index 9f84985..06e9295 100644
--- a/be/src/exec/hdfs-columnar-scanner.cc
+++ b/be/src/exec/hdfs-columnar-scanner.cc
@@ -19,8 +19,12 @@
 
 #include <algorithm>
 
+#include "codegen/llvm-codegen.h"
+
 namespace impala {
 
+const char* HdfsColumnarScanner::LLVM_CLASS_NAME = "class.impala::HdfsColumnarScanner";
+
 HdfsColumnarScanner::HdfsColumnarScanner(HdfsScanNodeBase* scan_node,
     RuntimeState* state) :
     HdfsScanner(scan_node, state),
@@ -64,4 +68,38 @@ int HdfsColumnarScanner::TransferScratchTuples(RowBatch* dst_batch) {
   return num_rows_to_commit;
 }
 
+Status HdfsColumnarScanner::Codegen(HdfsScanPlanNode* node, RuntimeState* state,
+    llvm::Function** process_scratch_batch_fn) {
+  DCHECK(state->ShouldCodegen());
+  *process_scratch_batch_fn = nullptr;
+  LlvmCodeGen* codegen = state->codegen();
+  DCHECK(codegen != nullptr);
+
+  llvm::Function* fn = codegen->GetFunction(IRFunction::PROCESS_SCRATCH_BATCH, true);
+  DCHECK(fn != nullptr);
+
+  llvm::Function* eval_conjuncts_fn;
+  const vector<ScalarExpr*>& conjuncts = node->conjuncts_;
+  RETURN_IF_ERROR(ExecNode::CodegenEvalConjuncts(codegen, conjuncts, &eval_conjuncts_fn));
+  DCHECK(eval_conjuncts_fn != nullptr);
+
+  int replaced = codegen->ReplaceCallSites(fn, eval_conjuncts_fn, "EvalConjuncts");
+  DCHECK_REPLACE_COUNT(replaced, 1);
+
+  llvm::Function* eval_runtime_filters_fn;
+  RETURN_IF_ERROR(CodegenEvalRuntimeFilters(
+      codegen, node->runtime_filter_exprs_, &eval_runtime_filters_fn));
+  DCHECK(eval_runtime_filters_fn != nullptr);
+
+  replaced = codegen->ReplaceCallSites(fn, eval_runtime_filters_fn, "EvalRuntimeFilters");
+  DCHECK_REPLACE_COUNT(replaced, 1);
+
+  fn->setName("ProcessScratchBatch");
+  *process_scratch_batch_fn = codegen->FinalizeFunction(fn);
+  if (*process_scratch_batch_fn == nullptr) {
+    return Status("Failed to finalize process_scratch_batch_fn.");
+  }
+  return Status::OK();
+}
+
 }
diff --git a/be/src/exec/hdfs-columnar-scanner.h b/be/src/exec/hdfs-columnar-scanner.h
index a849282..5c627bf 100644
--- a/be/src/exec/hdfs-columnar-scanner.h
+++ b/be/src/exec/hdfs-columnar-scanner.h
@@ -21,6 +21,7 @@
 
 #include <boost/scoped_ptr.hpp>
 
+#include "codegen/impala-ir.h"
 #include "exec/hdfs-scan-node-base.h"
 #include "exec/scratch-tuple-batch.h"
 #include "runtime/row-batch.h"
@@ -35,6 +36,14 @@ class HdfsColumnarScanner : public HdfsScanner {
   HdfsColumnarScanner(HdfsScanNodeBase* scan_node, RuntimeState* state);
   virtual ~HdfsColumnarScanner();
 
+  /// Codegen ProcessScratchBatch(). Stores the resulting function in
+  /// 'process_scratch_batch_fn' if codegen was successful or NULL otherwise.
+  static Status Codegen(HdfsScanPlanNode* node, RuntimeState* state,
+      llvm::Function** process_scratch_batch_fn);
+
+  /// Class name in LLVM IR.
+  static const char* LLVM_CLASS_NAME;
+
  protected:
   /// Column readers will write slot values into this scratch batch for
   /// top-level tuples. See AssembleRows() in the derived classes.
diff --git a/be/src/exec/hdfs-orc-scanner.cc b/be/src/exec/hdfs-orc-scanner.cc
index 112b558..d1be330 100644
--- a/be/src/exec/hdfs-orc-scanner.cc
+++ b/be/src/exec/hdfs-orc-scanner.cc
@@ -154,7 +154,14 @@ Status HdfsOrcScanner::Open(ScannerContext* context) {
       ADD_COUNTER(scan_node_->runtime_profile(), "NumScannersWithNoReads", TUnit::UNIT);
   process_footer_timer_stats_ =
       ADD_SUMMARY_STATS_TIMER(scan_node_->runtime_profile(), "OrcFooterProcessingTime");
-  scan_node_->IncNumScannersCodegenDisabled();
+
+  codegend_process_scratch_batch_fn_ = reinterpret_cast<ProcessScratchBatchFn>(
+      scan_node_->GetCodegenFn(THdfsFileFormat::ORC));
+  if (codegend_process_scratch_batch_fn_ == nullptr) {
+    scan_node_->IncNumScannersCodegenDisabled();
+  } else {
+    scan_node_->IncNumScannersCodegenEnabled();
+  }
 
   DCHECK(parse_status_.ok()) << "Invalid parse_status_" << parse_status_.GetDetail();
   for (const FilterContext& ctx : context->filter_ctxs()) {
diff --git a/be/src/exec/hdfs-scan-node-base.cc b/be/src/exec/hdfs-scan-node-base.cc
index 984ca88..10f9266 100644
--- a/be/src/exec/hdfs-scan-node-base.cc
+++ b/be/src/exec/hdfs-scan-node-base.cc
@@ -19,6 +19,7 @@
 
 #include "exec/base-sequence-scanner.h"
 #include "exec/hdfs-avro-scanner.h"
+#include "exec/hdfs-columnar-scanner.h"
 #include "exec/hdfs-orc-scanner.h"
 #include "exec/hdfs-plugin-text-scanner.h"
 #include "exec/hdfs-rcfile-scanner.h"
@@ -434,7 +435,8 @@ void HdfsScanPlanNode::Codegen(RuntimeState* state, RuntimeProfile* profile) {
         status = HdfsAvroScanner::Codegen(this, state, &fn);
         break;
       case THdfsFileFormat::PARQUET:
-        status = HdfsParquetScanner::Codegen(this, state, &fn);
+      case THdfsFileFormat::ORC:
+        status = HdfsColumnarScanner::Codegen(this, state, &fn);
         break;
       default:
         // No codegen for this format
diff --git a/be/src/exec/parquet/hdfs-parquet-scanner.cc b/be/src/exec/parquet/hdfs-parquet-scanner.cc
index 0dd5d59..7b3f4d1 100644
--- a/be/src/exec/parquet/hdfs-parquet-scanner.cc
+++ b/be/src/exec/parquet/hdfs-parquet-scanner.cc
@@ -55,8 +55,6 @@ using namespace impala::io;
 // THIS RECORDS INFORMATION ABOUT PAST BEHAVIOR. DO NOT CHANGE THIS CONSTANT.
 const int LEGACY_IMPALA_MAX_DICT_ENTRIES = 40000;
 
-const char* HdfsParquetScanner::LLVM_CLASS_NAME = "class.impala::HdfsParquetScanner";
-
 static const string PARQUET_MEM_LIMIT_EXCEEDED =
     "HdfsParquetScanner::$0() failed to allocate $1 bytes for $2.";
 
@@ -1175,40 +1173,6 @@ Status HdfsParquetScanner::CommitRows(RowBatch* dst_batch, int num_rows) {
   return Status::OK();
 }
 
-Status HdfsParquetScanner::Codegen(HdfsScanPlanNode* node, RuntimeState* state,
-      llvm::Function** process_scratch_batch_fn) {
-  DCHECK(state->ShouldCodegen());
-  *process_scratch_batch_fn = nullptr;
-  LlvmCodeGen* codegen = state->codegen();
-  DCHECK(codegen != nullptr);
-
-  llvm::Function* fn = codegen->GetFunction(IRFunction::PROCESS_SCRATCH_BATCH, true);
-  DCHECK(fn != nullptr);
-
-  llvm::Function* eval_conjuncts_fn;
-  const vector<ScalarExpr*>& conjuncts = node->conjuncts_;
-  RETURN_IF_ERROR(ExecNode::CodegenEvalConjuncts(codegen, conjuncts, &eval_conjuncts_fn));
-  DCHECK(eval_conjuncts_fn != nullptr);
-
-  int replaced = codegen->ReplaceCallSites(fn, eval_conjuncts_fn, "EvalConjuncts");
-  DCHECK_REPLACE_COUNT(replaced, 1);
-
-  llvm::Function* eval_runtime_filters_fn;
-  RETURN_IF_ERROR(CodegenEvalRuntimeFilters(
-      codegen, node->runtime_filter_exprs_, &eval_runtime_filters_fn));
-  DCHECK(eval_runtime_filters_fn != nullptr);
-
-  replaced = codegen->ReplaceCallSites(fn, eval_runtime_filters_fn, "EvalRuntimeFilters");
-  DCHECK_REPLACE_COUNT(replaced, 1);
-
-  fn->setName("ProcessScratchBatch");
-  *process_scratch_batch_fn = codegen->FinalizeFunction(fn);
-  if (*process_scratch_batch_fn == nullptr) {
-    return Status("Failed to finalize process_scratch_batch_fn.");
-  }
-  return Status::OK();
-}
-
 bool HdfsParquetScanner::AssembleCollection(
     const vector<ParquetColumnReader*>& column_readers, int new_collection_rep_level,
     CollectionValueBuilder* coll_value_builder) {
diff --git a/be/src/exec/parquet/hdfs-parquet-scanner.h b/be/src/exec/parquet/hdfs-parquet-scanner.h
index 9167bf5..9ed3cd2 100644
--- a/be/src/exec/parquet/hdfs-parquet-scanner.h
+++ b/be/src/exec/parquet/hdfs-parquet-scanner.h
@@ -19,7 +19,6 @@
 #ifndef IMPALA_EXEC_HDFS_PARQUET_SCANNER_H
 #define IMPALA_EXEC_HDFS_PARQUET_SCANNER_H
 
-#include "codegen/impala-ir.h"
 #include "exec/hdfs-columnar-scanner.h"
 #include "exec/parquet/parquet-column-stats.h"
 #include "exec/parquet/parquet-common.h"
@@ -348,11 +347,6 @@ class HdfsParquetScanner : public HdfsColumnarScanner {
   virtual Status ProcessSplit() WARN_UNUSED_RESULT;
   virtual void Close(RowBatch* row_batch);
 
-  /// Codegen ProcessScratchBatch(). Stores the resulting function in
-  /// 'process_scratch_batch_fn' if codegen was successful or NULL otherwise.
-  static Status Codegen(HdfsScanPlanNode* node, RuntimeState* state,
-      llvm::Function** process_scratch_batch_fn);
-
   /// Helper function to create ColumnStatsReader object. 'col_order' might be NULL.
   ColumnStatsReader CreateColumnStatsReader(
       const parquet::ColumnChunk& col_chunk, const ColumnType& col_type,
@@ -362,9 +356,6 @@ class HdfsParquetScanner : public HdfsColumnarScanner {
   /// of the column.
   ParquetTimestampDecoder CreateTimestampDecoder(const parquet::SchemaElement& element);
 
-  /// Class name in LLVM IR.
-  static const char* LLVM_CLASS_NAME;
-
  private:
   friend class ParquetColumnReader;
   friend class CollectionColumnReader;