[amd-vkale]

Motivation

Under rocprofv3 --replay-mode kernel --kernel-replay-beta-enabled, rocpd reports a counter at N times its value when N --pmc groups list it, for example a sanity counter such as SQ_WAVES in every group (the kernel-replay integration test does this on purpose). rocpd is the default output format, so this reaches the counters_collection view, the counter CSV and Perfetto output that rocpd convert builds from it, and tools that read the view, such as rocprofiler-compute. Counters listed in a single group, and runs without kernel replay, are correct.

Technical Details

Every replay pass of a dispatch shares the dispatch's id, so the rocpd writer puts every pass's counter rows under the dispatch's single event, and replay_pass was dropped. counters_collection groups by dispatch and counter name and takes SUM(value) to add up the counter's hardware instances, so it added the passes together as well.

• source/lib/output/generateRocpd.cpp: rows from passes after the first carry {"replay_pass":N} in rocpd_pmc_event.extdata. Pass 0 and non-replay rows bind {}, the column default, so their content is unchanged; binding it on every row keeps the insert batch on one column list.
• source/share/rocprofiler-sdk-rocpd/data_views.sql: counters_collection divides the sum by the number of distinct passes that collected the counter, i.e. it reports the mean over passes of each pass's instance sum. Untagged rows count as pass 0, so non-replay data and databases written before this change read exactly as before.
• No table change and no schema version bump: readers rebuild views from their own SQL, and a 3.0.4 reader without this view still opens the new databases (it reports the old sum). Recorded in the versions.yml history.
• Docs: kernel-replay how-to (rocpd section), rocpd README, CHANGELOG.

The mean is exact for deterministic counters, since every pass runs against the same restored inputs, and uses every measurement for the others. The individual passes stay in the database.

Issue Tracking

JIRA ID: ROCM-30873

Test Plan

• The kernel-replay rocprofv3 tests also write rocpd now, and validate.py checks every counters_collection row against the mean of that counter's per-pass JSON values, in both the plain and the interval run.
• rocpd-back-compat: the latest-schema database gains a counter in three tagged replay passes with different per-pass sums, a single-pass counter, and an untagged dispatch; the converted CSV must report the means.
• Existing suites: rocprofv3-test-kernel-replay*, rocprofv3-test-rocpd*, rocpd-back-compat, rocpd-api, rocpd help tests.

Test Result

On an 8x MI325X (gfx942) node, Ubuntu 22.04, ROCm 7.14 runtime:

• develop, three groups sharing SQ_WAVES and SQ_INSTS_VALU: dispatch 1 reports SQ_WAVES 49152 and SQ_INSTS_VALU 1818624 for per-pass values of 16384 and 606208 (exactly 3x); 20 of 50 (dispatch, counter) pairs are wrong. With this change, 0 of 50, both in the database view and in the rocpd convert CSV.
• Both new checks fail on develop's output (kernel replay: 1818624 against 606208; back-compat: 54 against 18) and pass with the fix.
• rocprofv3-test-kernel-replay* 7/7, rocprofv3-test-rocpd* 36/36, and rocpd-back-compat, rocpd-api, rocpd help and the kernel-replay CLI unit test 68/68.
• View cost: the rocpd CSV query over 3.2M counter rows takes 8.0-8.3 s instead of 7.5-7.9 s, with identical rows for non-replay data.
• clang-format 11, black 26, flake8 and cmake-format clean.

Submission Checklist

• Look over the contributing guidelines at https://github.com/ROCm/rocm-systems/blob/develop/CONTRIBUTING.md.

[therock-pr-bot]

✅ All Checks Passed — Ready for Review

Check	Status	Details
📝 PR Description	✅ Pass	—
⛔ Forbidden Files	✅ Pass	—
🧪 Unit Test	⚠️ Warning	Error: Source/code files changed without an accompanying unit test. Expected: add at least one test file named like test_<name>.py / test_<name>.cpp (or <name>_test.*). Current: code file(s) changed: projects/rocprofiler-sdk/source/bin/rocprofv3.py, projects/rocprofiler-sdk/source/lib/output/generateRocpd.cpp, projects/rocprofiler-sdk/tests/rocpd-back-compat/make_db.py, projects/rocprofiler-sdk/tests/rocpd-back-compat/validate.py, projects/rocprofiler-sdk/tests/rocprofv3/kernel-replay/conftest.py (+1 more); no test file found
🔎 pre-commit	✅ Pass	—
🚫 Draft PR	🔜 To Be Enabled	—
🚩 Feature Flag	🔜 To Be Enabled	—
📊 Code Coverage	🔜 To Be Enabled	—
🤖 therock-pr-bot	✅ Pass	—

