Skip to content

Conversation

@iomaganaris
Copy link
Contributor

Not sure if this is useful but I'm opening a PR to avoid losing these changes.

It generates the following NVTX ranges:
image

Requirements

Copy link
Contributor

@edopao edopao left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Definitely useful, in my opinion. Adding @egparedes because this PR is useful in the tracing of asynchronous execution to visualize the program call.

dace.Memlet(f"{output}[0]"),
)

if (config.COLLECT_METRICS_LEVEL == metrics.GPU_TX_MARKERS) and gpu:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your current check is at compile time. The check in the instrumentation above is at runtime. I propose to always add NVTX instrumentation, because the config.COLLECT_METRICS_LEVEL value is meant to be a runtime setting. It is still possible to skip SDFG instrumentation by setting use_metrics=False on the dace backend, at compile-time.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not sure I would be up to enabling NVTX ranges by default since they will add some (really minimal) overhead to the CPU side. If use_metrics=False can be set by default then it would be fine but it would increase some more complexity for production and profiling runs

@havogt havogt requested a review from egparedes October 27, 2025 09:58
@egparedes
Copy link
Contributor

Thanks @iomaganaris for this PR and @edopao for pointing me here. I would like to make this PR larger to add NVTX markers in the whole program/operator hot path, not only in the DaCe backend. I didn't have time today but I propose to discuss offline the scope and design of this together, since @edopao has also done some work with this.

edopao
edopao previously approved these changes Dec 4, 2025
@edopao edopao dismissed their stale review December 4, 2025 11:32

LGTM but Enrique had some ideas to extend this PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants