[rocprofiler-systems] AMD SMI collector using policy-based design by adjordje-amd · Pull Request #3703 · ROCm/rocm-systems

adjordje-amd · 2026-03-03T16:12:59Z

Motivation

The current AMD SMI implementation faces several challenges related to its maintainability and architecture, which this refactor aims to address:

Monolithic Design: The original amd_smi.cpp file, comprising 1354 lines, merges sampling, caching, and output logic. This tight coupling makes it challenging to modify or extend individual components.

Lack of Testability: Due to the integration with AMD SMI hardware APIs, the implementation is difficult to unit test and necessitates real hardware for verification.

Limited Flexibility: Introducing support for new output formats, such as RocPD alongside Perfetto, involves substantial code duplication.

Accumulated Bugs: The implementation contains several persistent bugs

The refactoring process aims to tackle these issues by replacing the monolithic setup with a contemporary, policy-based design. This new approach emphasizes modularity, testability, and enhanced performance

Technical Details

New architecture

JIRA ID

AIPROFSYST-13
AIPROFSYST-28
AIPROFSYST-29

Test Plan

Test Result

Submission Checklist

Look over the contributing guidelines at https://github.com/ROCm/ROCm/blob/develop/CONTRIBUTING.md#pull-requests.

...ts/rocprofiler-systems/source/lib/rocprof-sys/library/pmc/collectors/nic/perfetto_policy.hpp

projects/rocprofiler-systems/source/lib/rocprof-sys/library/pmc/collectors/nic/device.hpp

...ects/rocprofiler-systems/source/lib/rocprof-sys/library/pmc/collectors/base/traits_check.hpp

Rewrite AMD SMI and AINIC collectors using a policy-based design pattern that enables code reuse and extensibility. Key changes: - Add base collector template with configurable policies for caching, Perfetto output, and device handling - Implement GPU collector with SDMA utilization metrics - Implement NIC collector for AINIC RDMA metrics - Reorganize collector files into unified hierarchy under pmc/ - Update trace cache processors for new sample types - Add comprehensive unit tests with mock drivers

- Replace std::stringstream with fmt::format in types.hpp - Use std::vector instead of std::unordered_map for SDMA states - Template sample() callback to avoid std::function overhead - Fix CMake include dirs and link GPU/NIC tests to unit test target

Remove debug printf statement left in perfetto_policy.hpp that was printing JPEG activity status to stdout in production. Also apply clang-format-18 to fix alignment issues in: - sample_processor.hpp (type alias alignment) - base/collector.hpp (comment wrapping, blank line) - gpu/collector.hpp (LOG_ERROR formatting, blank line)

Address PR review feedback and modernize PMC collectors: - Replace strcmp chain with hash map for O(1) lookup (NIC device) - Move static storage into policy classes for encapsulation - Convert all namespaces to C++17 nested syntax (18 files) - Replace timemory::join with fmt::format for type safety Net reduction: 163 lines. Build verified.

dgaliffiAMD

Still reviewing the new files, but here are some initial thoughts.

It looks like the AI-NIC addition is breaking the back-compat. configurations. It probably just needs that preprocessor version check.

Please add and update to CHANGELOGS.

I'm seeing some unit test failures locally but haven't had a change to root cause them yet.

1: [==========] 443 tests from 24 test suites ran. (2660 ms total)
1: [  PASSED  ] 428 tests.
1: [  FAILED  ] 15 tests, listed below:
1: [  FAILED  ] DeviceTest.device_construction_no_support
1: [  FAILED  ] DeviceTest.device_construction_partial_support
1: [  FAILED  ] DeviceTest.edge_temperature_collection
1: [  FAILED  ] DeviceTest.jpeg_activity_collection_all_xcps
1: [  FAILED  ] DeviceTest.xcp_metrics_not_collected_when_unsupported
1: [  FAILED  ] DeviceTest.mixed_vcn_jpeg_support
1: [  FAILED  ] DeviceTest.all_metrics_supported_detection
1: [  FAILED  ] DeviceTest.vcn_activity_support_detection_any_xcp
1: [  FAILED  ] DeviceTest.vcn_activity_unsupported_all_sentinels
1: [  FAILED  ] DeviceTest.jpeg_activity_support_detection_any_xcp
1: [  FAILED  ] DeviceTest.vcn_activity_top_level_field_only
1: [  FAILED  ] DeviceTest.vcn_activity_in_both_fields
1: [  FAILED  ] DeviceTest.vcn_activity_detection_should_check_both_sources
1: [  FAILED  ] DeviceTest.vcn_activity_xcp_disabled_top_level_valid
1: [  FAILED  ] DeviceTest.full_lifecycle_with_realistic_data
1: 
1: 15 FAILED TESTS

projects/rocprofiler-systems/source/lib/core/trace_cache/perfetto_processor.cpp

projects/rocprofiler-systems/source/lib/core/trace_cache/rocpd_processor.cpp