🎉 All checks passed! This PR is ready for review.

📖 Need help? See the Policy FAQ for details on every check and how to fix failures.

🙋 Wish to Override Policy?

[therock-pr-bot]

🎉 All checks passed! This PR is ready for review.

[amd-vkale]

@copilot please give a detailed review of this.

[Copilot]

🟡 Changes recommended

Perfetto PMC annotations still expose raw replay-pass rows despite the documented averaging behavior.

1 open finding

• Perfetto PMC annotations skip per-pass mean aggregation · New

What changed in this PR

Fixes inflated rocPD counters when the same counter is collected across kernel-replay passes.

Changes:

• Tags PMC rows with their replay pass and averages pass totals in counters_collection.
• Adds kernel-replay and backward-compatibility tests.
• Updates documentation and release notes.

File	Description
tests/​rocprofv3/​kernel-replay/​validate.py	Validates averaged rocPD counters.
tests/​rocprofv3/​kernel-replay/​conftest.py	Adds rocPD database fixture.
tests/​rocprofv3/​kernel-replay/​CMakeLists.txt	Generates and validates rocPD output.
tests/​rocpd-back-compat/​validate.py	Checks converted counter means.
tests/​rocpd-back-compat/​make_db.py	Adds representative replay-pass data.
source/​share/​rocprofiler-sdk-rocpd/​versions.yml	Records the behavioral change.
source/​share/​rocprofiler-sdk-rocpd/​data_views.sql	Averages counters across distinct passes.
source/​lib/​python/​rocpd/​README.md	Documents aggregation semantics.
source/​lib/​output/​generateRocpd.cpp	Stores replay-pass metadata.
source/​docs/​how-to/​using-kernel-replay-rocprofv3.rst	Documents rocPD replay behavior.
CHANGELOG.md	Announces the fix.

🧠 Review effort: Balanced

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

@copilot please give a detailed review of this.

Review of 385f45e6

No high-confidence, commit-introduced correctness bugs found. The fix is appropriately scoped: it preserves replay identity in the writer and corrects aggregation at the shared consumer view. No file changes were needed for this review.

Correctness

• Writer: Each hardware-instance row receives its record’s numeric replay-pass index. Pass 0 and ordinary collection retain {}. The pass identity already comes from callback user data, not callback arrival order. The insertion machinery owns the string and binds it with SQLITE_TRANSIENT, so the local extdata string does not introduce a lifetime defect. Always binding the column preserves the PMC batch layout; existing batching also handles a later switch to SPM columns.
• Aggregation: SUM(value) / COUNT(DISTINCT pass) is algebraically the mean of per-pass instance sums—not the mean of individual instances. The denominator is calculated within each dispatch/counter group, so a counter collected only in a later pass remains unchanged, and a counter present in only some passes is divided by those passes rather than by the total replay count. GUID and agent remain in the grouping, preventing unrelated captures from being averaged together. Untagged rows map to pass 0. SPM samples remain excluded.
• Numeric behavior: The stored value has REAL affinity, avoiding integer truncation of fractional means. Zero-valued measurements still count as collected passes.

Compatibility and consumers

The existing extdata column accommodates the metadata without a table migration. The importer rebuilds temporary views from its installed, version-specific SQL; schema 3.0.4 selects the revised view, while historical schema definitions remain unchanged. CSV reads counters_collection directly, and Perfetto obtains its counter input from the same view.

Two compatibility limits are important: old replay databases without pass tags cannot be repaired by this change, and an older reader rebuilding its original 3.0.4 view will still sum newly tagged measurements. Structural readability is preserved, but that does not imply corrected values with old readers. Raw PMC rows continue to contain every measurement; consumers bypassing the aggregation view must interpret the tags themselves.

Tests and evidence

The committed GPU checks compare every (dispatch, counter) against the JSON per-pass mean in both ordinary and interval runs. The synthetic fixture is particularly useful because unequal sums of 16, 18, and 20 distinguish averaging from simply choosing one pass; it also checks a single-pass counter and an untagged dispatch.

I independently exercised the existing fixture and actual SQL in memory with SQLite 3.45.1:

• Revised view: SQ_WAVES 18, single-pass GRBM_COUNT 1000, untagged SQ_WAVES 8.
• Previous view, identical data: SQ_WAVES 54, with the other values unchanged.
• git diff --check HEAD~1 HEAD passed.

