From 3b318953dcfb67f2e7763e1303aa03533b4038b4 Mon Sep 17 00:00:00 2001 From: Vyas Ramasubramani Date: Fri, 15 Jul 2022 16:00:03 -0700 Subject: [PATCH 1/2] Remove ptds as an option and always use per-thread default stream. --- build.sh | 9 +---- ci/benchmark/build.sh | 2 +- ci/gpu/build.sh | 2 +- cpp/CMakeLists.txt | 15 ++------ cpp/cmake/thirdparty/get_nvcomp.cmake | 2 +- cpp/include/cudf/utilities/default_stream.hpp | 11 ------ cpp/src/utilities/default_stream.cpp | 35 ------------------- cpp/tests/CMakeLists.txt | 1 - .../utilities_tests/default_stream_tests.cpp | 25 ------------- java/README.md | 4 +-- java/ci/build-in-docker.sh | 4 --- java/pom.xml | 2 -- java/src/main/native/CMakeLists.txt | 9 +---- java/src/main/native/src/CudfJni.cpp | 20 ----------- 14 files changed, 10 insertions(+), 131 deletions(-) delete mode 100644 cpp/src/utilities/default_stream.cpp delete mode 100644 cpp/tests/utilities_tests/default_stream_tests.cpp diff --git a/build.sh b/build.sh index eee3ee512fa..f72689b9de0 100755 --- a/build.sh +++ b/build.sh @@ -17,7 +17,7 @@ ARGS=$* # script, and that this script resides in the repo dir! REPODIR=$(cd $(dirname $0); pwd) -VALIDARGS="clean libcudf cudf cudfjar dask_cudf benchmarks tests libcudf_kafka cudf_kafka custreamz -v -g -n -l --allgpuarch --disable_nvtx --opensource_nvcomp --show_depr_warn --ptds -h --build_metrics --incl_cache_stats" +VALIDARGS="clean libcudf cudf cudfjar dask_cudf benchmarks tests libcudf_kafka cudf_kafka custreamz -v -g -n -l --allgpuarch --disable_nvtx --opensource_nvcomp --show_depr_warn -h --build_metrics --incl_cache_stats" HELP="$0 [clean] [libcudf] [cudf] [cudfjar] [dask_cudf] [benchmarks] [tests] [libcudf_kafka] [cudf_kafka] [custreamz] [-v] [-g] [-n] [-h] [--cmake-args=\\\"\\\"] clean - remove all existing build artifacts and configuration (start over) @@ -37,7 +37,6 @@ HELP="$0 [clean] [libcudf] [cudf] [cudfjar] [dask_cudf] [benchmarks] [tests] [li --disable_nvtx - disable inserting NVTX profiling ranges --opensource_nvcomp - disable use of proprietary nvcomp extensions --show_depr_warn - show cmake deprecation warnings - --ptds - enable per-thread default stream --build_metrics - generate build metrics report for libcudf --incl_cache_stats - include cache statistics in build metrics report --cmake-args=\\\"\\\" - pass arbitrary list of CMake configuration options (escape all quotes in argument) @@ -150,7 +149,6 @@ function buildLibCudfJniInDocker { -DCUDF_USE_ARROW_STATIC=ON \ -DCUDF_ENABLE_ARROW_S3=OFF \ -DBUILD_TESTS=OFF \ - -DCUDF_USE_PER_THREAD_DEFAULT_STREAM=ON \ -DRMM_LOGGING_LEVEL=OFF \ -DBUILD_SHARED_LIBS=OFF && \ cmake --build . --parallel ${PARALLEL_LEVEL} && \ @@ -165,7 +163,6 @@ function buildLibCudfJniInDocker { -DCMAKE_CXX_LINKER_LAUNCHER=ccache' \ -DCUDF_CPP_BUILD_DIR=$workspaceRepoDir/java/target/libcudf-cmake-build \ -DCUDA_STATIC_RUNTIME=ON \ - -DCUDF_USE_PER_THREAD_DEFAULT_STREAM=ON \ -DUSE_GDS=ON \ -DGPU_ARCHS=${CUDF_CMAKE_CUDA_ARCHITECTURES} \ -DCUDF_JNI_LIBCUDF_STATIC=ON \ @@ -218,9 +215,6 @@ fi if hasArg --show_depr_warn; then BUILD_DISABLE_DEPRECATION_WARNING=OFF fi -if hasArg --ptds; then - BUILD_PER_THREAD_DEFAULT_STREAM=ON -fi if hasArg --build_metrics; then BUILD_REPORT_METRICS=ON fi @@ -286,7 +280,6 @@ if buildAll || hasArg libcudf; then -DBUILD_TESTS=${BUILD_TESTS} \ -DBUILD_BENCHMARKS=${BUILD_BENCHMARKS} \ -DDISABLE_DEPRECATION_WARNING=${BUILD_DISABLE_DEPRECATION_WARNING} \ - -DCUDF_USE_PER_THREAD_DEFAULT_STREAM=${BUILD_PER_THREAD_DEFAULT_STREAM} \ -DCMAKE_BUILD_TYPE=${BUILD_TYPE} \ ${EXTRA_CMAKE_ARGS} diff --git a/ci/benchmark/build.sh b/ci/benchmark/build.sh index 5d03a518fcf..96e932b3ede 100755 --- a/ci/benchmark/build.sh +++ b/ci/benchmark/build.sh @@ -102,7 +102,7 @@ conda list --show-channel-urls ################################################################################ logger "Build libcudf..." -"$WORKSPACE/build.sh" clean libcudf cudf dask_cudf benchmarks tests --ptds +"$WORKSPACE/build.sh" clean libcudf cudf dask_cudf benchmarks tests ################################################################################ # BENCHMARK - Run and parse libcudf and cuDF benchmarks diff --git a/ci/gpu/build.sh b/ci/gpu/build.sh index 1a5073b2f24..be975b906a9 100755 --- a/ci/gpu/build.sh +++ b/ci/gpu/build.sh @@ -125,7 +125,7 @@ if [[ -z "$PROJECT_FLASH" || "$PROJECT_FLASH" == "0" ]]; then ################################################################################ gpuci_logger "Build from source" - "$WORKSPACE/build.sh" clean libcudf cudf dask_cudf libcudf_kafka cudf_kafka benchmarks tests --ptds + "$WORKSPACE/build.sh" clean libcudf cudf dask_cudf libcudf_kafka cudf_kafka benchmarks tests ################################################################################ # TEST - Run GoogleTest diff --git a/cpp/CMakeLists.txt b/cpp/CMakeLists.txt index 86bfdc1444b..be5bf0e7423 100644 --- a/cpp/CMakeLists.txt +++ b/cpp/CMakeLists.txt @@ -48,12 +48,6 @@ option(CUDF_ENABLE_ARROW_ORC "Build the Arrow ORC adapter" OFF) option(CUDF_ENABLE_ARROW_PYTHON "Find (or build) Arrow with Python support" OFF) option(CUDF_ENABLE_ARROW_PARQUET "Find (or build) Arrow with Parquet support" OFF) option(CUDF_ENABLE_ARROW_S3 "Build/Enable AWS S3 Arrow filesystem support" ON) -option( - CUDF_USE_PER_THREAD_DEFAULT_STREAM - "Build cuDF with per-thread default stream, including passing the per-thread default - stream to external libraries." - OFF -) option(DISABLE_DEPRECATION_WARNING "Disable warnings generated from deprecated declarations." OFF) # Option to enable line info in CUDA device compilation to allow introspection when profiling / # memchecking @@ -70,7 +64,6 @@ message(VERBOSE "CUDF: Build cuDF shared libraries: ${BUILD_SHARED_LIBS}") message(VERBOSE "CUDF: Use a file cache for JIT compiled kernels: ${JITIFY_USE_CACHE}") message(VERBOSE "CUDF: Build and statically link Arrow libraries: ${CUDF_USE_ARROW_STATIC}") message(VERBOSE "CUDF: Build and enable S3 filesystem support for Arrow: ${CUDF_ENABLE_ARROW_S3}") -message(VERBOSE "CUDF: Build with per-thread default stream: ${CUDF_USE_PER_THREAD_DEFAULT_STREAM}") message( VERBOSE "CUDF: Disable warnings generated from deprecated declarations: ${DISABLE_DEPRECATION_WARNING}" @@ -594,11 +587,9 @@ if(JITIFY_USE_CACHE) endif() # Per-thread default stream -if(CUDF_USE_PER_THREAD_DEFAULT_STREAM) - target_compile_definitions( - cudf PUBLIC CUDA_API_PER_THREAD_DEFAULT_STREAM CUDF_USE_PER_THREAD_DEFAULT_STREAM - ) -endif() +target_compile_definitions( + cudf PUBLIC CUDA_API_PER_THREAD_DEFAULT_STREAM +) # Disable NVTX if necessary if(NOT USE_NVTX) diff --git a/cpp/cmake/thirdparty/get_nvcomp.cmake b/cpp/cmake/thirdparty/get_nvcomp.cmake index 41bbf44abc8..d3ae82a366a 100644 --- a/cpp/cmake/thirdparty/get_nvcomp.cmake +++ b/cpp/cmake/thirdparty/get_nvcomp.cmake @@ -23,7 +23,7 @@ function(find_and_configure_nvcomp) ) # Per-thread default stream - if(TARGET nvcomp AND CUDF_USE_PER_THREAD_DEFAULT_STREAM) + if(TARGET nvcomp) target_compile_definitions(nvcomp PRIVATE CUDA_API_PER_THREAD_DEFAULT_STREAM) endif() endfunction() diff --git a/cpp/include/cudf/utilities/default_stream.hpp b/cpp/include/cudf/utilities/default_stream.hpp index 94bc01787e3..044bbc34f17 100644 --- a/cpp/include/cudf/utilities/default_stream.hpp +++ b/cpp/include/cudf/utilities/default_stream.hpp @@ -26,17 +26,6 @@ namespace cudf { * Use this value to ensure the correct stream is used when compiled with per * thread default stream. */ -#if defined(CUDF_USE_PER_THREAD_DEFAULT_STREAM) static const rmm::cuda_stream_view default_stream_value{rmm::cuda_stream_per_thread}; -#else -static constexpr rmm::cuda_stream_view default_stream_value{}; -#endif - -/** - * @brief Check if per-thread default stream is enabled. - * - * @return true if PTDS is enabled, false otherwise. - */ -bool is_ptds_enabled(); } // namespace cudf diff --git a/cpp/src/utilities/default_stream.cpp b/cpp/src/utilities/default_stream.cpp deleted file mode 100644 index d580972bc97..00000000000 --- a/cpp/src/utilities/default_stream.cpp +++ /dev/null @@ -1,35 +0,0 @@ -/* - * Copyright (c) 2020, NVIDIA CORPORATION. - * - * 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. - */ - -#include - -namespace cudf { - -/** - * @brief Check if per-thread default stream is enabled. - * - * @return true if PTDS is enabled, false otherwise. - */ -bool is_ptds_enabled() -{ -#ifdef CUDA_API_PER_THREAD_DEFAULT_STREAM - return true; -#else - return false; -#endif -} - -} // namespace cudf diff --git a/cpp/tests/CMakeLists.txt b/cpp/tests/CMakeLists.txt index a1e3cfed286..42346049948 100644 --- a/cpp/tests/CMakeLists.txt +++ b/cpp/tests/CMakeLists.txt @@ -274,7 +274,6 @@ ConfigureTest( utilities_tests/column_utilities_tests.cpp utilities_tests/column_wrapper_tests.cpp utilities_tests/lists_column_wrapper_tests.cpp - utilities_tests/default_stream_tests.cpp utilities_tests/type_check_tests.cpp ) diff --git a/cpp/tests/utilities_tests/default_stream_tests.cpp b/cpp/tests/utilities_tests/default_stream_tests.cpp deleted file mode 100644 index f5c55879b9c..00000000000 --- a/cpp/tests/utilities_tests/default_stream_tests.cpp +++ /dev/null @@ -1,25 +0,0 @@ -/* - * Copyright (c) 2020, NVIDIA CORPORATION. - * - * 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. - */ - -#include - -#include - -#ifdef CUDA_API_PER_THREAD_DEFAULT_STREAM -TEST(DefaultStreamTest, PtdsIsEnabled) { EXPECT_TRUE(cudf::is_ptds_enabled()); } -#else -TEST(DefaultStreamTest, PtdsIsNotEnabled) { EXPECT_FALSE(cudf::is_ptds_enabled()); } -#endif diff --git a/java/README.md b/java/README.md index 05a24c1d3d3..fb60adb1d7f 100644 --- a/java/README.md +++ b/java/README.md @@ -101,7 +101,7 @@ Since the PTDS option is for each compilation unit, it should be done at the sam whole codebase. To enable PTDS, first build cuDF: ```shell script cd src/cudf/cpp/build -cmake .. -DCMAKE_INSTALL_PREFIX=$CONDA_PREFIX -DCUDF_USE_PER_THREAD_DEFAULT_STREAM=ON +cmake .. -DCMAKE_INSTALL_PREFIX=$CONDA_PREFIX make -j`nproc` make install ``` @@ -109,7 +109,7 @@ make install then build the jar: ```shell script cd src/cudf/java -mvn clean install -DCUDF_USE_PER_THREAD_DEFAULT_STREAM=ON +mvn clean install ``` ## GPUDirect Storage (GDS) diff --git a/java/ci/build-in-docker.sh b/java/ci/build-in-docker.sh index b5ce0dd5f9f..5a79ada0c16 100755 --- a/java/ci/build-in-docker.sh +++ b/java/ci/build-in-docker.sh @@ -22,7 +22,6 @@ gcc --version SKIP_JAVA_TESTS=${SKIP_JAVA_TESTS:-true} BUILD_CPP_TESTS=${BUILD_CPP_TESTS:-OFF} ENABLE_CUDA_STATIC_RUNTIME=${ENABLE_CUDA_STATIC_RUNTIME:-ON} -ENABLE_PTDS=${ENABLE_PTDS:-ON} RMM_LOGGING_LEVEL=${RMM_LOGGING_LEVEL:-OFF} ENABLE_NVTX=${ENABLE_NVTX:-ON} ENABLE_GDS=${ENABLE_GDS:-OFF} @@ -38,7 +37,6 @@ echo "SIGN_FILE: $SIGN_FILE,\ SKIP_JAVA_TESTS: $SKIP_JAVA_TESTS,\ BUILD_CPP_TESTS: $BUILD_CPP_TESTS,\ ENABLE_CUDA_STATIC_RUNTIME: $ENABLE_CUDA_STATIC_RUNTIME,\ - ENABLED_PTDS: $ENABLE_PTDS,\ ENABLE_NVTX: $ENABLE_NVTX,\ ENABLE_GDS: $ENABLE_GDS,\ RMM_LOGGING_LEVEL: $RMM_LOGGING_LEVEL,\ @@ -61,7 +59,6 @@ cmake .. -G"${CMAKE_GENERATOR}" \ -DCUDF_USE_ARROW_STATIC=ON \ -DCUDF_ENABLE_ARROW_S3=OFF \ -DBUILD_TESTS=$BUILD_CPP_TESTS \ - -DCUDF_USE_PER_THREAD_DEFAULT_STREAM=$ENABLE_PTDS \ -DRMM_LOGGING_LEVEL=$RMM_LOGGING_LEVEL \ -DBUILD_SHARED_LIBS=OFF @@ -75,7 +72,6 @@ cmake --install . ###### Build cudf jar ###### BUILD_ARG="-Dmaven.repo.local=\"$WORKSPACE/.m2\"\ -DskipTests=$SKIP_JAVA_TESTS\ - -DCUDF_USE_PER_THREAD_DEFAULT_STREAM=$ENABLE_PTDS\ -DCUDA_STATIC_RUNTIME=$ENABLE_CUDA_STATIC_RUNTIME\ -DCUDF_JNI_LIBCUDF_STATIC=ON\ -DUSE_GDS=$ENABLE_GDS -Dtest=*,!CuFileTest,!CudaFatalTest" diff --git a/java/pom.xml b/java/pom.xml index 210a0ee9e2d..bffc1270e0c 100644 --- a/java/pom.xml +++ b/java/pom.xml @@ -164,7 +164,6 @@ OFF OFF - OFF OFF ALL OFF @@ -449,7 +448,6 @@ - diff --git a/java/src/main/native/CMakeLists.txt b/java/src/main/native/CMakeLists.txt index 0972dba22cf..44122bb626b 100755 --- a/java/src/main/native/CMakeLists.txt +++ b/java/src/main/native/CMakeLists.txt @@ -38,7 +38,6 @@ project( option(USE_NVTX "Build with NVTX support" ON) option(BUILD_SHARED_LIBS "Build cuDF JNI shared libraries" ON) option(BUILD_TESTS "Configure CMake to build tests" ON) -option(CUDF_USE_PER_THREAD_DEFAULT_STREAM "Build with per-thread default stream" OFF) option(CUDA_STATIC_RUNTIME "Statically link the CUDA runtime" OFF) option(USE_GDS "Build with GPUDirect Storage (GDS)/cuFile support" OFF) option(CUDF_JNI_LIBCUDF_STATIC "Link with libcudf.a" OFF) @@ -46,9 +45,6 @@ option(CUDF_JNI_LIBCUDF_STATIC "Link with libcudf.a" OFF) message(VERBOSE "CUDF_JNI: Build with NVTX support: ${USE_NVTX}") message(VERBOSE "CUDF_JNI: Build cuDF JNI shared libraries: ${BUILD_SHARED_LIBS}") message(VERBOSE "CUDF_JNI: Configure CMake to build tests: ${BUILD_TESTS}") -message(VERBOSE - "CUDF_JNI: Build with per-thread default stream: ${CUDF_USE_PER_THREAD_DEFAULT_STREAM}" -) message(VERBOSE "CUDF_JNI: Statically link the CUDA runtime: ${CUDA_STATIC_RUNTIME}") message(VERBOSE "CUDF_JNI: Build with GPUDirect Storage support: ${USE_GDS}") message(VERBOSE "CUDF_JNI: Link with libcudf statically: ${CUDF_JNI_LIBCUDF_STATIC}") @@ -82,10 +78,7 @@ if(NOT USE_NVTX) target_compile_definitions(cudfjni PUBLIC NVTX_DISABLE) endif() -if(CUDF_USE_PER_THREAD_DEFAULT_STREAM) - message(STATUS "Using per-thread default stream") - add_compile_definitions(CUDA_API_PER_THREAD_DEFAULT_STREAM CUDF_USE_PER_THREAD_DEFAULT_STREAM) -endif() +add_compile_definitions(CUDA_API_PER_THREAD_DEFAULT_STREAM) # ################################################################################################## # * build type ------------------------------------------------------------------------------------ diff --git a/java/src/main/native/src/CudfJni.cpp b/java/src/main/native/src/CudfJni.cpp index 928e167c4da..c06806a2cf1 100644 --- a/java/src/main/native/src/CudfJni.cpp +++ b/java/src/main/native/src/CudfJni.cpp @@ -39,12 +39,6 @@ class jvm_detach_on_destruct { namespace cudf { namespace jni { -#ifdef CUDA_API_PER_THREAD_DEFAULT_STREAM -constexpr bool is_ptds_enabled{true}; -#else -constexpr bool is_ptds_enabled{false}; -#endif - static jclass Host_memory_buffer_jclass; static jmethodID Host_buffer_allocate; static jfieldID Host_buffer_address; @@ -145,16 +139,6 @@ JNIEXPORT jint JNI_OnLoad(JavaVM *vm, void *) { return JNI_ERR; } - // make sure libcudf and the JNI library are built with the same PTDS mode - if (cudf::is_ptds_enabled() != cudf::jni::is_ptds_enabled) { - std::ostringstream ss; - ss << "Libcudf is_ptds_enabled=" << cudf::is_ptds_enabled() - << ", which does not match cudf jni is_ptds_enabled=" << cudf::jni::is_ptds_enabled - << ". They need to be built with the same per-thread default stream flag."; - env->ThrowNew(env->FindClass("java/lang/RuntimeException"), ss.str().c_str()); - return JNI_ERR; - } - // cache any class objects and method IDs here if (!cudf::jni::cache_contiguous_table_jni(env)) { if (!env->ExceptionCheck()) { @@ -186,8 +170,4 @@ JNIEXPORT void JNI_OnUnload(JavaVM *vm, void *) { cudf::jni::release_host_memory_buffer_jni(env); } -JNIEXPORT jboolean JNICALL Java_ai_rapids_cudf_Cuda_isPtdsEnabled(JNIEnv *env, jclass, jlong size) { - return cudf::jni::is_ptds_enabled; -} - } // extern "C" From 19b6fe7e7142fb765542bc7b80ced9f55adb0920 Mon Sep 17 00:00:00 2001 From: Vyas Ramasubramani Date: Fri, 15 Jul 2022 16:12:48 -0700 Subject: [PATCH 2/2] Fix copyright. --- cpp/CMakeLists.txt | 1 - java/src/main/native/src/CudfJni.cpp | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/cpp/CMakeLists.txt b/cpp/CMakeLists.txt index be5bf0e7423..be7150f76f1 100644 --- a/cpp/CMakeLists.txt +++ b/cpp/CMakeLists.txt @@ -538,7 +538,6 @@ add_library( src/unary/math_ops.cu src/unary/nan_ops.cu src/unary/null_ops.cu - src/utilities/default_stream.cpp src/utilities/type_checks.cpp ) diff --git a/java/src/main/native/src/CudfJni.cpp b/java/src/main/native/src/CudfJni.cpp index c06806a2cf1..606f58ee797 100644 --- a/java/src/main/native/src/CudfJni.cpp +++ b/java/src/main/native/src/CudfJni.cpp @@ -1,5 +1,5 @@ /* - * Copyright (c) 2019-2021, NVIDIA CORPORATION. + * Copyright (c) 2019-2022, NVIDIA CORPORATION. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License.