Add class for documented Nvtx Ranges #12035
Conversation
Signed-off-by: Zach Puller <zpuller@nvidia.com>
Signed-off-by: Zach Puller <zpuller@nvidia.com>
Signed-off-by: Zach Puller <zpuller@nvidia.com>
Signed-off-by: Zach Puller <zpuller@nvidia.com>
Signed-off-by: Zach Puller <zpuller@nvidia.com>
| @@ -1,7 +1,7 @@ | |||
| --- | |||
| layout: page | |||
| title: Compute Sanitizer | |||
| nav_order: 7 | |||
There was a problem hiding this comment.
We are adding in a new page in the developer docs. Additionally, I noticed that there were two existing pages with the same nav_order, which I am also fixing, so the end result is all the back pages get moved back by 2.
There was a problem hiding this comment.
this is a separate concern worth its own PR
Signed-off-by: Zach Puller <zpuller@nvidia.com>
Signed-off-by: Zach Puller <zpuller@nvidia.com>
| @@ -1,7 +1,7 @@ | |||
| --- | |||
| layout: page | |||
| title: Compute Sanitizer | |||
| nav_order: 7 | |||
There was a problem hiding this comment.
this is a separate concern worth its own PR
| object NvtxId { | ||
| val registeredRanges = new ListBuffer[NvtxId]() | ||
|
|
||
| private def register(nvtxId: NvtxId): Unit = registeredRanges += nvtxId |
There was a problem hiding this comment.
I would make it a Map and detect collisions
| } | ||
| } | ||
|
|
||
| class NvtxRangeWithDoc(val id: NvtxId, color: NvtxColor) extends AutoCloseable { |
There was a problem hiding this comment.
nit
| class NvtxRangeWithDoc(val id: NvtxId, color: NvtxColor) extends AutoCloseable { | |
| case class NvtxRangeWithDoc(val id: NvtxId, color: NvtxColor) extends AutoCloseable { |
| color: NvtxColor, | ||
| f: => A): A = { | ||
| val start = System.nanoTime() | ||
| withResource(new NvtxRangeWithDoc(range, color)) { _ => |
There was a problem hiding this comment.
nit: if you make it a case class or add a companion with factory apply manually
| withResource(new NvtxRangeWithDoc(range, color)) { _ => | |
| withResource(NvtxRangeWithDoc(range, color)) { _ => |
| } | ||
| } | ||
|
|
||
| class NvtxRangeWithDoc(val id: NvtxId, color: NvtxColor) extends AutoCloseable { |
There was a problem hiding this comment.
Can we please change the API? The AutoClosable was me trying to do an RAII like API, but that is not a great API for java/scala. I know this will require changes to the CUDF code to open up the API, but can we please add some static push and pop methods in NvtxRange that take the name and the color bits but do check if nvtx is enabled.
Then we can have each of the NvtxIds, might want to rename them, have an apply method so we get the following code instead.
Nntx.ACQUIRE_GPU(semWaitTimeNs) {
f()
}
Nvtx.RELEASE_GPU {
...
}
You might have guessed from this that I would like the color to be a part of the NvtxId as well. I think it is better if we are consistent in the color for a given range and that we have it documented.
This PR adds a scala class which can wrap
NvtxRangeand add a docstring to a given range. It then provides functionality to take all such ranges and auto generate a readme containing the doc strings.All existing ranges will need to be migrated to the new class, but I'm planning to do that in a series of smaller migration PRs. This PR migrates a couple ranges over for example purposes.