I did not rerun the reported GPU suites, native writer, or conversions: this sandbox lacks pytest, a built rocpd extension, and ROCm. Automated parallel validation returned “No changes detected” and skipped, so it supplied no additional review/security-scan evidence.

Nonblocking follow-ups

1. Add a synthetic counter collected in multiple but not all passes, including no pass-0 measurement, with a fractional expected mean.
2. Assert raw rows’ exact pass tags independently of aggregate values.
3. Decode Perfetto output and assert counter values; current back-compat checks establish file existence/nonempty output, not numeric correctness.

The algorithm assumes complete, nonduplicated instance records and valid JSON metadata; it intentionally does not detect missing instances or duplicate measurements. The reported query overhead is plausible for JSON extraction and distinct aggregation, but I did not reproduce the performance benchmark.

[yhuiYH]

We probably want to bump the schema version to identify support for this.

We also added features.py support for backwards compatibility reasons (usually when adding tables or fields to tables). This change seems to only change the data_views, so it may be seamless enough that it doesn't require an added flag to features.py, but just calling it out here: https://github.com/ROCm/rocm-systems/blob/develop/projects/rocprofiler-sdk/source/lib/python/rocpd/features.py#L54-L67

[vedithal-amd]

No impact on rocprofiler-compute:

• Compute doesn't use kernel replay mode, so replay_pass is always 0 and counters_collection returns the same values as before.
• Compute only reads counters_collection, and its columns are unchanged.

[github-actions]

Code Coverage Report

Code Coverage Report

Tests Only

Samples Only

Tests + Samples

[amd-vkale]

@copilot resolve the merge conflicts on this branch.

[yhuiYH]

Looks like your back-compat tests are still failing on RHEL and SLES.

The following tests FAILED:
	1011 - tests.integration.validate.rocpd-back-compat.test_counters_collection_schema_unchanged_from_3_0_5 (Failed)
	1012 - tests.integration.validate.rocpd-back-compat.test_pmc_events_replay_pass_absent_before_3_0_6[3.0.0] (Failed)
	1013 - tests.integration.validate.rocpd-back-compat.test_pmc_events_replay_pass_absent_before_3_0_6[3.0.1] (Failed)
	1014 - tests.integration.validate.rocpd-back-compat.test_pmc_events_replay_pass_absent_before_3_0_6[3.0.2] (Failed)
	1015 - tests.integration.validate.rocpd-back-compat.test_pmc_events_replay_pass_absent_before_3_0_6[3.0.3] (Failed)
	1016 - tests.integration.validate.rocpd-back-compat.test_pmc_events_replay_pass_absent_before_3_0_6[3.0.4] (Failed)
	1017 - tests.integration.validate.rocpd-back-compat.test_pmc_events_replay_pass_absent_before_3_0_6[3.0.5] (Failed)
	1018 - tests.integration.validate.rocpd-back-compat.test_pmc_events_report_replay_pass (Failed)

Also, you need to run make format in the build directory to fix the linting and cmake test failures.

[amd-vkale]

@copilot Fix the conflicts

[amd-vkale]

@copilot resolve the merge conflicts in this pull request

[amd-vkale]

Looks like your back-compat tests are still failing on RHEL and SLES.

The following tests FAILED:
	1011 - tests.integration.validate.rocpd-back-compat.test_counters_collection_schema_unchanged_from_3_0_5 (Failed)
	1012 - tests.integration.validate.rocpd-back-compat.test_pmc_events_replay_pass_absent_before_3_0_6[3.0.0] (Failed)
	1013 - tests.integration.validate.rocpd-back-compat.test_pmc_events_replay_pass_absent_before_3_0_6[3.0.1] (Failed)
	1014 - tests.integration.validate.rocpd-back-compat.test_pmc_events_replay_pass_absent_before_3_0_6[3.0.2] (Failed)
	1015 - tests.integration.validate.rocpd-back-compat.test_pmc_events_replay_pass_absent_before_3_0_6[3.0.3] (Failed)
	1016 - tests.integration.validate.rocpd-back-compat.test_pmc_events_replay_pass_absent_before_3_0_6[3.0.4] (Failed)
	1017 - tests.integration.validate.rocpd-back-compat.test_pmc_events_replay_pass_absent_before_3_0_6[3.0.5] (Failed)
	1018 - tests.integration.validate.rocpd-back-compat.test_pmc_events_report_replay_pass (Failed)

Also, you need to run make format in the build directory to fix the linting and cmake test failures.

This should be fixed now. The make format in the build directory is done.