GDV-43: [C++] Introduce error codes as error handling strategy. (apac…

…he#8) * GDV-43: [C++] Introduce error codes as error handling strategy. Introduced status codes and using the same as the error handling strategy. The decision was taken to accommodate existing libraries that use error codes and because Arrow also uses error codes and not exception. Changed the signatures across the board for the same.
praveenbingo · Sep 10, 2018 · 164a4ec · 164a4ec
1 parent 5b86c10
commit 164a4ec
Show file tree

Hide file tree

Showing 15 changed files with 503 additions and 174 deletions.
diff --git a/cpp/src/gandiva/codegen/CMakeLists.txt b/cpp/src/gandiva/codegen/CMakeLists.txt
@@ -31,6 +31,7 @@ add_library(gandiva SHARED
   llvm_types.cc
   node.cc
   tree_expr_builder.cc
+  status.cc
   ${BC_FILE_PATH_CC})
 
 # For users of gandiva library (including integ tests), include-dir is :
@@ -85,12 +86,13 @@ install(
 
 #args: label test-file src-files
 add_gandiva_unit_test(dex_llvm_test.cc)
-add_gandiva_unit_test(engine_llvm_test.cc engine.cc llvm_types.cc ${BC_FILE_PATH_CC})
+add_gandiva_unit_test(engine_llvm_test.cc engine.cc llvm_types.cc status.cc ${BC_FILE_PATH_CC})
 add_gandiva_unit_test(function_signature_test.cc)
 add_gandiva_unit_test(function_registry_test.cc function_registry.cc)
 add_gandiva_unit_test(llvm_types_test.cc llvm_types.cc)
-add_gandiva_unit_test(llvm_generator_test.cc llvm_generator.cc engine.cc llvm_types.cc function_registry.cc annotator.cc ${BC_FILE_PATH_CC})
+add_gandiva_unit_test(llvm_generator_test.cc llvm_generator.cc engine.cc llvm_types.cc function_registry.cc annotator.cc status.cc ${BC_FILE_PATH_CC})
 add_gandiva_unit_test(annotator_test.cc annotator.cc)
 add_gandiva_unit_test(tree_expr_test.cc tree_expr_builder.cc node.cc annotator.cc function_registry.cc)
+add_gandiva_unit_test(status_test.cc status.cc)
 
 add_gandiva_integ_test(evaluator_test.cc)
diff --git a/cpp/src/gandiva/codegen/codegen_exception.h b/cpp/src/gandiva/codegen/codegen_exception.h
diff --git a/cpp/src/gandiva/codegen/engine.cc b/cpp/src/gandiva/codegen/engine.cc
@@ -34,7 +34,6 @@
 #include <string>
 #include <unordered_set>
 #include <utility>
-#include "codegen/codegen_exception.h"
 #include "codegen/engine.h"
 
 namespace gandiva {
@@ -58,41 +57,47 @@ void Engine::InitOnce() {
   init_once_done_ = true;
 }
 
-Engine::Engine()
-    : module_finalized_(false) {
-  std::call_once(init_once_flag, InitOnce);
-  context_.reset(new llvm::LLVMContext());
-  ir_builder_.reset(new llvm::IRBuilder<>(*context()));
+/// factory method to construct the engine.
+Status Engine::Make(std::unique_ptr<Engine> *engine) {
+  std::unique_ptr<Engine> engine_obj(new Engine());
+
+  std::call_once(init_once_flag, [&engine_obj] {engine_obj->InitOnce();});
+  engine_obj->context_.reset(new llvm::LLVMContext());
+  engine_obj->ir_builder_.reset(new llvm::IRBuilder<>(*(engine_obj->context())));
 
   /* Create the execution engine */
-  std::unique_ptr<llvm::Module> cg_module(new llvm::Module("codegen", *context()));
-  module_ = cg_module.get();
+  std::unique_ptr<llvm::Module> cg_module(new llvm::Module("codegen",
+                                          *(engine_obj->context())));
+  engine_obj->module_ = cg_module.get();
 
   llvm::EngineBuilder engineBuilder(std::move(cg_module));
   engineBuilder.setEngineKind(llvm::EngineKind::JIT);
   engineBuilder.setOptLevel(llvm::CodeGenOpt::Aggressive);
-  engineBuilder.setErrorStr(&llvm_error_);
-  execution_engine_.reset(engineBuilder.create());
-  if (execution_engine_ == NULL) {
-    module_ = NULL;
-    throw CodeGenException(llvm_error_);
+  engineBuilder.setErrorStr(&(engine_obj->llvm_error_));
+  engine_obj->execution_engine_.reset(engineBuilder.create());
+  if (engine_obj->execution_engine_ == NULL) {
+    engine_obj->module_ = NULL;
+    return Status::CodeGenError(engine_obj->llvm_error_);
   }
 
-  LoadPreCompiledIRFiles();
+  Status result = engine_obj->LoadPreCompiledIRFiles();
+  GANDIVA_RETURN_NOT_OK(result);
+  *engine = std::move(engine_obj);
+  return Status::OK();
 }
 
 /*
  * Handling for pre-compiled IR libraries.
  */
-void Engine::LoadPreCompiledIRFiles() {
+Status Engine::LoadPreCompiledIRFiles() {
   /// Read from file into memory buffer.
   llvm::ErrorOr<std::unique_ptr<llvm::MemoryBuffer>> buffer_or_error =
       llvm::MemoryBuffer::getFile(kByteCodeFilePath);
   if (!buffer_or_error) {
     std::stringstream ss;
     ss << "Could not load module from IR " << kByteCodeFilePath << ": " <<
           buffer_or_error.getError().message();
-    throw CodeGenException(ss.str());
+    return Status::CodeGenError(ss.str());
   }
   std::unique_ptr<llvm::MemoryBuffer> buffer = move(buffer_or_error.get());
 
@@ -104,26 +109,26 @@ void Engine::LoadPreCompiledIRFiles() {
     llvm::handleAllErrors(module_or_error.takeError(), [&](llvm::ErrorInfoBase &eib) {
       error_string = eib.message();
     });
-    throw CodeGenException(error_string);
+    return Status::CodeGenError(error_string);
   }
   std::unique_ptr<llvm::Module> ir_module = move(module_or_error.get());
 
   /// Verify the IR module
   if (llvm::verifyModule(*ir_module.get(), &llvm::errs())) {
-    throw CodeGenException("verify of IR Module failed");
+    return Status::CodeGenError("verify of IR Module failed");
   }
 
   // Link this to the primary module.
   if (llvm::Linker::linkModules(*module_, move(ir_module))) {
-    throw CodeGenException("failed to link IR Modules");
+    return Status::CodeGenError("failed to link IR Modules");
   }
+  return Status::OK();
 }
 
 /*
  * Optimise and compile the module.
  */
-void
-Engine::FinalizeModule(bool optimise_ir, bool dump_ir) {
+Status Engine::FinalizeModule(bool optimise_ir, bool dump_ir) {
   if (dump_ir) {
     DumpIR("Before optimise");
   }
@@ -178,16 +183,17 @@ Engine::FinalizeModule(bool optimise_ir, bool dump_ir) {
   }
 
   if (llvm::verifyModule(*module_, &llvm::errs())) {
-    throw CodeGenException("verify of module failed after optimisation passes");
+    return Status::CodeGenError("verify of module failed after optimisation passes");
   }
 
   // do the compilation
   execution_engine_->finalizeObject();
   module_finalized_ = true;
+  return Status::OK();
 }
 
 void *Engine::CompiledFunction(llvm::Function *irFunction) {
-  assert(module_finalized_);
+  DCHECK(module_finalized_);
   return execution_engine_->getPointerToFunction(irFunction);
 }
 

diff --git a/cpp/src/gandiva/codegen/engine.h b/cpp/src/gandiva/codegen/engine.h
@@ -24,19 +24,24 @@
 #include <string>
 #include <vector>
 #include "gandiva/logging.h"
+#include "gandiva/status.h"
 
 namespace gandiva {
 
 /// \brief LLVM Execution engine wrapper.
 class Engine {
  public:
-  Engine();
-
   llvm::LLVMContext *context() { return context_.get(); }
   llvm::IRBuilder<> &ir_builder() { return *ir_builder_.get(); }
 
   llvm::Module *module() { return module_; }
 
+  /// factory method to create and initialize the engine
+  /// object.
+  ///
+  /// @param engine (out) : the created engine.
+  static Status Make(std::unique_ptr<Engine> *engine);
+
   /// Add the function to the list of IR functions that need to be compiled.
   /// Compiling only the functions that are used by the module saves time.
   void AddFunctionToCompile(const std::string &fname) {
@@ -45,29 +50,33 @@ class Engine {
   }
 
   /// Optimise and compile the module.
-  void FinalizeModule(bool optimise_ir, bool dump_ir);
+  Status FinalizeModule(bool optimise_ir, bool dump_ir);
 
   /// Get the compiled function corresponding to the irfunction.
   void *CompiledFunction(llvm::Function *irFunction);
 
  private:
-  // do one time inits.
+  /// private constructor to ensure engine is created
+  /// only through the factory.
+  Engine() : module_finalized_(false) {}
+
+  /// do one time inits.
   static void InitOnce();
   static bool init_once_done_;
 
   llvm::ExecutionEngine &execution_engine() { return *execution_engine_.get(); }
 
-  // load pre-compiled modules and merge them into the main module.
-  void LoadPreCompiledIRFiles();
+  /// load pre-compiled modules and merge them into the main module.
+  Status LoadPreCompiledIRFiles();
 
-  // dump the IR code to stdout with the prefix string.
+  /// dump the IR code to stdout with the prefix string.
   void DumpIR(std::string prefix);
 
   std::unique_ptr<llvm::LLVMContext> context_;
   std::unique_ptr<llvm::ExecutionEngine> execution_engine_;
   std::unique_ptr<llvm::IRBuilder<>> ir_builder_;
-  llvm::Module *module_; // This is owned by the execution_engine_, so doesn't need to be
-                         // explicitly deleted.
+  llvm::Module *module_; /// This is owned by the execution_engine_, so doesn't need to be
+                         /// explicitly deleted.
 
   std::vector<std::string> functions_to_compile_;
 

diff --git a/cpp/src/gandiva/codegen/engine_llvm_test.cc b/cpp/src/gandiva/codegen/engine_llvm_test.cc
@@ -17,7 +17,6 @@
 #include <gtest/gtest.h>
 #include "codegen/engine.h"
 #include "codegen/llvm_types.h"
-#include "codegen/codegen_exception.h"
 
 namespace gandiva {
 
@@ -103,26 +102,28 @@ llvm::Function *TestEngine::BuildVecAdd(Engine *engine, LLVMTypes *types) {
 }
 
 TEST_F(TestEngine, TestAddUnoptimised) {
-  Engine engine;
-  LLVMTypes types(*engine.context());
-  llvm::Function *ir_func = BuildVecAdd(&engine, &types);
-  engine.FinalizeModule(false, false);
+  std::unique_ptr<Engine> engine;
+  Engine::Make(&engine);
+  LLVMTypes types(*engine->context());
+  llvm::Function *ir_func = BuildVecAdd(engine.get(), &types);
+  engine->FinalizeModule(false, false);
 
   add_vector_func_t add_func =
-      reinterpret_cast<add_vector_func_t>(engine.CompiledFunction(ir_func));
+      reinterpret_cast<add_vector_func_t>(engine->CompiledFunction(ir_func));
 
   int64_t my_array[] = {1, 3, -5, 8, 10};
   EXPECT_EQ(add_func(my_array, 5), 17);
 }
 
 TEST_F(TestEngine, TestAddOptimised) {
-  Engine engine;
-  LLVMTypes types(*engine.context());
-  llvm::Function *ir_func = BuildVecAdd(&engine, &types);
-  engine.FinalizeModule(true, false);
+  std::unique_ptr<Engine> engine;
+  Engine::Make(&engine);
+  LLVMTypes types(*engine->context());
+  llvm::Function *ir_func = BuildVecAdd(engine.get(), &types);
+  engine->FinalizeModule(true, false);
 
   add_vector_func_t add_func =
-      reinterpret_cast<add_vector_func_t>(engine.CompiledFunction(ir_func));
+      reinterpret_cast<add_vector_func_t>(engine->CompiledFunction(ir_func));
 
   int64_t my_array[] = {1, 3, -5, 8, 10};
   EXPECT_EQ(add_func(my_array, 5), 17);

diff --git a/cpp/src/gandiva/codegen/evaluator.cc b/cpp/src/gandiva/codegen/evaluator.cc
@@ -31,15 +31,17 @@ Evaluator::Evaluator(std::unique_ptr<LLVMGenerator> llvm_generator,
     output_fields_(output_fields),
     pool_(pool) {}
 
-// TODO : exceptions
-std::shared_ptr<Evaluator> Evaluator::Make(SchemaPtr schema,
-                                           const ExpressionVector &exprs,
-                                           arrow::MemoryPool *pool) {
+Status Evaluator::Make(SchemaPtr schema,
+                       const ExpressionVector &exprs,
+                       arrow::MemoryPool *pool,
+                       std::shared_ptr<Evaluator> *evaluator) {
   // TODO: validate schema
   // TODO : validate expressions (fields, function signatures, output types, ..)
 
   // Build LLVM generator, and generate code for the specified expressions
-  std::unique_ptr<LLVMGenerator> llvm_gen(new LLVMGenerator());
+  std::unique_ptr<LLVMGenerator> llvm_gen;
+  Status status = LLVMGenerator::Make(&llvm_gen);
+  GANDIVA_RETURN_NOT_OK(status);
   llvm_gen->Build(exprs);
 
   // save the output field types. Used for validation at Evaluate() time.
@@ -49,10 +51,11 @@ std::shared_ptr<Evaluator> Evaluator::Make(SchemaPtr schema,
   }
 
   // Instantiate the evaluator with the completely built llvm generator
-  return std::shared_ptr<Evaluator>(new Evaluator(std::move(llvm_gen),
-                                                  schema,
-                                                  output_fields,
-                                                  pool));
+  *evaluator = std::shared_ptr<Evaluator>(new Evaluator(std::move(llvm_gen),
+                                                        schema,
+                                                        output_fields,
+                                                        pool));
+  return Status::OK();
 }
 
 arrow::ArrayVector Evaluator::Evaluate(const arrow::RecordBatch &batch) {

diff --git a/cpp/src/gandiva/codegen/evaluator.h b/cpp/src/gandiva/codegen/evaluator.h
@@ -21,6 +21,7 @@
 #include <vector>
 #include "gandiva/arrow.h"
 #include "gandiva/expression.h"
+#include "gandiva/status.h"
 
 namespace gandiva {
 
@@ -33,9 +34,10 @@ class LLVMGenerator;
 class Evaluator {
  public:
   /// Build an evaluator for the given schema to evaluate the vector of expressions.
-  static std::shared_ptr<Evaluator> Make(SchemaPtr schema,
-                                         const ExpressionVector &exprs,
-                                         arrow::MemoryPool *pool);
+  static Status Make(SchemaPtr schema,
+                     const ExpressionVector &exprs,
+                     arrow::MemoryPool *pool,
+                     std::shared_ptr<Evaluator> *evaluator);
 
   /// Evaluate the specified record batch, and fill the output vectors.
   /// TODO : need a zero-copy variant if the caller can alloc the output vectors.