From 5b209d825affed2f7205d0bd6b270a5a91b12a38 Mon Sep 17 00:00:00 2001 From: Christian Zentgraf Date: Wed, 16 Oct 2024 14:24:09 -0400 Subject: [PATCH] Add Clang Linux build to CI pipeline The linux build is refactored to be able to switch between clang and gcc based builds of Velox. The gcc based linux build is run on pull and push. The clang based linux build is added to the scheduled jobs and executed on schedule only. --- .github/workflows/linux-build-base.yml | 178 +++++++++++++++++++++++++ .github/workflows/linux-build.yml | 155 +-------------------- .github/workflows/scheduled.yml | 7 + CMakeLists.txt | 12 +- scripts/setup-ubuntu.sh | 2 +- velox/dwio/common/tests/CMakeLists.txt | 15 +++ 6 files changed, 214 insertions(+), 155 deletions(-) create mode 100644 .github/workflows/linux-build-base.yml diff --git a/.github/workflows/linux-build-base.yml b/.github/workflows/linux-build-base.yml new file mode 100644 index 000000000000..e349fcbf69e3 --- /dev/null +++ b/.github/workflows/linux-build-base.yml @@ -0,0 +1,178 @@ +# Copyright (c) Facebook, Inc. and its affiliates. +# +# Licensed 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. + +name: Linux Build + +on: + workflow_call: + inputs: + use-clang: + description: 'Use Clang to compile the project.' + default: false + required: false + type: boolean + +jobs: + adapters: + name: Linux release with adapters + # prevent errors when forks ff their main branch + if: ${{ github.repository == 'facebookincubator/velox' }} + runs-on: 8-core-ubuntu + container: ghcr.io/facebookincubator/velox-dev:adapters + defaults: + run: + shell: bash + env: + CCACHE_DIR: "${{ github.workspace }}/.ccache" + VELOX_DEPENDENCY_SOURCE: SYSTEM + GTest_SOURCE: BUNDLED + simdjson_SOURCE: BUNDLED + xsimd_SOURCE: BUNDLED + CUDA_VERSION: "12.4" + USE_CLANG: "${{ inputs.use-clang && 'true' || 'false' }}" + steps: + - uses: actions/checkout@v4 + + - name: Fix git permissions + # Usually actions/checkout does this but as we run in a container + # it doesn't work + run: git config --global --add safe.directory ${GITHUB_WORKSPACE} + + - name: Install Dependencies + run: | + # Allows to install arbitrary cuda-version whithout needing to update + # docker container before. It simplifies testing new/different versions + if ! yum list installed cuda-nvcc-$(echo ${CUDA_VERSION} | tr '.' '-') 1>/dev/null; then + source scripts/setup-centos9.sh + install_cuda ${CUDA_VERSION} + fi + + - name: Install Minio + run: | + MINIO_BINARY="minio-2022-05-26" + if [ ! -f /usr/local/bin/${MINIO_BINARY} ]; then + wget https://dl.min.io/server/minio/release/linux-amd64/archive/minio.RELEASE.2022-05-26T05-48-41Z -O ${MINIO_BINARY} + chmod +x ./${MINIO_BINARY} + mv ./${MINIO_BINARY} /usr/local/bin/ + fi + + - uses: assignUser/stash/restore@v1 + with: + path: '${{ env.CCACHE_DIR }}' + key: ccache-linux-adapters-${{ inputs.use-clang && 'clang' || 'gcc' }} + + - name: "Zero Ccache Statistics" + run: | + ccache -sz + + - name: Make Release Build + env: + MAKEFLAGS: 'NUM_THREADS=8 MAX_HIGH_MEM_JOBS=4 MAX_LINK_JOBS=4' + CUDA_ARCHITECTURES: 70 + CUDA_COMPILER: /usr/local/cuda-${CUDA_VERSION}/bin/nvcc + # Set compiler to GCC 12 + CUDA_FLAGS: "-ccbin /opt/rh/gcc-toolset-12/root/usr/bin" + run: | + EXTRA_CMAKE_FLAGS=( + "-DVELOX_ENABLE_BENCHMARKS=ON" + "-DVELOX_ENABLE_ARROW=ON" + "-DVELOX_ENABLE_PARQUET=ON" + "-DVELOX_ENABLE_HDFS=ON" + "-DVELOX_ENABLE_S3=ON" + "-DVELOX_ENABLE_GCS=ON" + "-DVELOX_ENABLE_ABFS=ON" + "-DVELOX_ENABLE_REMOTE_FUNCTIONS=ON" + "-DVELOX_ENABLE_GPU=ON" + "-DVELOX_MONO_LIBRARY=ON" + ) + if [[ "${USE_CLANG}" = "true" ]]; then scripts/setup-centos9.sh install_clang15; export CC=/usr/bin/clang-15; export CXX=/usr/bin/clang++-15; CUDA_FLAGS="-ccbin /usr/lib64/llvm15/bin/clang++-15"; fi + make release EXTRA_CMAKE_FLAGS="${EXTRA_CMAKE_FLAGS[*]}" + + - name: Ccache after + run: ccache -s + + - uses: assignUser/stash/save@v1 + with: + path: '${{ env.CCACHE_DIR }}' + key: ccache-linux-adapters-${{ inputs.use-clang && 'clang' || 'gcc' }} + + - name: Run Tests + # Some of the adapters dependencies are in the 'adapters' conda env + shell: mamba run --no-capture-output -n adapters /usr/bin/bash -e {0} + env: + LIBHDFS3_CONF: "${{ github.workspace }}/scripts/hdfs-client.xml" + working-directory: _build/release + run: | + export CLASSPATH=`/usr/local/hadoop/bin/hdfs classpath --glob` + ctest -j 8 --label-exclude cuda_driver --output-on-failure --no-tests=error + + ubuntu-debug: + runs-on: 8-core-ubuntu + # prevent errors when forks ff their main branch + if: ${{ github.repository == 'facebookincubator/velox' }} + name: "Ubuntu debug with resolve_dependency" + env: + CCACHE_DIR: "${{ github.workspace }}/.ccache" + USE_CLANG: "${{ inputs.use-clang && 'true' || 'false' }}" + defaults: + run: + shell: bash + working-directory: velox + steps: + + - name: Get Ccache Stash + uses: assignUser/stash/restore@v1 + with: + path: '${{ env.CCACHE_DIR }}' + key: ccache-ubuntu-debug-default-${{ inputs.use-clang && 'clang' || 'gcc' }} + + - name: Ensure Stash Dirs Exists + working-directory: ${{ github.workspace }} + run: | + mkdir -p '${{ env.CCACHE_DIR }}' + + - uses: actions/checkout@v4 + with: + path: velox + + - name: Install Dependencies + run: | + source scripts/setup-ubuntu.sh && install_apt_deps + + - name: Clear CCache Statistics + run: | + ccache -sz + + - name: Make Debug Build + env: + VELOX_DEPENDENCY_SOURCE: BUNDLED + ICU_SOURCE: SYSTEM + MAKEFLAGS: "NUM_THREADS=8 MAX_HIGH_MEM_JOBS=4 MAX_LINK_JOBS=3" + EXTRA_CMAKE_FLAGS: "-DVELOX_ENABLE_ARROW=ON -DVELOX_ENABLE_PARQUET=ON" + run: | + if [[ "${USE_CLANG}" = "true" ]]; then export CC=/usr/bin/clang-15; export CXX=/usr/bin/clang++-15; fi + make debug + + - name: CCache after + run: | + ccache -vs + + - uses: assignUser/stash/save@v1 + with: + path: '${{ env.CCACHE_DIR }}' + key: ccache-ubuntu-debug-default-${{ inputs.use-clang && 'clang' || 'gcc' }} + + - name: Run Tests + run: | + cd _build/debug && ctest -j 8 --output-on-failure --no-tests=error diff --git a/.github/workflows/linux-build.yml b/.github/workflows/linux-build.yml index 476c786a63e3..39a84d986782 100644 --- a/.github/workflows/linux-build.yml +++ b/.github/workflows/linux-build.yml @@ -12,7 +12,7 @@ # See the License for the specific language governing permissions and # limitations under the License. -name: Linux Build +name: Linux Build using GCC on: push: @@ -27,6 +27,7 @@ on: - "scripts/setup-ubuntu.sh" - "scripts/setup-helper-functions.sh" - ".github/workflows/linux-build.yml" + - ".github/workflows/linux-build-base.yml" pull_request: paths: @@ -38,6 +39,7 @@ on: - "scripts/setup-ubuntu.sh" - "scripts/setup-helper-functions.sh" - ".github/workflows/linux-build.yml" + - ".github/workflows/linux-build-base.yml" permissions: contents: read @@ -47,151 +49,6 @@ concurrency: cancel-in-progress: true jobs: - adapters: - name: Linux release with adapters - # prevent errors when forks ff their main branch - if: ${{ github.repository == 'facebookincubator/velox' }} - runs-on: 8-core-ubuntu - container: ghcr.io/facebookincubator/velox-dev:adapters - defaults: - run: - shell: bash - env: - CCACHE_DIR: "${{ github.workspace }}/.ccache" - VELOX_DEPENDENCY_SOURCE: SYSTEM - GTest_SOURCE: BUNDLED - simdjson_SOURCE: BUNDLED - xsimd_SOURCE: BUNDLED - CUDA_VERSION: "12.4" - steps: - - uses: actions/checkout@v4 - - - name: Fix git permissions - # Usually actions/checkout does this but as we run in a container - # it doesn't work - run: git config --global --add safe.directory ${GITHUB_WORKSPACE} - - - name: Install Dependencies - run: | - # Allows to install arbitrary cuda-version whithout needing to update - # docker container before. It simplifies testing new/different versions - if ! yum list installed cuda-nvcc-$(echo ${CUDA_VERSION} | tr '.' '-') 1>/dev/null; then - source scripts/setup-centos9.sh - install_cuda ${CUDA_VERSION} - fi - - - name: Install Minio - run: | - MINIO_BINARY="minio-2022-05-26" - if [ ! -f /usr/local/bin/${MINIO_BINARY} ]; then - wget https://dl.min.io/server/minio/release/linux-amd64/archive/minio.RELEASE.2022-05-26T05-48-41Z -O ${MINIO_BINARY} - chmod +x ./${MINIO_BINARY} - mv ./${MINIO_BINARY} /usr/local/bin/ - fi - - - uses: assignUser/stash/restore@v1 - with: - path: '${{ env.CCACHE_DIR }}' - key: ccache-linux-adapters - - - name: "Zero Ccache Statistics" - run: | - ccache -sz - - - name: Make Release Build - env: - MAKEFLAGS: 'NUM_THREADS=8 MAX_HIGH_MEM_JOBS=4 MAX_LINK_JOBS=4' - CUDA_ARCHITECTURES: 70 - CUDA_COMPILER: /usr/local/cuda-${CUDA_VERSION}/bin/nvcc - # Set compiler to GCC 12 - CUDA_FLAGS: "-ccbin /opt/rh/gcc-toolset-12/root/usr/bin" - run: | - EXTRA_CMAKE_FLAGS=( - "-DVELOX_ENABLE_BENCHMARKS=ON" - "-DVELOX_ENABLE_ARROW=ON" - "-DVELOX_ENABLE_PARQUET=ON" - "-DVELOX_ENABLE_HDFS=ON" - "-DVELOX_ENABLE_S3=ON" - "-DVELOX_ENABLE_GCS=ON" - "-DVELOX_ENABLE_ABFS=ON" - "-DVELOX_ENABLE_REMOTE_FUNCTIONS=ON" - "-DVELOX_ENABLE_GPU=ON" - "-DVELOX_MONO_LIBRARY=ON" - ) - make release EXTRA_CMAKE_FLAGS="${EXTRA_CMAKE_FLAGS[*]}" - - - name: Ccache after - run: ccache -s - - - uses: assignUser/stash/save@v1 - with: - path: '${{ env.CCACHE_DIR }}' - key: ccache-linux-adapters - - - name: Run Tests - # Some of the adapters dependencies are in the 'adapters' conda env - shell: mamba run --no-capture-output -n adapters /usr/bin/bash -e {0} - env: - LIBHDFS3_CONF: "${{ github.workspace }}/scripts/hdfs-client.xml" - working-directory: _build/release - run: | - export CLASSPATH=`/usr/local/hadoop/bin/hdfs classpath --glob` - ctest -j 8 --label-exclude cuda_driver --output-on-failure --no-tests=error - - ubuntu-debug: - runs-on: 8-core-ubuntu - # prevent errors when forks ff their main branch - if: ${{ github.repository == 'facebookincubator/velox' }} - name: "Ubuntu debug with resolve_dependency" - env: - CCACHE_DIR: "${{ github.workspace }}/.ccache" - defaults: - run: - shell: bash - working-directory: velox - steps: - - - name: Get Ccache Stash - uses: assignUser/stash/restore@v1 - with: - path: '${{ env.CCACHE_DIR }}' - key: ccache-ubuntu-debug-default - - - name: Ensure Stash Dirs Exists - working-directory: ${{ github.workspace }} - run: | - mkdir -p '${{ env.CCACHE_DIR }}' - - - uses: actions/checkout@v4 - with: - path: velox - - - name: Install Dependencies - run: | - source scripts/setup-ubuntu.sh && install_apt_deps - - - name: Clear CCache Statistics - run: | - ccache -sz - - - name: Make Debug Build - env: - VELOX_DEPENDENCY_SOURCE: BUNDLED - ICU_SOURCE: SYSTEM - MAKEFLAGS: "NUM_THREADS=8 MAX_HIGH_MEM_JOBS=4 MAX_LINK_JOBS=3" - EXTRA_CMAKE_FLAGS: "-DVELOX_ENABLE_ARROW=ON -DVELOX_ENABLE_PARQUET=ON" - run: | - make debug - - - name: CCache after - run: | - ccache -vs - - - uses: assignUser/stash/save@v1 - with: - path: '${{ env.CCACHE_DIR }}' - key: ccache-ubuntu-debug-default - - - name: Run Tests - run: | - cd _build/debug && ctest -j 8 --output-on-failure --no-tests=error + linux-gcc: + name: Build with GCC + uses: ./.github/workflows/linux-build-base.yml diff --git a/.github/workflows/scheduled.yml b/.github/workflows/scheduled.yml index aa4286c0abb4..e6cf425881e6 100644 --- a/.github/workflows/scheduled.yml +++ b/.github/workflows/scheduled.yml @@ -992,3 +992,10 @@ jobs: /tmp/writer_fuzzer_repro /tmp/server.log /var/log + + linux-clang: + if: ${{ github.event_name == 'schedule' }} + name: Build with Clang + uses: ./.github/workflows/linux-build-base.yml + with: + use-clang: true diff --git a/CMakeLists.txt b/CMakeLists.txt index 7fd99b6dc744..99a791e56b21 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -231,11 +231,6 @@ if(${VELOX_FORCE_COLORED_OUTPUT}) endif() endif() -if("${CMAKE_CXX_COMPILER_ID}" STREQUAL "Clang" - AND "${CMAKE_CXX_COMPILER_VERSION}" VERSION_GREATER_EQUAL 15) - set(CMAKE_EXE_LINKER_FLAGS "-latomic") -endif() - # At the moment we prefer static linking but by default cmake looks for shared # libs first. This will still fallback to shared libs when static ones are not # found @@ -384,6 +379,13 @@ if(${VELOX_ENABLE_GPU}) find_package(CUDAToolkit REQUIRED) endif() +# Set after the test of the CUDA compiler. Otherwise, the test fails with +# -latomic not found because it is added right after the compiler exe. +if("${CMAKE_CXX_COMPILER_ID}" STREQUAL "Clang" + AND "${CMAKE_CXX_COMPILER_VERSION}" VERSION_GREATER_EQUAL 15) + set(CMAKE_EXE_LINKER_FLAGS "-latomic") +endif() + set(CMAKE_EXPORT_COMPILE_COMMANDS ON) if(CMAKE_SYSTEM_NAME MATCHES "Darwin") diff --git a/scripts/setup-ubuntu.sh b/scripts/setup-ubuntu.sh index 7b6216d2f715..3d502547a7f6 100755 --- a/scripts/setup-ubuntu.sh +++ b/scripts/setup-ubuntu.sh @@ -56,7 +56,7 @@ function install_clang15 { fi CLANG_PACKAGE_LIST=clang-15 if [[ ${VERSION} =~ "22.04" ]]; then - CLANG_PACKAGE_LIST=${CLANG_PACKAGE_LIST} gcc-12 g++-12 libc++-12-dev + CLANG_PACKAGE_LIST="${CLANG_PACKAGE_LIST} gcc-12 g++-12 libc++-12-dev" fi ${SUDO} apt install ${CLANG_PACKAGE_LIST} -y } diff --git a/velox/dwio/common/tests/CMakeLists.txt b/velox/dwio/common/tests/CMakeLists.txt index 3db9799d18ce..6c8c79dec99b 100644 --- a/velox/dwio/common/tests/CMakeLists.txt +++ b/velox/dwio/common/tests/CMakeLists.txt @@ -17,6 +17,21 @@ velox_add_library(velox_dwio_faulty_file_sink FaultyFileSink.cpp) velox_link_libraries(velox_dwio_faulty_file_sink velox_file_test_utils velox_dwio_common) +# There is an issue with the VTT symbol for the InlineExecutor from folly when +# building on Linux with Clang15. It is not created and results in a SEGV when +# attempting to construct the InlineExecutor. nm output shows no address for the +# "v VTT for folly::InlineExecutor" symbol while for a debug build it shows +# "000000000185d3e0 V VTT for folly::InlineExecutor" symbol. It seems to have +# been optimized away by Clang15 and may be some kind of bug. Changing the +# optimization level to 0 results in proper creation/linkage and successful +# execution of the test. Review if this is still necessary when upgrading Clang. +if(CMAKE_CXX_COMPILER_ID MATCHES "Clang") + set_property( + SOURCE ParallelForTest.cpp + APPEND + PROPERTY COMPILE_OPTIONS -O0) +endif() + add_executable( velox_dwio_common_test BitConcatenationTest.cpp