Skip to content

fix(rocprofiler-sdk) fix pc sampling microsecond interval being clamped - #10247

Merged
amd-justchen merged 1 commit into
developfrom
users/scrater/microsecond-interval-clamp-fix
Sep 3, 2026
Merged

fix(rocprofiler-sdk) fix pc sampling microsecond interval being clamped#10247
amd-justchen merged 1 commit into
developfrom
users/scrater/microsecond-interval-clamp-fix

Conversation

@samcrater-amd

@samcrater-amd samcrater-amd commented Aug 17, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Motivation

PR8431 had the unintended effect of also limiting the maximum number of microseconds users could specify when using PC sampling.

Technical Details

Gated clamp behind a check on interval type.

JIRA ID

JIRA ID : AIPROFSDK-913

Test Plan

ran rocprofv3-avail info --pc-sampling to check valid intervals

Test Result

time-based interval was unbounded after fix

Submission Checklist

@samcrater-amd
samcrater-amd requested review from a team as code owners August 17, 2026 18:06
@therock-pr-bot

therock-pr-bot Bot commented Aug 17, 2026 •
edited
Loading

Copy link
Copy Markdown

✅ All Policy Checks Passed

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/lib/rocprofiler-sdk/pc_sampling/ioctl/ioctl_adapter.cpp; no test file found
🚫 Draft PR 🔜 To Be Enabled —
🚩 Feature Flag 🔜 To Be Enabled —
📊 Code Coverage 🔜 To Be Enabled —

🎉 All policy checks passed!

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

🙋 Wish to Override Policy?

@therock-pr-bot

therock-pr-bot Bot commented Aug 17, 2026

Copy link
Copy Markdown

🚫 Please fix the failed policies before requesting reviews.

The following policy checks failed:

  • ❌ PR Description

The Not ready to Review label has been added to this PR.
Once all policies pass, the label will be removed automatically.

MythreyaK
MythreyaK approved these changes Aug 17, 2026
View reviewed changes

@MythreyaK MythreyaK left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM but a unit test for user-facing behavior might be useful.

vlaindic
vlaindic approved these changes Aug 17, 2026
View reviewed changes

@vlaindic vlaindic left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM, thanks!

@vlaindic

vlaindic commented Aug 17, 2026 •
edited
Loading

Copy link
Copy Markdown
Member

LGTM but a unit test for user-facing behavior might be useful.

Agree, could you maybe open an issue/JIRA to track this, as it's not of a high importance for now? @samcrater-amd

@github-actions

github-actions Bot commented Aug 17, 2026

Copy link
Copy Markdown
Contributor

Code Coverage Report

Code Coverage Report

Tests Only

code coverage tests.png

Samples Only

code coverage samples.png

Tests + Samples

code coverage all.png

@samcrater-amd
samcrater-amd force-pushed the users/scrater/microsecond-interval-clamp-fix branch from 8a6ec1a to 8d160b3 Compare August 18, 2026 13:53
@samcrater-amd
samcrater-amd force-pushed the users/scrater/microsecond-interval-clamp-fix branch 3 times, most recently from 735f7ad to 655252e Compare August 31, 2026 15:46
@samcrater-amd
samcrater-amd force-pushed the users/scrater/microsecond-interval-clamp-fix branch from 655252e to 6a34435 Compare September 2, 2026 14:19
@amd-justchen

amd-justchen commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

Will merge given code owner approval.
The TheRock CI failures are only known infra issues, unrelated to the rocprofiler-sdk pc-sampling microsecond-interval clamp fix:

  • Test rocprofiler-sdk (gfx94X): known infra test-timeout, fails across unrelated PRs

@amd-justchen
amd-justchen merged commit 61331e3 into develop Sep 3, 2026
46 of 52 checks passed
@amd-justchen
amd-justchen deleted the users/scrater/microsecond-interval-clamp-fix branch September 3, 2026 22:40
@systems-assistant

systems-assistant Bot commented Sep 8, 2026

Copy link
Copy Markdown
Contributor

TheRock Submodule Bump Activity

Newest first

@bgopesh bgopesh mentioned this pull request Oct 14, 2026
Merged
1 task done
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ApoKalipse-V ApoKalipse-V ApoKalipse-V approved these changes
@MythreyaK MythreyaK MythreyaK approved these changes
@vlaindic vlaindic vlaindic approved these changes

Assignees

No one assigned

Labels

Not ready to Review organization: ROCm project: rocprofiler-sdk

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@samcrater-amd @vlaindic @amd-justchen @ApoKalipse-V @MythreyaK