The NIC collector was a standalone 264-line implementation that duplicated much of the base::collector pattern. This refactoring aligns it with the GPU collector approach for consistency and maintainability. Key changes: - Create nic_traits.hpp with NIC-specific behavior (name-based filtering, device context caching, agent registration) - Simplify collector.hpp to a type alias (264 → 24 lines) - Make perfetto_policy::post_process_device() public for traits access - Use uint64_t consistently for timestamps - Add noexcept to simple getter methods The traits class bridges NIC-specific requirements to base::collector: - Name-based device filtering (vs GPU's index-based) - Device context storage for APIs needing device_name/product_name - Agent registration during enumeration

Add SDMA (System DMA) usage as a new GPU metric for PMC collection, enabling monitoring of DMA engine utilization. Key changes: - Add sdma_usage metric (ID 14) with accessor, setter, and mask - Add SDMA to user metric aliases and Perfetto track info - Update metric validation pattern to accept "sdma_usage" - Add pytest marker for SDMA tests Additional fixes: - Change get_use_rocpd/get_caching_perfetto to return bool by value - Fix store_sample parameter order (enabled before supported) - Use core/amd_smi.hpp wrapper instead of direct amdsmi.h include - Change sample.device_id from size_t to uint32_t - Add debug logging for perfetto processor creation

Save point before implementing device_view type erasure. Fix DeviceTest unit tests failing due to missing mock expectation for get_gpu_asic_info(), which is now called during device initialization to populate vendor/product names.

Fix issues identified during PR review: Log level corrections: - Revert LOG_INFO to LOG_TRACE in agent_manager (routine operation) - Remove debug LOG_INFO messages from cache_manager - Change LOG_INFO to LOG_TRACE in rocpd_processor (hot path) - Change LOG_INFO to LOG_DEBUG in perfetto_policy post-processing Dead code removal: - Remove unused device_desc variable in nic/cache_policy - Remove uninitialized sample_metrics method from device_slice - Remove commented-out code and unused typeinfo include Move semantics fix: - Add noexcept to provider destructor - Implement explicit move constructor/assignment to prevent double-shutdown of AMD SMI driver in moved-from objects

The NIC/AINIC AMD SMI APIs (amdsmi_nic_asic_info_t, amdsmi_get_nic_*, AMDSMI_PROCESSOR_TYPE_AMD_NIC, etc.) are only available in AMD SMI >= 26.3 (ROCm 7.3+). CI tests against older ROCm versions (6.3-7.2) were failing because these APIs were used unconditionally. This commit adds #if defined(ROCPROFSYS_BUILD_AINIC) guards around: - NIC API wrapper functions in driver.hpp - AMDSMI_PROCESSOR_TYPE_AMD_NIC usage in provider.hpp This matches the existing pattern used for SDMA support with AMD_SMI_SDMA_SUPPORTED.

projects/rocprofiler-systems/source/lib/rocprof-sys/library/pmc/collectors/nic/device.hpp

projects/rocprofiler-systems/source/lib/rocprof-sys/library/rocm.cpp

dgaliffiAMD

I've tested on my "Navi" system. The output generated from the transpose and decode tests look good.
I haven't yet tested it on an MI system. I suspect those CI failures are MI-specific. Is it due to the XCP metrics?

projects/rocprofiler-systems/source/lib/rocprof-sys/library/pmc/collectors/base/collector.hpp

marantic-amd

Some minor nit-pics and suggestions. Looks good.

projects/rocprofiler-systems/source/lib/rocprof-sys/library/pmc/collectors/gpu/types.hpp

...rocprofiler-systems/source/lib/rocprof-sys/library/pmc/device_providers/amd_smi/provider.hpp

projects/rocprofiler-systems/source/lib/rocprof-sys/library/pmc/collectors/gpu/types.hpp

.../rocprofiler-systems/source/lib/rocprof-sys/library/pmc/collectors/gpu/tests/test_device.cpp

Track metadata registration was called inside the per-device loop, causing redundant re-initialization for each device. Move it before the loop alongside category metadata initialization, which is the correct initialization order.

adjordje-amd requested review from a team and jrmadsen as code owners March 3, 2026 16:13

github-actions bot added the project: rocprofiler-systems label Mar 3, 2026

systems-assistant bot added the organization: ROCm label Mar 3, 2026

adjordje-amd force-pushed the users/adjordje-amd/pmc-collector branch from ddfae4b to 13392da Compare March 10, 2026 15:13

adjordje-amd changed the title ~~[DNM] [rocprofiler-systems] AMD SMI collector using policy-based design~~ [rocprofiler-systems] AMD SMI collector using policy-based design Mar 10, 2026

mradosav-amd reviewed Mar 12, 2026

View reviewed changes

...ts/rocprofiler-systems/source/lib/rocprof-sys/library/pmc/collectors/nic/perfetto_policy.hpp Outdated Show resolved Hide resolved

mradosav-amd reviewed Mar 12, 2026

View reviewed changes

...ts/rocprofiler-systems/source/lib/rocprof-sys/library/pmc/collectors/nic/perfetto_policy.hpp Outdated Show resolved Hide resolved

mradosav-amd reviewed Mar 12, 2026

View reviewed changes

projects/rocprofiler-systems/source/lib/rocprof-sys/library/pmc/collectors/nic/device.hpp Outdated Show resolved Hide resolved

mradosav-amd reviewed Mar 12, 2026

View reviewed changes

...ects/rocprofiler-systems/source/lib/rocprof-sys/library/pmc/collectors/base/traits_check.hpp Outdated Show resolved Hide resolved

adjordje-amd added 2 commits March 12, 2026 16:59

adjordje-amd force-pushed the users/adjordje-amd/pmc-collector branch from 13392da to 3710324 Compare March 12, 2026 17:56

adjordje-amd added 4 commits March 12, 2026 18:57

Merge branch 'develop' into users/adjordje-amd/pmc-collector

e85e8b5

adjordje-amd force-pushed the users/adjordje-amd/pmc-collector branch from e946934 to 6dff09c Compare March 12, 2026 19:57

Fix formatting

6055620

dgaliffiAMD reviewed Mar 12, 2026

View reviewed changes

adjordje-amd force-pushed the users/adjordje-amd/pmc-collector branch from c8a3965 to 27dd089 Compare March 13, 2026 11:35

Fix unit tests, remove get_use_cache_output

b815fa2

adjordje-amd force-pushed the users/adjordje-amd/pmc-collector branch from 27dd089 to b815fa2 Compare March 13, 2026 15:31

adjordje-amd added 4 commits March 13, 2026 16:44

Cleanup

a6deb21

WIP: refactoring before type erasure implementation

a2e14be

Save point before implementing device_view type erasure. Fix DeviceTest unit tests failing due to missing mock expectation for get_gpu_asic_info(), which is now called during device initialization to populate vendor/product names.

adjordje-amd force-pushed the users/adjordje-amd/pmc-collector branch from 013b786 to cfa3744 Compare March 17, 2026 13:45

adjordje-amd force-pushed the users/adjordje-amd/pmc-collector branch from cfa3744 to ff2044c Compare March 17, 2026 13:54

adjordje-amd added 3 commits March 19, 2026 11:20

Fix AINIC dependency

af8f1d0

Change function used to gather devices

ae71e2b

adjordje-amd force-pushed the users/adjordje-amd/pmc-collector branch from cbaf852 to 5c708a2 Compare March 19, 2026 18:49

Fix SDMA tests in non-supported env

5879cac

adjordje-amd force-pushed the users/adjordje-amd/pmc-collector branch from 5c708a2 to 5879cac Compare March 20, 2026 10:21

adjordje-amd added 5 commits March 20, 2026 12:58

Cleanup

b7bf221

Add missing unit tests, bugfixes

bdd33e5

Renam AINIC pmc metrics

33c1217

Code cleanup

79d82b7

Merge branch 'develop' into users/adjordje-amd/pmc-collector

a06025c

mradosav-amd reviewed Mar 24, 2026

View reviewed changes

projects/rocprofiler-systems/source/lib/rocprof-sys/library/pmc/collectors/nic/device.hpp Outdated Show resolved Hide resolved

mradosav-amd reviewed Mar 24, 2026

View reviewed changes

projects/rocprofiler-systems/source/lib/rocprof-sys/library/rocm.cpp Outdated Show resolved Hide resolved

mradosav-amd approved these changes Mar 24, 2026

View reviewed changes

dgaliffiAMD reviewed Mar 24, 2026

View reviewed changes

projects/rocprofiler-systems/source/lib/rocprof-sys/library/pmc/collectors/base/collector.hpp Outdated Show resolved Hide resolved

marantic-amd approved these changes Mar 26, 2026

View reviewed changes

Update copyright header

3a0eec9

dgaliffiAMD reviewed Mar 26, 2026

View reviewed changes

.../rocprofiler-systems/source/lib/rocprof-sys/library/pmc/collectors/gpu/tests/test_device.cpp Show resolved Hide resolved

adjordje-amd added 3 commits March 30, 2026 13:40

Merge branch 'develop' into users/adjordje-amd/pmc-collector

77b71d0

Address comments

206926d

adjordje-amd merged commit 5e28f59 into develop Mar 30, 2026
52 of 54 checks passed

adjordje-amd deleted the users/adjordje-amd/pmc-collector branch March 30, 2026 14:59

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[rocprofiler-systems] AMD SMI collector using policy-based design#3703

[rocprofiler-systems] AMD SMI collector using policy-based design#3703
adjordje-amd merged 26 commits intodevelopfrom
users/adjordje-amd/pmc-collector

adjordje-amd commented Mar 3, 2026 •

edited by dgaliffiAMD

Loading

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

dgaliffiAMD left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

dgaliffiAMD left a comment

Uh oh!

Uh oh!

marantic-amd left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

adjordje-amd commented Mar 3, 2026 • edited by dgaliffiAMD Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Motivation

Technical Details

JIRA ID

Test Plan

Test Result

Submission Checklist

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

dgaliffiAMD left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

dgaliffiAMD left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

marantic-amd left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

adjordje-amd commented Mar 3, 2026 •

edited by dgaliffiAMD

Loading