Use nvtx3 includes in string examples.

[bdice]

Description

This PR updates libcudf string examples to use <nvtx3/nvToolsExt.h>. This ensures we fetch the header-only NVTX v3. See NVTX docs for more information: https://nvidia.github.io/NVTX/#c-and-c

Checklist

• I am familiar with the Contributing Guidelines.
• New or existing tests cover these changes.
• The documentation is up to date with these changes.

[jakirkham]

Should we also replace lines like these?

cudf/cpp/examples/strings/CMakeLists.txt

Line 34 in de676b1

target_link_libraries(libcudf_apis PRIVATE cudf::cudf nvToolsExt)

cc @vyasr

[jakirkham]

cc @robertmaynard (for the CMake perspective)

[bdice]

@jakirkham I think no, we do not change those yet. Once we require CMake >= 3.25 we can update those lines.

[jakirkham]

Would it make sense to conditionally use the CMake 3.25+ features when CMake is new enough? Or would that not make sense?

[robertmaynard]

Would it make sense to conditionally use the CMake 3.25+ features when CMake is new enough? Or would that not make sense?

We could do so by doing:

target_link_libraries(libcudf_apis PRIVATE cudf::cudf)
if(CMAKE_VERSION VERSION_GREATER_EQUAL 3.25)
  target_link_libraries(libcudf_apis PRIVATE CUDA::nvtx3)
else()
  target_link_libraries(libcudf_apis PRIVATE nvToolsExt)
endif()

[vyasr]

We could do the conditional if we want to. I don't know that it really buys us anything since we will likely switch to requiring CMake 3.25 before the vast majority of people building libcudf upgrade on their own.

[jrhemstad]

Ideally, we should be using the NVTX C++ API in nvtx3.hpp. Note that this isn't distributed with the CTK yet and is only on GitHub.

cuDF has a vendored, older version of that header here: https://github.com/rapidsai/cudf/blob/branch-23.06/cpp/include/cudf/detail/nvtx/nvtx3.hpp

[jrhemstad]

You also never need to link against libnvToolsExt.so when using the header-only version from <nvtx3/...>. The CUDA::nvtx3 cmake target just sets up include paths and makes sure you link against dl.

[davidwendt]

Adding this link here for reference #6476
Do we need to get NVIDIA/NVTX#61 merged so we don't incur a compiler warning/error?

[jrhemstad]

Do we need to get NVIDIA/NVTX#61 merged so we don't incur a compiler warning/error?

Thanks for the reminder about that, I'd completely forgot. I pinged Jason to get it reviewed and merged.

I wouldn't consider it to be a blocker because we can always use [[maybe_unused]] locally to silence warnings if needed.

[bdice]

Thanks so much for the context here, @jrhemstad @vyasr @davidwendt @robertmaynard.

I would like to keep the scope constrained for now. We have a lot of moving pieces for CUDA 12 and I think that further improvements on this are important but not for 23.06 / CUDA 12 support. The primary goal of this PR is to ensure that we fetch nvtx3 headers, even if we continue using the C API and older CMake target for now. I don't think we have time in the 23.06 release cycle (20 work days until CUDA 12 support needs to be ready) to adopt the new C++ API / change sources to GitHub / require CMake 3.25 and use CUDA::nvtx3 yet. However, I have documented the necessary next steps in this private roadmap issue (https://github.com/rapidsai/roadmap/issues/481#issuecomment-1516330688) and I will revisit further work on this in the future when I have time to do this upgrade for all of RAPIDS.

I'll merge this PR as-is for now.

[bdice]

/merge