Skip to content

[1/n] Add generalized event types and GPU Performance Monitoring counter event support#1212

Open
briancoutinho wants to merge 2 commits into
pytorch:mainfrom
briancoutinho:bcoutinho/gpu_pm_activity_type
Open

[1/n] Add generalized event types and GPU Performance Monitoring counter event support#1212
briancoutinho wants to merge 2 commits into
pytorch:mainfrom
briancoutinho:bcoutinho/gpu_pm_activity_type

Conversation

@briancoutinho

@briancoutinho briancoutinho commented Dec 30, 2025

Copy link
Copy Markdown
Contributor

Overview

Originally, this was part 1 of splitting PR #1148. It supports a new kind of GPU Counter events that will be published to the timeline as a time series. In the process I realized we should add more generic event types for accelerators rather than being tied to CUDA specific naming. This has historically lead to each new accelerator adding it's own events which is maintenance burden.

  1. Generalized Accelerator Event types: Add generic event types in the ActivityType enum. There are now aliases in the enum class definition for older events like CUDA_RUNTIME, MTIA_RUNTIME to name a few. The dynamic plugin will leverage generic events so upstream source code changes will not be required for new accelerators.
  2. GPU PM Counter Event Type: This enables supporting a performance counter event stream. Counter events can be serialized in ChromeTrace format and will be rendered as time series in the UI.

Details

Accelerator-Agnostic Event Types

The change reorganizes the ActivityType enum class to introduce generic, accelerator-agnostic event types that work across all hardware backends (CUDA, MTIA, HPU, XPU, etc.). Device-specific types are now deprecated aliases pointing to their generic counterparts. There are few corner case exceptions like MTIA_INSIGHT, CUDA_SYNC, see the header.

New Generic Event Types

Event Type Description Replaces (Deprecated Aliases)
RUNTIME Host-side runtime events from any accelerator backend CUDA_RUNTIME, MTIA_RUNTIME, GLOW_RUNTIME, XPU_RUNTIME, PRIVATEUSE1_RUNTIME, HPU_OP
DRIVER Host-side driver events from any accelerator backend CUDA_DRIVER, PRIVATEUSE1_DRIVER
CONCURRENT_KERNEL On-device kernel execution across all accelerators MTIA_CCP_EVENTS
GPU_PM_COUNTER Performance monitoring counters for hardware profiling (new)

Guidance for future use of ActivityTypes

Existing code using deprecated aliases will continue to work, but new code should use the generic types:

// ❌ Deprecated (still works for backward compatibility)
ActivityType::CUDA_RUNTIME
ActivityType::MTIA_RUNTIME

// ✅ Preferred
ActivityType::RUNTIME

I have not changed the usage of these types in the code base yet. That can happen in a follow up change.

Notes

  • Old string names like "cuda_runtime" still parse correctly via aliasMap, and old enum values like ActivityType::CUDA_RUNTIME still compile (as aliases).
  • defaultActivityTypesArray is now constexpr, enabling compile-time evaluation and eliminating runtime overhead.

GPU PM Counter Events

This is a straightforward change to emit Chrome Trace counter events] for counters obtained from the GPU. The event can be leveraged by any accelerator backend.

The values of the counters are embedded as key/val pairs in the output json

{..., "name": "ctr", "ph": "C", "ts":  0, "args": {"my_counter":  42}},

Testing

make test
// or
ctest -R ActivityTypes

The ParseTest.ActivityTypes validates that aliases in the Config string are correctly converted to the underlying base type. Since the enum class uses aliases all existing code in kineto that uses the original activity types continues to compile and work as expected.
The test above also checks default activity types are unchanged
https://github.com/pytorch/kineto/blob/main/libkineto/test/ConfigTest.cpp#L89-L108

@meta-cla meta-cla Bot added the cla signed label Dec 30, 2025
@briancoutinho
briancoutinho force-pushed the bcoutinho/gpu_pm_activity_type branch from e9e530b to fabc0c7 Compare December 30, 2025 22:11
@briancoutinho
briancoutinho marked this pull request as ready for review December 30, 2025 22:19
@briancoutinho
briancoutinho force-pushed the bcoutinho/gpu_pm_activity_type branch from fabc0c7 to 93105fe Compare December 31, 2025 18:18
@KarhouTam

Copy link
Copy Markdown

Hi @briancoutinho, this update is absolutely fantastic! As an out-of-tree accelerator developer, I’m genuinely excited about these advancements. If possible, could you kindly share an approximate timeline for the upcoming splitting changes? Can't wait to begin integrating our accelerator profiler with these impressive updates.

Additionally, may I ask if Kineto is still open to accepting contributions for third-party accelerator profiler as a plugin after these changes merged? Thank you!

At the end, happy new year to you and the community (maybe a little bit late, hah)!

@briancoutinho

Copy link
Copy Markdown
Contributor Author

cc @sraikund16 @malfet @valentinandrei could you help review this PR. This is the first split of my original longer PR but I also introduced generic events that came in the last PR discussion.
This should certainly help with modularizing Kineto as well.

@briancoutinho

briancoutinho commented Jan 8, 2026

Copy link
Copy Markdown
Contributor Author

@KarhouTam thanks for the feedback :)

If possible, could you kindly share an approximate timeline for the upcoming splitting changes?

It should not take too long for me to develop the changes like 1-2 weeks. There is one more somewhat larger PR that should enable the dynamic plugin interface.
But I think the main bottleneck will be to review the changes one by one. So I cannot create stacked PRs because I do not have write access to repo. Could take between 1-2 months I think.

