From 9d8be2e2cf9848cd8e9590a7083e46b639085cb7 Mon Sep 17 00:00:00 2001 From: achirkin Date: Wed, 6 Apr 2022 10:24:56 +0200 Subject: [PATCH 1/5] Allow enabling NVTX markers by downstream projects after install. --- cpp/CMakeLists.txt | 31 ++++++++++++++++++++++++++----- 1 file changed, 26 insertions(+), 5 deletions(-) diff --git a/cpp/CMakeLists.txt b/cpp/CMakeLists.txt index 35b066abc9..b57fa4655c 100644 --- a/cpp/CMakeLists.txt +++ b/cpp/CMakeLists.txt @@ -153,7 +153,6 @@ target_include_directories(raft INTERFACE target_link_libraries(raft INTERFACE raft::Thrust - $<$:CUDA::nvToolsExt> CUDA::cublas CUDA::curand CUDA::cusolver @@ -163,7 +162,6 @@ target_link_libraries(raft INTERFACE $<$:cuco::cuco> std::mdspan) -target_compile_definitions(raft INTERFACE $<$:NVTX_ENABLED>) target_compile_features(raft INTERFACE cxx_std_17 $) if(RAFT_COMPILE_LIBRARIES OR RAFT_COMPILE_DIST_LIBRARY OR RAFT_COMPILE_NN_LIBRARY) @@ -177,6 +175,29 @@ SECTIONS ]=]) endif() +############################################################################## +# - NVTX support in raft ----------------------------------------------------- + +if (NVTX) + # This enables NVTX within the project with no option to disable it downstream. + target_link_libraries(raft INTERFACE CUDA::nvToolsExt) + target_compile_definitions(raft INTERFACE NVTX_ENABLED) + set(nvtx_export_string "") +else() + # Allow enable NVTX downstream if not set here. + # This creates a new option at build/install time, which is set by default to OFF, + # but can be enabled in the dependent project. + get_property(nvtx_option_help_string CACHE NVTX PROPERTY HELPSTRING) + string(CONCAT nvtx_export_string + "option(NVTX \"" ${nvtx_option_help_string} "\" OFF)" + [=[ + +target_link_libraries(raft::raft INTERFACE $<$:CUDA::nvToolsExt>) +target_compile_definitions(raft::raft INTERFACE $<$:NVTX_ENABLED>) + + ]=]) +endif() + ############################################################################## # - raft_distance ------------------------------------------------------------ add_library(raft_distance INTERFACE) @@ -361,7 +382,8 @@ Imported Targets: ]=]) -set(code_string +string(CONCAT code_string +${nvtx_export_string} [=[ if(NOT TARGET raft::Thrust) @@ -381,8 +403,7 @@ if(nn IN_LIST raft_FIND_COMPONENTS) add_library(faiss ALIAS faiss::faiss) endif() endif() -]=] - ) +]=]) # Use `rapids_export` for 22.04 as it will have COMPONENT support include(cmake/modules/raft_export.cmake) From 577c85bd087b2a9996cedbb1503ac7ee8d7705b9 Mon Sep 17 00:00:00 2001 From: achirkin Date: Mon, 2 May 2022 09:40:44 +0200 Subject: [PATCH 2/5] Address the review comments --- build.sh | 2 +- cpp/CMakeLists.txt | 15 +++++++-------- 2 files changed, 8 insertions(+), 9 deletions(-) diff --git a/build.sh b/build.sh index 568de2956d..0e259c4517 100755 --- a/build.sh +++ b/build.sh @@ -219,7 +219,7 @@ if (( ${NUMARGS} == 0 )) || hasArg libraft || hasArg docs || hasArg tests || has -DCMAKE_CUDA_ARCHITECTURES=${RAFT_CMAKE_CUDA_ARCHITECTURES} \ -DRAFT_COMPILE_LIBRARIES=${COMPILE_LIBRARIES} \ -DRAFT_ENABLE_NN_DEPENDENCIES=${ENABLE_NN_DEPENDENCIES} \ - -DNVTX=${NVTX} \ + -DRAFT_NVTX=${NVTX} \ -DDISABLE_DEPRECATION_WARNINGS=${DISABLE_DEPRECATION_WARNINGS} \ -DBUILD_TESTS=${BUILD_TESTS} \ -DBUILD_BENCH=${BUILD_BENCH} \ diff --git a/cpp/CMakeLists.txt b/cpp/CMakeLists.txt index edb0c23e91..a853714f22 100644 --- a/cpp/CMakeLists.txt +++ b/cpp/CMakeLists.txt @@ -59,7 +59,7 @@ option(CUDA_STATIC_RUNTIME "Statically link the CUDA runtime" OFF) option(DETECT_CONDA_ENV "Enable detection of conda environment for dependencies" ON) option(DISABLE_DEPRECATION_WARNINGS "Disable deprecaction warnings " ON) option(DISABLE_OPENMP "Disable OpenMP" OFF) -option(NVTX "Enable nvtx markers" OFF) +option(RAFT_NVTX "Enable nvtx markers" OFF) option(RAFT_COMPILE_LIBRARIES "Enable building raft shared library instantiations" ${BUILD_TESTS}) option(RAFT_COMPILE_NN_LIBRARY "Enable building raft nearest neighbors shared library instantiations" OFF) @@ -91,7 +91,7 @@ message(VERBOSE "RAFT: Disable depreaction warnings " ${DISABLE_DEPRECATION_WARN message(VERBOSE "RAFT: Disable OpenMP: ${DISABLE_OPENMP}") message(VERBOSE "RAFT: Enable kernel resource usage info: ${CUDA_ENABLE_KERNELINFO}") message(VERBOSE "RAFT: Enable lineinfo in nvcc: ${CUDA_ENABLE_LINEINFO}") -message(VERBOSE "RAFT: Enable nvtx markers: ${NVTX}") +message(VERBOSE "RAFT: Enable nvtx markers: ${RAFT_NVTX}") message(VERBOSE "RAFT: Statically link the CUDA runtime: ${CUDA_STATIC_RUNTIME}") # Set RMM logging level @@ -193,22 +193,21 @@ endif() ############################################################################## # - NVTX support in raft ----------------------------------------------------- -if (NVTX) +if (RAFT_NVTX) # This enables NVTX within the project with no option to disable it downstream. target_link_libraries(raft INTERFACE CUDA::nvToolsExt) target_compile_definitions(raft INTERFACE NVTX_ENABLED) - set(nvtx_export_string "") else() # Allow enable NVTX downstream if not set here. # This creates a new option at build/install time, which is set by default to OFF, # but can be enabled in the dependent project. - get_property(nvtx_option_help_string CACHE NVTX PROPERTY HELPSTRING) + get_property(nvtx_option_help_string CACHE RAFT_NVTX PROPERTY HELPSTRING) string(CONCAT nvtx_export_string - "option(NVTX \"" ${nvtx_option_help_string} "\" OFF)" + "option(RAFT_NVTX \"" ${nvtx_option_help_string} "\" OFF)" [=[ -target_link_libraries(raft::raft INTERFACE $<$:CUDA::nvToolsExt>) -target_compile_definitions(raft::raft INTERFACE $<$:NVTX_ENABLED>) +target_link_libraries(raft::raft INTERFACE $<$:CUDA::nvToolsExt>) +target_compile_definitions(raft::raft INTERFACE $<$:NVTX_ENABLED>) ]=]) endif() From d6da71661c2831c2b2aea88ad6e9e3cf8346e894 Mon Sep 17 00:00:00 2001 From: achirkin Date: Mon, 2 May 2022 09:55:21 +0200 Subject: [PATCH 3/5] Enable NVTX by default when building from source. --- build.sh | 10 +++++----- conda/recipes/libraft_distance/build.sh | 2 +- conda/recipes/libraft_headers/build.sh | 2 +- conda/recipes/libraft_nn/build.sh | 2 +- conda/recipes/pylibraft/build.sh | 2 +- conda/recipes/pyraft/build.sh | 2 +- 6 files changed, 10 insertions(+), 10 deletions(-) diff --git a/build.sh b/build.sh index 0e259c4517..0fa26e8aa7 100755 --- a/build.sh +++ b/build.sh @@ -18,7 +18,7 @@ ARGS=$* # script, and that this script resides in the repo dir! REPODIR=$(cd $(dirname $0); pwd) -VALIDARGS="clean libraft pyraft pylibraft docs tests bench clean -v -g --install --compile-libs --compile-nn --compile-dist --allgpuarch --nvtx --show_depr_warn -h --buildfaiss --minimal-deps" +VALIDARGS="clean libraft pyraft pylibraft docs tests bench clean -v -g --install --compile-libs --compile-nn --compile-dist --allgpuarch --no-force-nvtx --show_depr_warn -h --buildfaiss --minimal-deps" HELP="$0 [ ...] [ ...] where is: clean - remove all existing build artifacts and configuration (start over) @@ -41,7 +41,7 @@ HELP="$0 [ ...] [ ...] --allgpuarch - build for all supported GPU architectures --buildfaiss - build faiss statically into raft --install - install cmake targets - --nvtx - enable nvtx for profiling support + --no-force-nvtx - disable nvtx (profiling markers), but allow enabling it in downstream projects --show_depr_warn - show cmake deprecation warnings -h - print this text @@ -70,7 +70,7 @@ ENABLE_thrust_DEPENDENCY=ON ENABLE_ucx_DEPENDENCY=OFF ENABLE_nccl_DEPENDENCY=OFF -NVTX=OFF +NVTX=ON CLEAN=0 UNINSTALL=0 DISABLE_DEPRECATION_WARNINGS=ON @@ -157,8 +157,8 @@ fi if hasArg --buildfaiss; then BUILD_STATIC_FAISS=ON fi -if hasArg --nvtx; then - NVTX=ON +if hasArg --no-force-nvtx; then + NVTX=OFF fi if hasArg --show_depr_warn; then DISABLE_DEPRECATION_WARNINGS=OFF diff --git a/conda/recipes/libraft_distance/build.sh b/conda/recipes/libraft_distance/build.sh index d0843fdd79..39e0ea8206 100644 --- a/conda/recipes/libraft_distance/build.sh +++ b/conda/recipes/libraft_distance/build.sh @@ -1,4 +1,4 @@ #!/usr/bin/env bash # Copyright (c) 2022, NVIDIA CORPORATION. -./build.sh libraft --install -v --allgpuarch --compile-dist +./build.sh libraft --install -v --allgpuarch --compile-dist --no-force-nvtx diff --git a/conda/recipes/libraft_headers/build.sh b/conda/recipes/libraft_headers/build.sh index d351b27577..f2be967b69 100644 --- a/conda/recipes/libraft_headers/build.sh +++ b/conda/recipes/libraft_headers/build.sh @@ -1,4 +1,4 @@ #!/usr/bin/env bash # Copyright (c) 2022, NVIDIA CORPORATION. -./build.sh libraft --install -v --allgpuarch \ No newline at end of file +./build.sh libraft --install -v --allgpuarch --no-force-nvtx diff --git a/conda/recipes/libraft_nn/build.sh b/conda/recipes/libraft_nn/build.sh index 9d53362738..d08a5157a9 100644 --- a/conda/recipes/libraft_nn/build.sh +++ b/conda/recipes/libraft_nn/build.sh @@ -1,4 +1,4 @@ #!/usr/bin/env bash # Copyright (c) 2022, NVIDIA CORPORATION. -./build.sh libraft --install -v --allgpuarch --compile-nn +./build.sh libraft --install -v --allgpuarch --compile-nn --no-force-nvtx diff --git a/conda/recipes/pylibraft/build.sh b/conda/recipes/pylibraft/build.sh index 442428e0ee..a417296d2b 100644 --- a/conda/recipes/pylibraft/build.sh +++ b/conda/recipes/pylibraft/build.sh @@ -2,4 +2,4 @@ #!/usr/bin/env bash # This assumes the script is executed from the root of the repo directory -./build.sh pylibraft --install +./build.sh pylibraft --install --no-force-nvtx diff --git a/conda/recipes/pyraft/build.sh b/conda/recipes/pyraft/build.sh index 4745f583f3..35293a7931 100644 --- a/conda/recipes/pyraft/build.sh +++ b/conda/recipes/pyraft/build.sh @@ -3,4 +3,4 @@ # Copyright (c) 2022, NVIDIA CORPORATION. # This assumes the script is executed from the root of the repo directory -./build.sh pyraft --install +./build.sh pyraft --install --no-force-nvtx From 0fde15f2a26340fcad216e3189f510e6c5e2675d Mon Sep 17 00:00:00 2001 From: achirkin Date: Tue, 3 May 2022 16:29:11 +0200 Subject: [PATCH 4/5] Rename --no-force-nvtx to --no-nvtx --- build.sh | 6 +++--- conda/recipes/libraft_distance/build.sh | 2 +- conda/recipes/libraft_headers/build.sh | 2 +- conda/recipes/libraft_nn/build.sh | 2 +- conda/recipes/pylibraft/build.sh | 2 +- conda/recipes/pyraft/build.sh | 2 +- 6 files changed, 8 insertions(+), 8 deletions(-) diff --git a/build.sh b/build.sh index 0fa26e8aa7..202c1b3b52 100755 --- a/build.sh +++ b/build.sh @@ -18,7 +18,7 @@ ARGS=$* # script, and that this script resides in the repo dir! REPODIR=$(cd $(dirname $0); pwd) -VALIDARGS="clean libraft pyraft pylibraft docs tests bench clean -v -g --install --compile-libs --compile-nn --compile-dist --allgpuarch --no-force-nvtx --show_depr_warn -h --buildfaiss --minimal-deps" +VALIDARGS="clean libraft pyraft pylibraft docs tests bench clean -v -g --install --compile-libs --compile-nn --compile-dist --allgpuarch --no-nvtx --show_depr_warn -h --buildfaiss --minimal-deps" HELP="$0 [ ...] [ ...] where is: clean - remove all existing build artifacts and configuration (start over) @@ -41,7 +41,7 @@ HELP="$0 [ ...] [ ...] --allgpuarch - build for all supported GPU architectures --buildfaiss - build faiss statically into raft --install - install cmake targets - --no-force-nvtx - disable nvtx (profiling markers), but allow enabling it in downstream projects + --no-nvtx - disable nvtx (profiling markers), but allow enabling it in downstream projects --show_depr_warn - show cmake deprecation warnings -h - print this text @@ -157,7 +157,7 @@ fi if hasArg --buildfaiss; then BUILD_STATIC_FAISS=ON fi -if hasArg --no-force-nvtx; then +if hasArg --no-nvtx; then NVTX=OFF fi if hasArg --show_depr_warn; then diff --git a/conda/recipes/libraft_distance/build.sh b/conda/recipes/libraft_distance/build.sh index 39e0ea8206..35a669d6df 100644 --- a/conda/recipes/libraft_distance/build.sh +++ b/conda/recipes/libraft_distance/build.sh @@ -1,4 +1,4 @@ #!/usr/bin/env bash # Copyright (c) 2022, NVIDIA CORPORATION. -./build.sh libraft --install -v --allgpuarch --compile-dist --no-force-nvtx +./build.sh libraft --install -v --allgpuarch --compile-dist --no-nvtx diff --git a/conda/recipes/libraft_headers/build.sh b/conda/recipes/libraft_headers/build.sh index f2be967b69..02ef674787 100644 --- a/conda/recipes/libraft_headers/build.sh +++ b/conda/recipes/libraft_headers/build.sh @@ -1,4 +1,4 @@ #!/usr/bin/env bash # Copyright (c) 2022, NVIDIA CORPORATION. -./build.sh libraft --install -v --allgpuarch --no-force-nvtx +./build.sh libraft --install -v --allgpuarch --no-nvtx diff --git a/conda/recipes/libraft_nn/build.sh b/conda/recipes/libraft_nn/build.sh index d08a5157a9..caa643a356 100644 --- a/conda/recipes/libraft_nn/build.sh +++ b/conda/recipes/libraft_nn/build.sh @@ -1,4 +1,4 @@ #!/usr/bin/env bash # Copyright (c) 2022, NVIDIA CORPORATION. -./build.sh libraft --install -v --allgpuarch --compile-nn --no-force-nvtx +./build.sh libraft --install -v --allgpuarch --compile-nn --no-nvtx diff --git a/conda/recipes/pylibraft/build.sh b/conda/recipes/pylibraft/build.sh index a417296d2b..4e64d031ec 100644 --- a/conda/recipes/pylibraft/build.sh +++ b/conda/recipes/pylibraft/build.sh @@ -2,4 +2,4 @@ #!/usr/bin/env bash # This assumes the script is executed from the root of the repo directory -./build.sh pylibraft --install --no-force-nvtx +./build.sh pylibraft --install --no-nvtx diff --git a/conda/recipes/pyraft/build.sh b/conda/recipes/pyraft/build.sh index 35293a7931..1462f365ff 100644 --- a/conda/recipes/pyraft/build.sh +++ b/conda/recipes/pyraft/build.sh @@ -3,4 +3,4 @@ # Copyright (c) 2022, NVIDIA CORPORATION. # This assumes the script is executed from the root of the repo directory -./build.sh pyraft --install --no-force-nvtx +./build.sh pyraft --install --no-nvtx From df14dde7a2ea076a91fa1d2eced27a525477482b Mon Sep 17 00:00:00 2001 From: achirkin Date: Thu, 5 May 2022 09:26:17 +0200 Subject: [PATCH 5/5] Fix usage of non-constexpr nvtxDomainCreateA in cuda <= 11.2 --- cpp/include/raft/common/detail/nvtx.hpp | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/cpp/include/raft/common/detail/nvtx.hpp b/cpp/include/raft/common/detail/nvtx.hpp index 4cef7c07bc..4a16ec81bd 100644 --- a/cpp/include/raft/common/detail/nvtx.hpp +++ b/cpp/include/raft/common/detail/nvtx.hpp @@ -1,5 +1,5 @@ /* - * Copyright (c) 2021, NVIDIA CORPORATION. + * Copyright (c) 2021-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. @@ -140,7 +140,7 @@ struct domain_store { /* If `Domain::name` does not exist, this default instance is used and throws the error. */ static_assert(sizeof(Domain) != sizeof(Domain), "Type used to identify a domain must contain a static member 'char const* name'"); - static inline nvtxDomainHandle_t const kValue = nullptr; + static inline auto value() -> const nvtxDomainHandle_t { return nullptr; } }; template @@ -150,7 +150,12 @@ struct domain_store< std::enable_if_t< std::is_same::type>::value, Domain>> { - static inline nvtxDomainHandle_t const kValue = nvtxDomainCreateA(Domain::name); + static inline auto value() -> const nvtxDomainHandle_t + { + // NB: static modifier ensures the domain is created only once + static const nvtxDomainHandle_t kValue = nvtxDomainCreateA(Domain::name); + return kValue; + } }; template @@ -163,7 +168,7 @@ inline void push_range_name(const char* name) event_attrib.color = generate_next_color(name); event_attrib.messageType = NVTX_MESSAGE_TYPE_ASCII; event_attrib.message.ascii = name; - nvtxDomainRangePushEx(domain_store::kValue, &event_attrib); + nvtxDomainRangePushEx(domain_store::value(), &event_attrib); } template @@ -183,7 +188,7 @@ inline void push_range(const char* format, Args... args) template inline void pop_range() { - nvtxDomainRangePop(domain_store::kValue); + nvtxDomainRangePop(domain_store::value()); } #else // NVTX_ENABLED