diff --git a/.travis.yml b/.travis.yml index 09487f90f208..24fde86ab057 100644 --- a/.travis.yml +++ b/.travis.yml @@ -33,6 +33,7 @@ git: before_install: # Common pre-install steps for all builds + - eval "${MATRIX_EVAL}" - ulimit -c unlimited -S - | if [ $TRAVIS_OS_NAME == "linux" ]; then @@ -92,6 +93,29 @@ matrix: - 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 + - compiler: gcc + language: cpp + os: linux + jdk: openjdk8 + env: + - ARROW_TRAVIS_USE_TOOLCHAIN=1 + - ARROW_TRAVIS_VALGRIND=1 + - ARROW_TRAVIS_CLANG_FORMAT=1 + - ARROW_BUILD_WARNING_LEVEL=CHECKIN + - ARROW_TRAVIS_GANDIVA=1 + - MATRIX_EVAL="CC=gcc-4.9 && CXX=g++-4.9" + before_script: + # Run if something changed in CPP. + - if [ $ARROW_CI_CPP_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.sh # [OS X] C++ & Python w/ XCode 6.4 - compiler: clang language: cpp diff --git a/ci/travis_before_script_cpp.sh b/ci/travis_before_script_cpp.sh index 1729ca4fa463..1b389a21de12 100755 --- a/ci/travis_before_script_cpp.sh +++ b/ci/travis_before_script_cpp.sh @@ -88,6 +88,10 @@ if [ $ARROW_TRAVIS_PARQUET == "1" ]; then -DPARQUET_BUILD_EXECUTABLES=ON" fi +if [ $ARROW_TRAVIS_GANDIVA == "1" ]; then + CMAKE_COMMON_FLAGS="$CMAKE_COMMON_FLAGS -DARROW_GANDIVA=ON" +fi + if [ $ARROW_TRAVIS_VALGRIND == "1" ]; then CMAKE_COMMON_FLAGS="$CMAKE_COMMON_FLAGS -DARROW_TEST_MEMCHECK=ON" fi diff --git a/ci/travis_install_linux.sh b/ci/travis_install_linux.sh index e8cd0102350f..c1db2d5c5bda 100755 --- a/ci/travis_install_linux.sh +++ b/ci/travis_install_linux.sh @@ -21,6 +21,10 @@ sudo apt-get install -y -q \ gdb binutils ccache libboost-dev libboost-filesystem-dev \ libboost-system-dev libboost-regex-dev libjemalloc-dev +if [ "$CXX" == "g++-4.9" ]; then + sudo apt-get install -y -q g++-4.9 +fi + if [ "$ARROW_TRAVIS_VALGRIND" == "1" ]; then sudo apt-get install -y -q valgrind fi diff --git a/ci/travis_install_toolchain.sh b/ci/travis_install_toolchain.sh index 405c496f8ce1..eb4bdf21d65c 100755 --- a/ci/travis_install_toolchain.sh +++ b/ci/travis_install_toolchain.sh @@ -40,5 +40,6 @@ if [ ! -e $CPP_TOOLCHAIN ]; then thrift-cpp=0.11.0 \ zlib \ glog \ - zstd + zstd \ + re2 fi diff --git a/ci/travis_script_gandiva.sh b/ci/travis_script_gandiva.sh new file mode 100755 index 000000000000..3dbd993c0a3e --- /dev/null +++ b/ci/travis_script_gandiva.sh @@ -0,0 +1,33 @@ +#!/usr/bin/env bash + +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +set -e + +source $TRAVIS_BUILD_DIR/ci/travis_env_common.sh + +pushd $CPP_BUILD_DIR + +PATH=$ARROW_BUILD_TYPE:$PATH ctest -j2 --output-on-failure -L unittest + +# not running in parallel, since some of them are benchmarks +PATH=$ARROW_BUILD_TYPE:$PATH ctest -VV -L integ + +popd + +# TODO : Capture C++ coverage info diff --git a/cpp/cmake_modules/FindRE2.cmake b/cpp/cmake_modules/FindRE2.cmake index f2a4670c6154..42aea146f121 100644 --- a/cpp/cmake_modules/FindRE2.cmake +++ b/cpp/cmake_modules/FindRE2.cmake @@ -22,6 +22,7 @@ # This module defines # RE2_INCLUDE_DIR, directory containing headers # RE2_STATIC_LIB, path to libre2.a +# RE2_SHARED_LIB, path to libre2.so # RE2_FOUND, whether re2 has been found if( NOT "${RE2_HOME}" STREQUAL "") @@ -51,9 +52,22 @@ find_library(RE2_STATIC_LIB NAMES libre2${CMAKE_STATIC_LIBRARY_SUFFIX} DOC "Google's re2 regex static library" ) +find_library(RE2_SHARED_LIB NAMES libre2${CMAKE_SHARED_LIBRARY_SUFFIX} + PATHS ${_re2_path} + NO_DEFAULT_PATH + PATH_SUFFIXES ${lib_dirs} + DOC "Google's re2 regex static library" +) + message(STATUS ${RE2_INCLUDE_DIR}) -if (NOT RE2_INCLUDE_DIR OR NOT RE2_STATIC_LIB) +if (ARROW_RE2_LINKAGE STREQUAL "static" AND (NOT RE2_STATIC_LIB)) + set(RE2_LIB_NOT_FOUND TRUE) +elseif(ARROW_RE2_LINKAGE STREQUAL "shared" AND (NOT RE2_SHARED_LIB)) + set(RE2_LIB_NOT_FOUND TRUE) +endif() + +if (NOT RE2_INCLUDE_DIR OR RE2_LIB_NOT_FOUND) set(RE2_FOUND FALSE) if (_re2_path) set (RE2_ERR_MSG "Could not find re2. Looked in ${_re2_path}.") @@ -68,11 +82,13 @@ if (NOT RE2_INCLUDE_DIR OR NOT RE2_STATIC_LIB) endif () else() set(RE2_FOUND TRUE) - message(STATUS "Found the RE2 headers : ${RE2_INCLUDE_DIR}") - message(STATUS "Found the RE2 static library : ${RE2_STATIC_LIB}") + message(STATUS "RE2 headers : ${RE2_INCLUDE_DIR}") + message(STATUS "RE2 static library : ${RE2_STATIC_LIB}") + message(STATUS "RE2 shared library : ${RE2_SHARED_LIB}") endif() mark_as_advanced( RE2_INCLUDE_DIR + RE2_SHARED_LIB RE2_STATIC_LIB ) diff --git a/cpp/cmake_modules/GandivaBuildUtils.cmake b/cpp/cmake_modules/GandivaBuildUtils.cmake index a1ed48eee834..4a069f0bb178 100644 --- a/cpp/cmake_modules/GandivaBuildUtils.cmake +++ b/cpp/cmake_modules/GandivaBuildUtils.cmake @@ -39,7 +39,7 @@ function(build_gandiva_lib TYPE ARROW) Boost::system Boost::filesystem LLVM::LLVM_INTERFACE - ${RE2_STATIC_LIB}) + re2) if (${TYPE} MATCHES "static" AND NOT APPLE) target_link_libraries(gandiva_${TYPE} @@ -102,6 +102,8 @@ function(add_precompiled_unit_test REL_TEST_NAME) get_filename_component(TEST_NAME ${REL_TEST_NAME} NAME_WE) add_executable(${TEST_NAME} ${REL_TEST_NAME} ${ARGN}) + # Require toolchain to be built + add_dependencies(${TEST_NAME} arrow_dependencies) target_include_directories(${TEST_NAME} PRIVATE ${CMAKE_SOURCE_DIR}/src) target_link_libraries(${TEST_NAME} PRIVATE ${GANDIVA_TEST_LINK_LIBS}) target_compile_definitions(${TEST_NAME} PRIVATE GANDIVA_UNIT_TEST=1) diff --git a/cpp/cmake_modules/ThirdpartyToolchain.cmake b/cpp/cmake_modules/ThirdpartyToolchain.cmake index 2e2f7d1b32b5..e6c7f7828f47 100644 --- a/cpp/cmake_modules/ThirdpartyToolchain.cmake +++ b/cpp/cmake_modules/ThirdpartyToolchain.cmake @@ -15,6 +15,11 @@ # specific language governing permissions and limitations # under the License. +# ---------------------------------------------------------------------- +# Toolchain linkage options + +set(ARROW_RE2_LINKAGE "static" CACHE STRING + "How to link the re2 library. static|shared (default static)") # ---------------------------------------------------------------------- # Thirdparty versions, environment variables, source URLs @@ -37,7 +42,8 @@ if (NOT "$ENV{ARROW_BUILD_TOOLCHAIN}" STREQUAL "") # set(ORC_HOME "$ENV{ARROW_BUILD_TOOLCHAIN}") set(PROTOBUF_HOME "$ENV{ARROW_BUILD_TOOLCHAIN}") set(RAPIDJSON_HOME "$ENV{ARROW_BUILD_TOOLCHAIN}") - set(RE2_HOME "$ENV{ARROW_BUILD_TOOLCHAIN}") + # conda-forge doesn't have a static re2. + #set(RE2_HOME "$ENV{ARROW_BUILD_TOOLCHAIN}") set(SNAPPY_HOME "$ENV{ARROW_BUILD_TOOLCHAIN}") set(THRIFT_HOME "$ENV{ARROW_BUILD_TOOLCHAIN}") set(ZLIB_HOME "$ENV{ARROW_BUILD_TOOLCHAIN}") @@ -100,6 +106,10 @@ if (DEFINED ENV{RAPIDJSON_HOME}) set(RAPIDJSON_HOME "$ENV{RAPIDJSON_HOME}") endif() +if (DEFINED ENV{RE2_HOME}) + set(RE2_HOME "$ENV{RAPIDJSON_HOME}") +endif() + if (DEFINED ENV{SNAPPY_HOME}) set(SNAPPY_HOME "$ENV{SNAPPY_HOME}") endif() @@ -1091,8 +1101,14 @@ if (ARROW_GANDIVA) endif () include_directories (SYSTEM ${RE2_INCLUDE_DIR}) - ADD_THIRDPARTY_LIB(re2 - STATIC_LIB ${RE2_STATIC_LIB}) + + if (ARROW_RE2_LINKAGE STREQUAL "shared") + ADD_THIRDPARTY_LIB(re2 + STATIC_LIB ${RE2_SHARED_LIB}) + else() + ADD_THIRDPARTY_LIB(re2 + STATIC_LIB ${RE2_STATIC_LIB}) + endif() if (RE2_VENDORED) add_dependencies (arrow_dependencies re2_ep) diff --git a/cpp/src/arrow/util/bit-util.h b/cpp/src/arrow/util/bit-util.h index a22ff0fc4b92..4bb44111f80d 100644 --- a/cpp/src/arrow/util/bit-util.h +++ b/cpp/src/arrow/util/bit-util.h @@ -325,6 +325,15 @@ static inline void ClearBit(uint8_t* bits, int64_t i) { static inline void SetBit(uint8_t* bits, int64_t i) { bits[i / 8] |= kBitmask[i % 8]; } +static inline void SetBitTo(uint8_t* bits, int64_t i, bool bit_is_set) { + // https://graphics.stanford.edu/~seander/bithacks.html + // "Conditionally set or clear bits without branching" + // NOTE: this seems to confuse Valgrind as it reads from potentially + // uninitialized memory + bits[i / 8] ^= static_cast(-static_cast(bit_is_set) ^ bits[i / 8]) & + kBitmask[i % 8]; +} + /// \brief Convert vector of bytes to bitmap buffer ARROW_EXPORT Status BytesToBits(const std::vector&, MemoryPool*, std::shared_ptr*); diff --git a/cpp/src/gandiva/CMakeLists.txt b/cpp/src/gandiva/CMakeLists.txt index 91f851fb92b6..d47a685cbbc6 100644 --- a/cpp/src/gandiva/CMakeLists.txt +++ b/cpp/src/gandiva/CMakeLists.txt @@ -20,8 +20,6 @@ cmake_minimum_required(VERSION 3.11) project(gandiva) -find_package(RE2 REQUIRED) - include(GandivaBuildUtils) find_package(LLVM) @@ -120,7 +118,7 @@ target_include_directories(gandiva_helpers ${ARROW_INCLUDE_DIR} ) -target_link_libraries(gandiva_helpers PRIVATE Boost::boost ${RE2_STATIC_LIB}) +target_link_libraries(gandiva_helpers PRIVATE Boost::boost re2) if (NOT APPLE) target_link_libraries(gandiva_helpers LINK_PRIVATE -static-libstdc++ -static-libgcc) endif() diff --git a/cpp/src/gandiva/bitmap_accumulator_test.cc b/cpp/src/gandiva/bitmap_accumulator_test.cc index c7a0b46a73af..fc89421344e8 100644 --- a/cpp/src/gandiva/bitmap_accumulator_test.cc +++ b/cpp/src/gandiva/bitmap_accumulator_test.cc @@ -38,7 +38,7 @@ void TestBitMapAccumulator::FillBitMap(uint8_t* bmap, int nrecords) { for (int i = 0; i < nbytes; ++i) { rand_r(&cur); - bmap[i] = cur % UINT8_MAX; + bmap[i] = static_cast(cur % UINT8_MAX); } } @@ -49,7 +49,7 @@ void TestBitMapAccumulator::ByteWiseIntersectBitMaps(uint8_t* dst, for (int i = 0; i < nbytes; ++i) { dst[i] = 0xff; for (uint32_t j = 0; j < srcs.size(); ++j) { - dst[i] &= srcs[j][i]; + dst[i] = dst[i] & srcs[j][i]; } } } diff --git a/cpp/src/gandiva/function_registry.cc b/cpp/src/gandiva/function_registry.cc index 2b7962477bb2..84a265f45f3f 100644 --- a/cpp/src/gandiva/function_registry.cc +++ b/cpp/src/gandiva/function_registry.cc @@ -365,7 +365,7 @@ FunctionRegistry::SignatureMap FunctionRegistry::pc_registry_map_ = InitPCMap(); FunctionRegistry::SignatureMap FunctionRegistry::InitPCMap() { SignatureMap map; - int num_entries = sizeof(pc_registry_) / sizeof(NativeFunction); + int num_entries = static_cast(sizeof(pc_registry_) / sizeof(NativeFunction)); printf("Registry has %d pre-compiled functions\n", num_entries); for (int i = 0; i < num_entries; i++) { diff --git a/cpp/src/gandiva/precompiled/CMakeLists.txt b/cpp/src/gandiva/precompiled/CMakeLists.txt index b71cde3937d4..4857314ce091 100644 --- a/cpp/src/gandiva/precompiled/CMakeLists.txt +++ b/cpp/src/gandiva/precompiled/CMakeLists.txt @@ -34,7 +34,7 @@ foreach(SRC_FILE ${PRECOMPILED_SRCS}) set(BC_FILE ${CMAKE_CURRENT_BINARY_DIR}/${SRC_BASE}.bc) add_custom_command( OUTPUT ${BC_FILE} - COMMAND ${CLANG_EXECUTABLE} + COMMAND ${CLANG_EXECUTABLE} -I${CMAKE_SOURCE_DIR}/src -std=c++11 -emit-llvm -O2 -c ${ABSOLUTE_SRC} -o ${BC_FILE} DEPENDS ${SRC_FILE}) list(APPEND BC_FILES ${BC_FILE}) diff --git a/cpp/src/gandiva/precompiled/arithmetic_ops.cc b/cpp/src/gandiva/precompiled/arithmetic_ops.cc index 20d2c69c9c0b..7d05699a4ada 100644 --- a/cpp/src/gandiva/precompiled/arithmetic_ops.cc +++ b/cpp/src/gandiva/precompiled/arithmetic_ops.cc @@ -55,9 +55,11 @@ extern "C" { } // Symmetric binary fns : left, right params and return type are same. -#define BINARY_SYMMETRIC(NAME, TYPE, OP) \ - FORCE_INLINE \ - TYPE NAME##_##TYPE##_##TYPE(TYPE left, TYPE right) { return left OP right; } +#define BINARY_SYMMETRIC(NAME, TYPE, OP) \ + FORCE_INLINE \ + TYPE NAME##_##TYPE##_##TYPE(TYPE left, TYPE right) { \ + return static_cast(left OP right); \ + } NUMERIC_TYPES(BINARY_SYMMETRIC, add, +) NUMERIC_TYPES(BINARY_SYMMETRIC, subtract, -) diff --git a/cpp/src/gandiva/precompiled/bitmap.cc b/cpp/src/gandiva/precompiled/bitmap.cc index 53f17354b36e..651f2cbf0a20 100644 --- a/cpp/src/gandiva/precompiled/bitmap.cc +++ b/cpp/src/gandiva/precompiled/bitmap.cc @@ -17,6 +17,8 @@ // BitMap functions +#include "arrow/util/bit-util.h" + extern "C" { #include "./types.h" @@ -28,26 +30,20 @@ extern "C" { #define POS_TO_BIT_INDEX(p) (p % 8) FORCE_INLINE -bool bitMapGetBit(const unsigned char* bmap, int position) { - int byteIdx = POS_TO_BYTE_INDEX(position); - int bitIdx = POS_TO_BIT_INDEX(position); - return ((bmap[byteIdx] & (1 << bitIdx)) > 0); +bool bitMapGetBit(const uint8_t* bmap, int position) { + return arrow::BitUtil::GetBit(bmap, position); } FORCE_INLINE -void bitMapSetBit(unsigned char* bmap, int position, bool value) { - int byteIdx = POS_TO_BYTE_INDEX(position); - int bitIdx = POS_TO_BIT_INDEX(position); - bmap[byteIdx] ^= (-value ^ bmap[byteIdx]) & (1UL << bitIdx); +void bitMapSetBit(uint8_t* bmap, int position, bool value) { + arrow::BitUtil::SetBitTo(bmap, position, value); } // Clear the bit if value = false. Does nothing if value = true. FORCE_INLINE -void bitMapClearBitIfFalse(unsigned char* bmap, int position, bool value) { +void bitMapClearBitIfFalse(uint8_t* bmap, int position, bool value) { if (!value) { - int byteIdx = POS_TO_BYTE_INDEX(position); - int bitIdx = POS_TO_BIT_INDEX(position); - bmap[byteIdx] &= ~(1 << bitIdx); + arrow::BitUtil::ClearBit(bmap, position); } } diff --git a/cpp/src/gandiva/precompiled/hash_test.cc b/cpp/src/gandiva/precompiled/hash_test.cc index d58344d32965..265385be85bc 100644 --- a/cpp/src/gandiva/precompiled/hash_test.cc +++ b/cpp/src/gandiva/precompiled/hash_test.cc @@ -44,8 +44,8 @@ TEST(TestHash, TestHash32) { EXPECT_EQ(hash32(u16, 0), zero_hash); EXPECT_EQ(hash32(s32, 0), zero_hash); EXPECT_EQ(hash32(u32, 0), zero_hash); - EXPECT_EQ(hash32(s64, 0), zero_hash); - EXPECT_EQ(hash32(u64, 0), zero_hash); + EXPECT_EQ(hash32(static_cast(s64), 0), zero_hash); + EXPECT_EQ(hash32(static_cast(u64), 0), zero_hash); EXPECT_EQ(hash32(f32, 0), zero_hash); EXPECT_EQ(hash32(f64, 0), zero_hash); @@ -82,8 +82,8 @@ TEST(TestHash, TestHash64) { EXPECT_EQ(hash64(u16, 0), zero_hash); EXPECT_EQ(hash64(s32, 0), zero_hash); EXPECT_EQ(hash64(u32, 0), zero_hash); - EXPECT_EQ(hash64(s64, 0), zero_hash); - EXPECT_EQ(hash64(u64, 0), zero_hash); + EXPECT_EQ(hash64(static_cast(s64), 0), zero_hash); + EXPECT_EQ(hash64(static_cast(u64), 0), zero_hash); EXPECT_EQ(hash64(f32, 0), zero_hash); EXPECT_EQ(hash64(f64, 0), zero_hash); diff --git a/cpp/src/gandiva/tests/micro_benchmarks.cc b/cpp/src/gandiva/tests/micro_benchmarks.cc index d8706fa09435..fd6d87de6f0a 100644 --- a/cpp/src/gandiva/tests/micro_benchmarks.cc +++ b/cpp/src/gandiva/tests/micro_benchmarks.cc @@ -30,7 +30,9 @@ using arrow::int32; using arrow::int64; using arrow::utf8; -float tolerance_ratio = 4.0; +// TODO : the base numbers are from a mac. they need to be caliberated +// for the hardware used by travis. +float tolerance_ratio = 6.0; class TestBenchmarks : public ::testing::Test { public: diff --git a/cpp/src/gandiva/tests/projector_test.cc b/cpp/src/gandiva/tests/projector_test.cc index 6f72a03c81ae..57497cd1c31b 100644 --- a/cpp/src/gandiva/tests/projector_test.cc +++ b/cpp/src/gandiva/tests/projector_test.cc @@ -209,10 +209,10 @@ static void TestArithmeticOpsForType(arrow::MemoryPool* pool) { std::vector eq; std::vector lt; for (int i = 0; i < num_records; i++) { - sum.push_back(input0[i] + input1[i]); - sub.push_back(input0[i] - input1[i]); - mul.push_back(input0[i] * input1[i]); - div.push_back(input0[i] / input1[i]); + sum.push_back(static_cast(input0[i] + input1[i])); + sub.push_back(static_cast(input0[i] - input1[i])); + mul.push_back(static_cast(input0[i] * input1[i])); + div.push_back(static_cast(input0[i] / input1[i])); eq.push_back(input0[i] == input1[i]); lt.push_back(input0[i] < input1[i]); }