fix: make rocProfiler-SDK temporal ordering check non-fatal by default#479
Open
mapatel-amd wants to merge 1 commit into
Open
fix: make rocProfiler-SDK temporal ordering check non-fatal by default#479mapatel-amd wants to merge 1 commit into
mapatel-amd wants to merge 1 commit into
Conversation
The external_correlation_id_request example threw a std::runtime_error whenever a correlation ID appeared to retire before its operation's end timestamp. GPU dispatch and CPU-side retirement timestamps come from different clock domains; on RDNA3.5 APUs (gfx1151) they skew by up to ~200 us, so a correctly-ordered retirement looks early and the example aborts. rocprofiler itself only warns and swaps such inversions by default, failing only when ROCPROFILER_CI_STRICT_TIMESTAMPS is set. Mirror that policy: always report the violation and count, but only throw when ROCPROFILER_CI_STRICT_TIMESTAMPS is set. The unseen/unretired correlation-id invariants remain unconditional. Documented in the README. Co-Authored-By: Claude <noreply@anthropic.com>
zichguan-amd
approved these changes
Jul 13, 2026
zichguan-amd
left a comment
zichguan-amd
left a comment
Collaborator
There was a problem hiding this comment.
LGTM with comments
| { | ||
| const char* strict_ts_env = std::getenv("ROCPROFILER_CI_STRICT_TIMESTAMPS"); | ||
| const bool strict_timestamps | ||
| = (strict_ts_env != nullptr && strict_ts_env[0] != '\0' && strict_ts_env[0] != '0'); |
Collaborator
There was a problem hiding this comment.
Maybe follow the same rules here and extract to a helper in Common utils? https://github.com/ROCm/rocm-systems/blob/6901b670c3e186f9c7052d6fc9748f0286536bdc/projects/rocprofiler-sdk/source/lib/common/environment.cpp#L93-L111, otherwise we could comment that this slightly differs from upstream behaviour.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
external_correlation_id_requestaborted whenever a correlation ID appeared to retire before its operation's end timestamp.looks early and the example throws. gfx1100 (discrete) is unaffected.
ROCPROFILER_CI_STRICT_TIMESTAMPS. This mirrors that policy: always print theviolation + count, only throw when that env var is set. The
unseen/unretiredinvariants stay unconditional.Evidence
Nightly CI on gfx1151:
terminate called ... what(): temporal ordering violation in correlation id retirementObserved skews: 1524 ns, 99057 ns, 209100 ns. Preceded by rocprofiler's own
hsa_amd_profiling_get_dispatch_time ... start time is after end time ... Swapping the valueswarning.
https://github.com/ROCm/rocm-examples/actions/runs/28764875296
Test plan
ROCPROFILER_CI_STRICT_TIMESTAMPS=1: still aborts on violation