Add RMM_USE_NVTX cmake option to provide localized control of NVTX for RMM

rapidsai / rmm

RAPIDS Memory Manager

https://docs.rapids.ai/api/rmm/stable/

Apache License 2.0

478 stars 195 forks source link

Add RMM_USE_NVTX cmake option to provide localized control of NVTX for RMM #1602

Closed jlowe closed 2 months ago

jlowe commented 3 months ago

Description

Adds a new RMM_USE_NVTX cmake option that defaults to the prior USE_NVTX setting if not set. If RMM_USE_NVTX is OFF, it stubs out the RMM_FUNC_RANGE macro which is used for RMM's NVTX ranges, effectively disabling NVTX within RMM yet leaving it enabled for the rest of the project using RMM.

Closes #1595

Checklist

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

jlowe commented 3 months ago

Verified that with this change by default RMM NVTX ranges appear in profiles, but I can add -DRMM_USE_NVTX=OFF to our libcudf build to remove RMM NVTX ranges while still preserving the other NVTX ranges in libcudf.

bdice commented 2 months ago

One final question before we merge: should this be RMM_NVTX instead of RMM_USE_NVTX? We have precedent for {LIB}_NVTX (without "USE") in RAFT here: https://github.com/rapidsai/raft/blob/5bf6642b9ec4c82e97db21f1ce3bb057fc20db82/cpp/CMakeLists.txt#L68

jlowe commented 2 months ago

Oh I didn't realize there was already precedence for a project-specific NVTX config. Makes sense to be consistent.

GregoryKimball commented 2 months ago

@robertmaynard is this ready to merge?

jlowe commented 2 months ago

Can someone with permissions update the PR labels or grant me the permissions to do so?

I think this is ready to go. Would really like to see this get into the 24.08 release.

vyasr commented 2 months ago

/merge