Additionally, may I ask if Kineto is still open to accepting contributions for third-party accelerator profiler as a plugin after these changes merged? Thank you!

Actually, that is what our proposal was #1121
There were some concerns from Meta maintainers on dynamic loading, so we still need some clarification if this approach is fundamentally ok with PyTorch.

@malfet / @valentinandrei could we actually meet on a VC to discuss this? It is totally reasonable if we need to address security and code issues in the PRs. But if dynamic loading itself is an issue then it would be good to know that before putting some effort on this.

Also see: pytorch/pytorch#166205 (comment)

@malfet

malfet commented Jan 9, 2026

Copy link
Copy Markdown
Contributor

@briancoutinho can you please coordinate with @eqy to get added to say Tue meeting series and we can discuss it there

And giving you a write permissions to the repo is not a problem imo

@fffrog

fffrog commented Jan 9, 2026

Copy link
Copy Markdown

Hi @briancoutinho, thank you so much for introducing this significant work; it's incredibly valuable for external accelerators.

There were some concerns from Meta maintainers on dynamic loading

Could you mind to share those concerns here after discussing with the Meta maintainers later? As far as I know, the ONNX runtime framework has been using dynamic loading for years to support multiple accelerators, which really helps in integrating new accelerators

@briancoutinho

Copy link
Copy Markdown
Contributor Author

Thanks @malfet and @eqy! Please also include @yisitu in the discussion.
Our emails are bcoutinho@nvidia.com and ysitu@nvida.com

@briancoutinho

Copy link
Copy Markdown
Contributor Author

ping @eqy can you please include Situ and me in the sync above
Our emails are bcoutinho@nvidia.com and ysitu@nvida.com
You can find us on slack too

@briancoutinho

Copy link
Copy Markdown
Contributor Author

@scotts Could you help review this PR. I will work on rebasing this in a bit but please share your general feedback.
IMO this change will help with making kineto more modular and extensible in the long run

Comment thread libkineto/src/output_json.cpp Outdated
@briancoutinho

Copy link
Copy Markdown
Contributor Author

cc @divyanshk Please have a look at this PR. This broadly a good thing to do irrespective of dynamic plugin approach. The idea is to avoid adding a new activity type for every custom profiler but keeping activity types consistent.

PYTHON_FUNCTION, // Python function calls
OVERHEAD, // Profiler-induced overhead events
COLLECTIVE_COMM, // Collective communication operations
GPU_PM_COUNTER, // Performance monitoring counters

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Is this intended to be a specific kind of GPU PM counter to be leveraged with the new profiler ? or can this be general enough to work with the CUPTI PM Sampling API (we don't have that in Kineto but say we add it in the future)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@divyanshk good question! This was intended to be a generic event for performance counters from an accelerator. It could be used with CUPTI PM Sampling as well as MTIA counters. Do you have another name suggestion?

CUDA_DRIVER = DRIVER,
MTIA_RUNTIME = RUNTIME,
MTIA_CCP_EVENTS = CONCURRENT_KERNEL,
MTIA_COUNTERS = GPU_PM_COUNTER,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I'm not confident that these aliases will work - they might, but we'd need to do a lot of internal verification. Because we're actually changing the value, there's real possibility that internal systems and tooling will break.

@scotts

scotts commented Apr 3, 2026

Copy link
Copy Markdown
Contributor

@divyanshk, @briancoutinho, I like this direction! We definitely want to avoid the reliance on device-specific constants. This nicely complements the work @divyanshk is doing to enable PrivateUse1.

The challenge I see is that because this changes the actual values for other devices, I think merging this work will require a lot of internal shepherding. That is, on the Meta side, we'll need to do a lot of testing and investigations to make sure this doesn't break anything internally. At the moment, I'm also not sure how big of a work item that is - the next step is for us to scope that out.

@fffrog

fffrog commented Apr 7, 2026

Copy link
Copy Markdown

@divyanshk, @briancoutinho, I like this direction! We definitely want to avoid the reliance on device-specific constants. This nicely complements the work @divyanshk is doing to enable PrivateUse1.

Hi @scotts, @divyanshk sorry to bother you.

I am the maintainer of the PrivateUse1 module in PyTorch. As is well known, PrivateUse1 is a generic Dispatch Key that helps various accelerators integrate into PyTorch. However, different accelerators may have different needs. Therefore, it would be great if Kineto could provide a registration mechanism for new accelerators, allowing them to dynamically customize ActivityType at runtime. Then, ActivityType could be divided into three categories:

  • Device-agnostic ActivityType.
  • Device-specific ActivityType (privileges for accelerators in the tree, maintaining ease of development and use).
  • ActivityType reserved for dynamically registering for PrivateUse1.

Could you offer some suggestions on dynamic registration, or provide some ideas on how to meet this diverse need?

cc @briancoutinho @KarhouTam

@divyanshk divyanshk left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@briancoutinho Can you rebase this PR on latest branch ? I want to kick off some internal testing.

@briancoutinho

Copy link
Copy Markdown
Contributor Author

Sorry I missed the notification @divyanshk , I will rebase this tomorrow.

@briancoutinho
briancoutinho force-pushed the bcoutinho/gpu_pm_activity_type branch from 3f1a113 to e7adde0 Compare April 23, 2026 06:22
Brian Coutinho and others added 2 commits April 23, 2026 11:30
@briancoutinho briancoutinho reopened this Apr 23, 2026
@briancoutinho

Copy link
Copy Markdown
Contributor Author

@divyanshk / @scotts I have rebased the branch please use this for internal validation.

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants