ARROW-3469: [Gandiva] remove mutable refs in engine

apache · Oct 24, 2018 · cfc60be · cfc60be
1 parent 1929e95
commit cfc60be
Show file tree

Hide file tree

Showing 17 changed files with 510 additions and 275 deletions.
diff --git a/.travis.yml b/.travis.yml
@@ -46,10 +46,103 @@ before_install:
 
 matrix:
   fast_finish: true
+  allow_failures:
+  - jdk: oraclejdk9
   include:
+  # C++ & Python w/ clang 6.0
+  - compiler: gcc
+    language: cpp
+    os: linux
+    jdk: openjdk8
+    env:
+    - ARROW_TRAVIS_USE_TOOLCHAIN=1
+    - ARROW_TRAVIS_VALGRIND=1
+    - ARROW_TRAVIS_PLASMA=1
+    - ARROW_TRAVIS_ORC=1
+    - ARROW_TRAVIS_CLANG_FORMAT=1
+    - ARROW_TRAVIS_COVERAGE=1
+    - ARROW_TRAVIS_PARQUET=1
+    - ARROW_TRAVIS_PYTHON_DOCS=1
+    - ARROW_BUILD_WARNING_LEVEL=CHECKIN
+    - ARROW_TRAVIS_PYTHON_JVM=1
+    - ARROW_TRAVIS_JAVA_BUILD_ONLY=1
+    # ARROW-2999 Benchmarks are disabled in Travis CI for the time being
+    # - ARROW_TRAVIS_PYTHON_BENCHMARKS=1
+    - CC="clang-6.0"
+    - CXX="clang++-6.0"
+    before_script:
+    # Always run RAT checks, in case another build in matrix breaks RAT
+    - $TRAVIS_BUILD_DIR/ci/travis_release_audit.sh
+    # (ARROW_CI_CPP_AFFECTED implies ARROW_CI_PYTHON_AFFECTED)
+    - if [ $ARROW_CI_PYTHON_AFFECTED != "1" ]; then exit; fi
+    - $TRAVIS_BUILD_DIR/ci/travis_install_linux.sh
+    - $TRAVIS_BUILD_DIR/ci/travis_install_clang_tools.sh
+    - $TRAVIS_BUILD_DIR/ci/travis_lint.sh
+    # If either C++ or Python changed, we must install the C++ libraries
+    - git submodule update --init
+    - $TRAVIS_BUILD_DIR/ci/travis_before_script_cpp.sh
+    script:
+    # All test steps are required for accurate C++ coverage info
+    - $TRAVIS_BUILD_DIR/ci/travis_script_cpp.sh
+    # Build Arrow Java to test the pyarrow<->JVM in-process bridge
+    - $TRAVIS_BUILD_DIR/ci/travis_script_java.sh
+    # Only run Plasma tests with valgrind in one of the Python builds because
+    # they are slow
+    - export PLASMA_VALGRIND=0
+    - $TRAVIS_BUILD_DIR/ci/travis_script_python.sh 2.7
+    - export PLASMA_VALGRIND=1
+    - $TRAVIS_BUILD_DIR/ci/travis_script_python.sh 3.6
+    - $TRAVIS_BUILD_DIR/ci/travis_upload_cpp_coverage.sh
+  # Gandiva C++ w/ gcc 4.9 and Java
+  - compiler: gcc
+    language: cpp
+    os: linux
+    jdk: openjdk8
+    env:
+    - ARROW_TRAVIS_GANDIVA=1
+    - ARROW_TRAVIS_USE_TOOLCHAIN=1
+    - ARROW_TRAVIS_VALGRIND=1
+    - ARROW_TRAVIS_CLANG_FORMAT=1
+    - ARROW_BUILD_WARNING_LEVEL=CHECKIN
+    - MATRIX_EVAL="CC=gcc-4.9 && CXX=g++-4.9"
+    before_script:
+    # Run if something changed in CPP or Java.
+    - if [ $ARROW_CI_CPP_AFFECTED != "1" ] && [ $ARROW_CI_JAVA_AFFECTED != "1" ]; then exit; fi
+    - $TRAVIS_BUILD_DIR/ci/travis_install_linux.sh
+    - $TRAVIS_BUILD_DIR/ci/travis_install_clang_tools.sh
+    - $TRAVIS_BUILD_DIR/ci/travis_lint.sh
+    # If either C++ or Python changed, we must install the C++ libraries
+    - git submodule update --init
+    - $TRAVIS_BUILD_DIR/ci/travis_before_script_cpp.sh --only-library
+    script:
+    - $TRAVIS_BUILD_DIR/ci/travis_script_gandiva_cpp.sh
+    - $TRAVIS_BUILD_DIR/ci/travis_script_gandiva_java.sh
+  # [OS X] C++ & Python w/ XCode 6.4
+  - compiler: clang
+    language: cpp
+    osx_image: xcode6.4
+    os: osx
+    cache:
+    addons:
+    env:
+    - ARROW_TRAVIS_USE_TOOLCHAIN=1
+    - ARROW_TRAVIS_PLASMA=1
+    - ARROW_TRAVIS_ORC=1
+    - ARROW_TRAVIS_PARQUET=1
+    - ARROW_BUILD_WARNING_LEVEL=CHECKIN
+    before_script:
+    - if [ $ARROW_CI_PYTHON_AFFECTED != "1" ]; then exit; fi
+    # If either C++ or Python changed, we must install the C++ libraries
+    - git submodule update --init
+    - $TRAVIS_BUILD_DIR/ci/travis_before_script_cpp.sh
+    script:
+    - if [ $ARROW_CI_CPP_AFFECTED == "1" ]; then $TRAVIS_BUILD_DIR/ci/travis_script_cpp.sh; fi
+    - $TRAVIS_BUILD_DIR/ci/travis_script_python.sh 2.7
+    - $TRAVIS_BUILD_DIR/ci/travis_script_python.sh 3.6
   # [OS X] Gandiva C++ w/ XCode 8.3 & Java
   - compiler: clang
     language: cpp
+    # xcode 7.3 has a bug in strptime.
     osx_image: xcode8.3
     os: osx
     cache:
@@ -67,6 +160,160 @@ matrix:
     script:
     - $TRAVIS_BUILD_DIR/ci/travis_script_gandiva_cpp.sh
     - $TRAVIS_BUILD_DIR/ci/travis_script_gandiva_java.sh
+  # [manylinux1] Python
+  - language: cpp
+    before_script:
+    - if [ $ARROW_CI_PYTHON_AFFECTED == "1" ]; then docker pull quay.io/xhochy/arrow_manylinux1_x86_64_base:latest; fi
+    script:
+    - if [ $ARROW_CI_PYTHON_AFFECTED == "1" ]; then $TRAVIS_BUILD_DIR/ci/travis_script_manylinux.sh; fi
+  # Java w/ OpenJDK 8
+  - language: java
+    os: linux
+    jdk: openjdk8
+    before_script:
+    - if [ $ARROW_CI_JAVA_AFFECTED != "1" ]; then exit; fi
+    - $TRAVIS_BUILD_DIR/ci/travis_install_linux.sh
+    script:
+    - $TRAVIS_BUILD_DIR/ci/travis_script_java.sh
+    - $TRAVIS_BUILD_DIR/ci/travis_script_javadoc.sh
+  # Java w/ Oracle JDK 9
+  - language: java
+    os: linux
+    jdk: oraclejdk9
+    before_script:
+    - if [ $ARROW_CI_JAVA_AFFECTED != "1" ]; then exit; fi
+    script:
+    - $TRAVIS_BUILD_DIR/ci/travis_script_java.sh
+    addons:
+      apt:
+        packages:
+          - oracle-java9-installer
+  # Integration w/ OpenJDK 8
+  - language: java
+    os: linux
+    env: ARROW_TEST_GROUP=integration
+    jdk: openjdk8
+    env:
+    - ARROW_TRAVIS_PLASMA=1
+    - ARROW_TRAVIS_PLASMA_JAVA_CLIENT=1
+    - CC="clang-6.0"
+    - CXX="clang++-6.0"
+    before_script:
+    - if [ $ARROW_CI_INTEGRATION_AFFECTED != "1" ]; then exit; fi
+    - $TRAVIS_BUILD_DIR/ci/travis_install_linux.sh
+    - $TRAVIS_BUILD_DIR/ci/travis_install_clang_tools.sh
+    - nvm install 10.1
+    - $TRAVIS_BUILD_DIR/ci/travis_before_script_js.sh
+    - $TRAVIS_BUILD_DIR/ci/travis_before_script_cpp.sh
+    script:
+    - $TRAVIS_BUILD_DIR/ci/travis_script_integration.sh
+    - $TRAVIS_BUILD_DIR/ci/travis_script_plasma_java_client.sh
+  # NodeJS
+  - language: node_js
+    os: linux
+    node_js:
+    - '10.1'
+    before_script:
+    - if [ $ARROW_CI_JS_AFFECTED != "1" ]; then exit; fi
+    - $TRAVIS_BUILD_DIR/ci/travis_install_linux.sh
+    - $TRAVIS_BUILD_DIR/ci/travis_before_script_js.sh
+    script:
+    - $TRAVIS_BUILD_DIR/ci/travis_script_js.sh
+  # C++ & GLib & Ruby w/ gcc 4.9
+  - compiler: gcc
+    language: cpp
+    os: linux
+    env:
+    - ARROW_TRAVIS_ORC=1
+    - ARROW_TRAVIS_PARQUET=1
+    - BUILD_TORCH_EXAMPLE=no
+    - CC="gcc-4.9"
+    - CXX="g++-4.9"
+    before_script:
+    - if [ $ARROW_CI_RUBY_AFFECTED != "1" ]; then exit; fi
+    - $TRAVIS_BUILD_DIR/ci/travis_install_linux.sh
+    - $TRAVIS_BUILD_DIR/ci/travis_before_script_cpp.sh --only-library
+    - $TRAVIS_BUILD_DIR/ci/travis_before_script_c_glib.sh
+    - $TRAVIS_BUILD_DIR/ci/travis_before_script_ruby.sh
+    script:
+    - if [ $ARROW_CI_C_GLIB_AFFECTED = "1" ]; then $TRAVIS_BUILD_DIR/ci/travis_script_c_glib.sh; fi
+    - $TRAVIS_BUILD_DIR/ci/travis_script_ruby.sh
+  # [OS X] C++ & GLib & Ruby w/ XCode 8.3 & homebrew
+  - compiler: clang
+    osx_image: xcode8.3
+    os: osx
+    env:
+    - ARROW_TRAVIS_ORC=1
+    cache:
+    addons:
+    rvm: 2.2
+    before_script:
+    - if [ $ARROW_CI_RUBY_AFFECTED != "1" ]; then exit; fi
+    - $TRAVIS_BUILD_DIR/ci/travis_install_osx.sh
+    - $TRAVIS_BUILD_DIR/ci/travis_before_script_cpp.sh --only-library --homebrew
+    - $TRAVIS_BUILD_DIR/ci/travis_before_script_c_glib.sh
+    - $TRAVIS_BUILD_DIR/ci/travis_before_script_ruby.sh
+    script:
+    - if [ $ARROW_CI_C_GLIB_AFFECTED = "1" ]; then $TRAVIS_BUILD_DIR/ci/travis_script_c_glib.sh; fi
+    - $TRAVIS_BUILD_DIR/ci/travis_script_ruby.sh
+  # Rust
+  - language: rust
+    cache: cargo
+    addons:
+      apt:
+        packages:
+          - libcurl4-openssl-dev
+          - libelf-dev
+          - libdw-dev
+          - binutils-dev
+          - cmake
+    before_script:
+    - if [ $ARROW_CI_RUST_AFFECTED != "1" ]; then exit; fi
+    - $TRAVIS_BUILD_DIR/ci/travis_install_cargo.sh
+    script:
+    - RUSTUP_TOOLCHAIN=stable $TRAVIS_BUILD_DIR/ci/travis_script_rust.sh
+    - RUSTUP_TOOLCHAIN=nightly $TRAVIS_BUILD_DIR/ci/travis_script_rust.sh || true
+    after_success:
+    - pushd ${TRAVIS_BUILD_DIR}/rust
+    # Run coverage for codecov.io
+    - mkdir -p target/kcov
+    - RUST_BACKTRACE=1 RUSTUP_TOOLCHAIN=stable cargo coverage --verbose
+    - bash <(curl -s https://codecov.io/bash) || echo "Codecov did not collect coverage reports"
+  # Go
+  - language: go
+    go_import_path: github.com/apache/arrow
+    os: linux
+    go:
+    - 1.11.x
+    before_script:
+    - if [ $ARROW_CI_GO_AFFECTED != "1" ]; then exit; fi
+    script:
+    - $TRAVIS_BUILD_DIR/ci/travis_script_go.sh
+    after_success:
+    - pushd ${TRAVIS_BUILD_DIR}/go/arrow
+    - bash <(curl -s https://codecov.io/bash) || echo "Codecov did not collect coverage reports"
+  # R
+  - language: r
+    cache: packages
+    latex: false
+    before_install:
+    # Have to copy-paste this here because of how R's build steps work
+    - eval `python $TRAVIS_BUILD_DIR/ci/detect-changes.py`
+    - if [ $ARROW_CI_R_AFFECTED != "1" ]; then exit; fi
+    - |
+        if [ $TRAVIS_OS_NAME == "linux" ]; then
+          sudo bash -c "echo -e 'Acquire::Retries 10; Acquire::http::Timeout \"20\";' > /etc/apt/apt.conf.d/99-travis-retry"
+          sudo add-apt-repository -y ppa:ubuntu-toolchain-r/test
+          sudo apt-get update -qq
+        fi
+    - $TRAVIS_BUILD_DIR/ci/travis_install_linux.sh
+    - $TRAVIS_BUILD_DIR/ci/travis_before_script_cpp.sh --only-library
+    - $TRAVIS_BUILD_DIR/ci/travis_install_clang_tools.sh
+    - $TRAVIS_BUILD_DIR/ci/travis_lint.sh
+    - export LD_LIBRARY_PATH=$LD_LIBRARY_PATH:$TRAVIS_BUILD_DIR/cpp-install/lib
+    - export PKG_CONFIG_PATH=$PKG_CONFIG_PATH:$TRAVIS_BUILD_DIR/cpp-install/lib/pkgconfig
+    - pushd ${TRAVIS_BUILD_DIR}/r
+
 
 after_failure:
 - |

diff --git a/cpp/cmake_modules/SetupCxxFlags.cmake b/cpp/cmake_modules/SetupCxxFlags.cmake
@@ -113,8 +113,6 @@ if ("${UPPERCASE_BUILD_WARNING_LEVEL}" STREQUAL "CHECKIN")
     if ("${COMPILER_VERSION}" VERSION_GREATER "3.6")
       set(CXX_COMMON_FLAGS "${CXX_COMMON_FLAGS} -Wno-reserved-id-macro")
       set(CXX_COMMON_FLAGS "${CXX_COMMON_FLAGS} -Wno-range-loop-analysis")
-    endif()
-    if ("${COMPILER_VERSION}" VERSION_GREATER "3.7")
       set(CXX_COMMON_FLAGS "${CXX_COMMON_FLAGS} -Wno-double-promotion")
     endif()
     if ("${COMPILER_VERSION}" VERSION_GREATER "3.8")

diff --git a/cpp/src/gandiva/context_helper.cc b/cpp/src/gandiva/context_helper.cc
@@ -25,16 +25,17 @@
 
 namespace gandiva {
 
-void ExportedContextFunctions::AddMappings(Engine& engine) const {
+void ExportedContextFunctions::AddMappings(Engine* engine) const {
   std::vector<llvm::Type*> args;
-  auto types = engine.types();
+  auto types = engine->types();
 
   // gdv_fn_context_set_error_msg
-  args = {types.i64_type(),      // int64_t context_ptr
-          types.i8_ptr_type()};  // char const* err_msg
+  args = {types->i64_type(),      // int64_t context_ptr
+          types->i8_ptr_type()};  // char const* err_msg
 
-  engine.AddGlobalMappingForFunc("gdv_fn_context_set_error_msg", types.void_type(), args,
-                                 reinterpret_cast<void*>(gdv_fn_context_set_error_msg));
+  engine->AddGlobalMappingForFunc("gdv_fn_context_set_error_msg", types->void_type(),
+                                  args,
+                                  reinterpret_cast<void*>(gdv_fn_context_set_error_msg));
 }
 
 }  // namespace gandiva

diff --git a/cpp/src/gandiva/engine.cc b/cpp/src/gandiva/engine.cc
@@ -221,7 +221,7 @@ void Engine::AddGlobalMappingForFunc(const std::string& name, llvm::Type* ret_ty
   execution_engine_->addGlobalMapping(fn, function_ptr);
 }
 
-void Engine::AddGlobalMappings() { ExportedFuncsRegistry::AddMappings(*this); }
+void Engine::AddGlobalMappings() { ExportedFuncsRegistry::AddMappings(this); }
 
 void Engine::DumpIR(std::string prefix) {
   std::string str;

diff --git a/cpp/src/gandiva/engine.h b/cpp/src/gandiva/engine.h
@@ -41,8 +41,8 @@ namespace gandiva {
 class Engine {
  public:
   llvm::LLVMContext* context() { return context_.get(); }
-  llvm::IRBuilder<>& ir_builder() { return *ir_builder_.get(); }
-
+  llvm::IRBuilder<>* ir_builder() { return ir_builder_.get(); }
+  LLVMTypes* types() { return types_.get(); }
   llvm::Module* module() { return module_; }
 
   /// factory method to create and initialize the engine object.
@@ -64,8 +64,6 @@ class Engine {
   /// Get the compiled function corresponding to the irfunction.
   void* CompiledFunction(llvm::Function* irFunction);
 
-  LLVMTypes& types() { return *types_; }
-
   // Create and add a mapping for the cpp function to make it accessible from LLVM.
   void AddGlobalMappingForFunc(const std::string& name, llvm::Type* ret_type,
                                const std::vector<llvm::Type*>& args, void* func);

diff --git a/cpp/src/gandiva/engine_llvm_test.cc b/cpp/src/gandiva/engine_llvm_test.cc
@@ -30,7 +30,7 @@ class TestEngine : public ::testing::Test {
 };
 
 llvm::Function* TestEngine::BuildVecAdd(Engine* engine, LLVMTypes* types) {
-  llvm::IRBuilder<>& builder = engine->ir_builder();
+  llvm::IRBuilder<>* builder = engine->ir_builder();
   llvm::LLVMContext* context = engine->context();
 
   // Create fn prototype :
@@ -62,39 +62,39 @@ llvm::Function* TestEngine::BuildVecAdd(Engine* engine, LLVMTypes* types) {
   llvm::BasicBlock* loop_exit = llvm::BasicBlock::Create(*context, "exit", fn);
 
   // Loop entry
-  builder.SetInsertPoint(loop_entry);
-  builder.CreateBr(loop_body);
+  builder->SetInsertPoint(loop_entry);
+  builder->CreateBr(loop_body);
 
   // Loop body
-  builder.SetInsertPoint(loop_body);
+  builder->SetInsertPoint(loop_body);
 
-  llvm::PHINode* loop_var = builder.CreatePHI(types->i32_type(), 2, "loop_var");
-  llvm::PHINode* sum = builder.CreatePHI(types->i64_type(), 2, "sum");
+  llvm::PHINode* loop_var = builder->CreatePHI(types->i32_type(), 2, "loop_var");
+  llvm::PHINode* sum = builder->CreatePHI(types->i64_type(), 2, "sum");
 
   loop_var->addIncoming(types->i32_constant(0), loop_entry);
   sum->addIncoming(types->i64_constant(0), loop_entry);
 
   // setup loop PHI
   llvm::Value* loop_update =
-      builder.CreateAdd(loop_var, types->i32_constant(1), "loop_var+1");
+      builder->CreateAdd(loop_var, types->i32_constant(1), "loop_var+1");
   loop_var->addIncoming(loop_update, loop_body);
 
   // get the current value
-  llvm::Value* offset = builder.CreateGEP(arg_elements, loop_var, "offset");
-  llvm::Value* current_value = builder.CreateLoad(offset, "value");
+  llvm::Value* offset = builder->CreateGEP(arg_elements, loop_var, "offset");
+  llvm::Value* current_value = builder->CreateLoad(offset, "value");
 
   // setup sum PHI
-  llvm::Value* sum_update = builder.CreateAdd(sum, current_value, "sum+ith");
+  llvm::Value* sum_update = builder->CreateAdd(sum, current_value, "sum+ith");
   sum->addIncoming(sum_update, loop_body);
 
   // check loop_var
   llvm::Value* loop_var_check =
-      builder.CreateICmpSLT(loop_update, arg_nelements, "loop_var < nrec");
-  builder.CreateCondBr(loop_var_check, loop_body, loop_exit);
+      builder->CreateICmpSLT(loop_update, arg_nelements, "loop_var < nrec");
+  builder->CreateCondBr(loop_var_check, loop_body, loop_exit);
 
   // Loop exit
-  builder.SetInsertPoint(loop_exit);
-  builder.CreateRet(sum_update);
+  builder->SetInsertPoint(loop_exit);
+  builder->CreateRet(sum_update);
   return fn;
 }