refactor(rocprofiler-sdk): migrate SPM off the per-queue callback registry by amd-vkale · Pull Request #8887 · ROCm/rocm-systems

amd-vkale · 2026-07-20T21:50:01Z

Motivation

The purpose of this PR is to isolate the SPM slice of the callback-removal effort (reference PRs #8730 / #8586) into a small, single-service change, per review guidance to land callback removal one service at a time. This is independent of the kernel replay feature in PR #7960 but it aids it.

Underlying Problem

Dispatch SPM currently registers a queue_callbacks_t on every HSA queue. That hides “is this service active?” inside a per-queue map, makes enqueue and completion the same walk, and serializes every GPU while any SPM context is active.

This PR:

Stops registering SPM with that map. The write interceptor and async signal handler call explicit free functions instead (spm::write_hook, signal_completion_hook, is_any_active).
Routes completions by provenance — enter iterates active contexts, exit iterates registered contexts so in-flight dispatches still complete after stop.
Scopes SPM to GPU agents via rocprofiler_spm_dispatch_counting_service_set_agents, with GPU drain on stop and per-agent serialization refcounting.

The per-queue callback registry remains for counters, thread trace, and PC sampling until those services land in their own PRs.

Notes on Bigger Picture

Aids PR #7960 and the kernel replay work tracked under AIPROFSDK-68. This PR also helps other features that require removing the SPM per-queue callback registry, but does not depend on kernel replay landing first.

Sibling migrations: thread trace (#8790), counter collection (#8891), PC sampling (#8895).

Technical Details

Enter = active, exit = registered. Exit self-filters via packet_return_map so in-flight dispatches complete after stop.

flowchart LR
  WriteInterceptor --> WriteHook["spm::write_hook: active + collects_on"]
  AsyncSignalHandler --> CompletionHook["signal_completion_hook: registered"]
  ContextStop --> Drain["enabled=false, queue_controller_sync, unserialize, clear slot"]

Permalinks pin to 758072b.

Hooks and call sites

Declarations: queue_hooks.hpp
Enter: queue_hooks.cpp#L42-L75
Exit (registered / provenance): queue_hooks.cpp#L77-L101
is_any_active: queue_hooks.cpp#L103-L107
no_real_consumers gate: queue.cpp#L318-L322
Enter call site: queue.cpp#L632-L642
Exit call site: queue.cpp#L172-L179
Batching disabled while active: queue.cpp#L778-L779
start_context (no add_callback): core.cpp#L258-L276
Stop (enabled → drain → unserialize): core.cpp#L283-L323
Stop services before active-slot CAS: context.cpp#L392-L408
Conflict uses intersects: context.cpp#L311-L317
set_agents API: spm.h#L318-L338
Producer tag: client_ids.hpp#L37-L40

Reviewer Guide

Confirm enter = active, exit = registered (queue_hooks.cpp).
SPM-only runs still enter WriteInterceptor (queue.cpp#L318-L322).
start_context does not call add_callback (core.cpp#L258-L276).
context::stop_context stops SPM before the active-slot CAS (context.cpp#L392-L408).
spm::stop_context keeps queue_controller_sync (core.cpp#L283-L323) — drain bounds when completions arrive; provenance routing bounds delivery.
In-flight regression: spm_core test stop_context_in_flight_completion_routes_via_hook_path (core.cpp#L990+) — packet_return_map drains via signal_completion_hook after stop.
Per-agent: set_agents_restricts_collection, disjoint SPM contexts (core.cpp#L1128+).
Gate-only unit test: queue_hooks_test.cpp#L33-L36.

Questions to Answer:

In-flight SPM dispatch still gets DISPATCH_END / kfd_stop after stop? packet_return_map empty?
GPU-1-only SPM leave GPU-0 alone?

Out of scope: deleting Queue::_callbacks; other service migrations.

Issue Tracking

JIRA ticket: AIPROFSDK-1018

Related JIRA ticket on kernel replay: AIPROFSDK-68. The kernel replay PR is #7960.

Test Plan

is_any_active
In-flight + per-agent tests in spm/tests/core.cpp
Existing start/stop / sync / restart tests updated off callback iteration

Test Result

Intended coverage listed above. Treat the Checks tab as source of truth for CI.

Submission Checklist

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

therock-pr-bot · 2026-07-20T21:50:50Z

✅ All Policy Checks Passed

Check	Status	Details
📝 PR Description	✅ Pass	—
⛔ Forbidden Files	✅ Pass	—
🧪 Unit Test	✅ Pass	—
🚫 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 · 2026-07-20T21:50:52Z

🚫 Please fix the failed policies before requesting reviews.

The following policy checks failed:

❌ Unit Test

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

bwelton

Correctness / lifetime: in-flight SPM dispatches lose completion handling when a context is stopped — same root cause as #8891

Nice split of the SPM migration. The no_real_consumers/should_batch/queue_id cleanup all look correct. Two blocking issues, both stemming from the same liveness-vs-provenance change flagged in #8891:

1. In-flight completions are dropped during stop. The new signal_completion_hook (spm/queue_hooks.cpp:76-87) routes by re-querying get_active_contexts(active_spm_contexts_filter()), but context::stop_context nulls the active slot (context/context.cpp:371) before calling spm::stop_context (context.cpp:383). spm::stop_context then drains in-flight kernels via queue_controller_sync() (spm/core.cpp:293) — but the context is already invisible, so those drained completions see zero active SPM contexts and post_kernel_call never runs. That skips the DISPATCH_END record callback, kfd_stop(), and barrier-signal cleanup, and leaks/stales the packet_return_map entry (spm/dispatch_handlers.cpp:239-274).

This is a regression: on develop, stop_context ran queue_controller_sync() while the registry callback was still registered, removing it only after the drain — so in-flight completions were routed correctly.

2. Serialization gap during stop/drain. pre_kernel_call's disabled path returns {empty, serialize=true} specifically to coordinate the serialized→unserialized transition (spm/dispatch_handlers.cpp:144). But write_hook now gates on get_active_contexts (spm/queue_hooks.cpp:52), which goes empty at context.cpp:371 — before disable_serialization() runs. So during the stop/drain window a new dispatch bypasses SPM serialization and can run concurrently with an in-flight SPM dispatch.

Same fix direction as #8891 (this PR shares its structure, and both should be fixed consistently): route completion by packet provenance (the packet_return_map / SPM_CLIENT_ID ownership already available) rather than current activeness, and keep SPM visible to write_hook/serialization via a "draining" state until queue_controller_sync() + disable_serialization() complete.

Test coverage: spm/tests/queue_hooks_test.cpp only checks is_any_active() == false, and tests/core.cpp calls pre_/post_kernel_call directly (bypassing the routing layer), so neither catches this. Please add a hook-path test that stops the context with a dispatch in flight and asserts post_kernel_call still ran (DISPATCH_END fired, map drained).

Non-blocking: there's no conflict guard rejecting a second concurrent dispatch_spm context (unlike dispatch_counter_collection at context.cpp:292), so the hooks iterate all active SPM contexts — pre-existing, but any completion fix should preserve per-origin routing.

SrirakshaNag · 2026-07-29T19:02:05Z


    if(controller)
    {
        hsa::queue_controller_sync();


Now that we do not have per-queue callback to remove, do we still need hsa::queue_controller_sync()?

I actually think we could keep this for now as it would conflict with Ben's suggestion in PR #8891 (on liveness-based instead provenance-based drain). See the changes for the remove callbacks counter collection PR (https://github.com/ROCm/rocm-systems/pull/9586/changes?w=1). Once I get that worked out, I will come back to this - and I think we can merge your suggestion on removing hsa:queue_controller_sync() with his.

This is by Cursor, but I (amd-vkale) own this post.

Coming back to this as promised. The conclusion is to keep hsa::queue_controller_sync(), and the reasoning is now written into the code in #11870.

The sync originally served two purposes, and only one of them is obsolete:

Avoiding dangling callback pointers once the per-queue callbacks were unregistered. This one is genuinely gone — after the migration there is no per-queue callback left to remove, which is exactly the point you raised.

Letting in-flight dispatches complete. This one survives, and it is weaker than it looks to drop. Provenance routing guarantees that a completion which arrives is delivered to the right context, but it says nothing about when completions arrive. The drain is what bounds that, and the rest of the teardown depends on the bound.

The ordering around the call is also load-bearing, which is worth flagging since it is not visible at the call site. context::stop_context invokes this while the context is still in the active list, so for the duration of the drain the enter hook still reaches pre_kernel_call, whose disabled path deliberately returns serialize=true in order to coordinate the serialized-to-unserialized transition. Only once this returns does the active slot get cleared. If the slot were cleared first, the enter hook would go blind before disable_serialization() ran, and a dispatch could be submitted without serializer packets while the serializer was still enabled.

SrirakshaNag · 2026-07-30T14:19:59Z

+// Only distinctness matters; values are fixed for test-order stability.
+// Services are migrated off the per-queue callback registry one at a time; the
+// unused ids are reserved so the tag values stay stable across those PRs.
+constexpr int64_t COUNTERS_CLIENT_ID     = 1;


Can we have an enum here instead?

Yes, I think an enum is reasonable and corresponds to @vlaindic 's suggestion in PR #8891 for removing
counter collection callbacks. See here: #8891 (comment).

I will also be making sure these changes are consistent across all 4 remove-callback PRs.

This is by Cursor, but I (amd-vkale) own this post.

Done — this is implemented in #11870 (commit 836d8bcfb1), which is stacked on this branch.

client_ids.hpp is now an enum client_id : int64_t:

enum client_id : int64_t { COUNTERS_CLIENT_ID = -1001, THREAD_TRACE_CLIENT_ID = -1002, PC_SAMPLING_CLIENT_ID = -1003, SPM_CLIENT_ID = -1004, };

It is deliberately unscoped rather than an enum class, so the implicit conversion to hsa::ClientID is preserved and the emplace sites need no cast, while the values still group under one type.

A static_assert keeps them negative, which is what holds them disjoint from the strictly positive ids QueueController::add_callback hands out counting up from 1. The two ranges have to stay separate until the per-queue callback registry is gone, since services that have not migrated yet still receive registry ids. As noted above, the same change is applied consistently across all four remove-callback PRs.

@vlaindic

Applies the changes requested in review of the base PR, so that merging this stacked PR clears them on users/vkale/remove-callbacks-counters. Renames, per @vlaindic's comment that 'write' does not convey that this is a write interceptor hook, and per the name agreed in the reply on that thread: counters::write_hook -> counters::kernel_dispatch_phase_enter_hook counters::signal_completion_hook -> counters::kernel_dispatch_phase_exit_hook The exit hook is renamed for symmetry even though only the enter hook was flagged; leaving one half renamed reads worse than either alternative, and the paired names make the enqueue-versus-completion asymmetry this branch introduces easier to follow. is_any_active keeps its name, being neither phase. SPM mirrors these names on its own branch and is deliberately left alone until the review of #8887 settles, so the two branches are briefly inconsistent by choice rather than by oversight. client_ids.hpp: the four constants become an unscoped enum with int64_t as its fixed underlying type. That answers the question asked in review -- the int64_t was not arbitrary, it matches hsa::ClientID, which is how the tag is stored in inst_pkt_t. Unscoped rather than enum class so the implicit conversion to ClientID survives and no emplace site needs a cast. The regression test now constructs the pair piecewise instead of through std::make_pair, which would deduce the tag's enum type and lean on a pair-to-pair conversion; piecewise converts the tag directly and matches the production hook. Copyright headers on the four files the base PR adds move from 2025 to 2026. Verified without a build, which is not available here: clang-format 11.1.0 (the version the repo's .clang-format pins) leaves no unrelated churn, no references to the old names remain, and a standalone translation unit confirms the enum converts to ClientID in both the piecewise and make_pair forms. Co-authored-by: Vivek Kale <vivek.kale@amd.com>

…s not address Comment-only. Names the two blocking issues raised in the review of #8887 and how each is handled, so a reader does not have to reconstruct it from the review. spm/queue_hooks.cpp: states the consequence of dropping an in-flight completion -- the DISPATCH_END record, kfd_stop() and barrier-signal cleanup are all skipped and the map entry leaks -- and records why widening to registered contexts stays per-origin: each context's post_kernel_call only claims packets in its own packet_return_map. That matters because, unlike dispatch_counter_collection, there is no conflict guard rejecting a second concurrent dispatch_spm context. spm/core.cpp: marks the drain removal as a deliberate divergence from the review, which asked for the sync to be kept plus an explicit draining state. The serialization window that state would protect is closed instead by context::stop_context running this function before clearing the active slot, so disable_serialization happens while the enter hook can still see the context. The two approaches are not obviously equivalent, so the reasoning is recorded rather than left implicit, and it should be revisited if that ordering changes. Co-authored-by: Vivek Kale <vivek.kale@amd.com>

… priority Restores the queue_controller_sync() an earlier commit on this branch removed. The review of #8887 asked for it to be kept, with SPM held visible to the enter hook and to serialization through a draining window until it and disable_serialization() complete. That takes precedence over the later suggestion to remove it. The removal argued that the sync's two original purposes were addressed by the migration: dangling callback pointers, and letting in-flight dispatches complete. The first is genuinely gone -- nothing is registered any more. The second does not survive scrutiny: provenance routing guarantees that a completion which arrives is delivered, whereas the drain bounds when completions arrive at all, and the rest of the teardown depends on the latter. The comment now records both the request and why the counter-argument is weaker than it appeared, so this is not relitigated silently. No draining flag is introduced, because the visibility half already comes from ordering: context::stop_context calls this function while the context is still in the active list, so throughout the drain the enter hook still reaches pre_kernel_call, whose disabled path deliberately returns serialize=true to coordinate the serialized->unserialized transition, and the slot is cleared only after this function returns. The comment marks that ordering as load-bearing -- if the slot were cleared first, the enter hook would go blind before disable_serialization() ran and a dispatch could be submitted unserialized while the serializer was still enabled. Also brings this branch's own earlier additions into clang-format compliance; they had not been formatted. Verified with clang-format 11.1.0, the version the repo pins, and confined to lines this branch adds -- nothing pre-existing is reformatted. Not compiled here -- no ROCm toolchain in this environment. Co-authored-by: Vivek Kale <vivek.kale@amd.com>

@vlaindic

Applies the changes requested in review of the base PR, so that merging this stacked PR clears them on users/vkale/remove-callbacks-counters. Renames, per @vlaindic's comment that 'write' does not convey that this is a write interceptor hook, and per the name agreed in the reply on that thread: counters::write_hook -> counters::kernel_dispatch_phase_enter_hook counters::signal_completion_hook -> counters::kernel_dispatch_phase_exit_hook The exit hook is renamed for symmetry even though only the enter hook was flagged; leaving one half renamed reads worse than either alternative, and the paired names make the enqueue-versus-completion asymmetry this branch introduces easier to follow. is_any_active keeps its name, being neither phase. SPM mirrors these names on its own branch and is deliberately left alone until the review of #8887 settles, so the two branches are briefly inconsistent by choice rather than by oversight. client_ids.hpp: the four constants become an unscoped enum with int64_t as its fixed underlying type. That answers the question asked in review -- the int64_t was not arbitrary, it matches hsa::ClientID, which is how the tag is stored in inst_pkt_t. Unscoped rather than enum class so the implicit conversion to ClientID survives and no emplace site needs a cast. The regression test now constructs the pair piecewise instead of through std::make_pair, which would deduce the tag's enum type and lean on a pair-to-pair conversion; piecewise converts the tag directly and matches the production hook. Copyright headers on the four files the base PR adds move from 2025 to 2026. Verified without a build, which is not available here: clang-format 11.1.0 (the version the repo's .clang-format pins) leaves no unrelated churn, no references to the old names remain, and a standalone translation unit confirms the enum converts to ClientID in both the piecewise and make_pair forms. Co-authored-by: Vivek Kale <vivek.kale@amd.com>

amd-vkale · 2026-09-19T00:02:42Z

@copilot resolve the merge conflicts in this pull request

amd-vkale · 2026-09-21T17:25:49Z

@copilot resolve the merge conflicts on this branch.

Marker commit only; no file changes. It keeps ROCm#8887 open as a distinct pull request now that its base is users/vkale/remove-callbacks-spm--with-cursor-fixes, which already contains every commit on this branch.

amd-vkale · 2026-09-21T20:04:07Z

Status for reviewers: this branch's head is now the complete change. The stacked fix PR has been merged into it, so there is nothing else to pull in to review #8887.

#11870 (serializer creation race, transition-ordering race, queue hook tag range) is merged into users/vkale/remove-callbacks-spm. This branch has also had develop merged in and the conflicts resolved.

@SrirakshaNag — both of your comments are addressed on the current head:

"Can we have an enum here instead?" — done. hsa/queue_hooks/client_ids.hpp is now an enum client_id : int64_t with COUNTERS_CLIENT_ID, THREAD_TRACE_CLIENT_ID, PC_SAMPLING_CLIENT_ID and SPM_CLIENT_ID, plus a static_assert holding the values negative. The values have to stay negative because they share the inst_pkt_t tag space with the per-queue callback registry, and QueueController::add_callback hands out strictly positive ids counting up from 1; the two ranges must stay disjoint until the registry is gone. The enum is deliberately unscoped so the implicit conversion to ClientID survives and the emplace sites need no cast.

"Do we still need hsa::queue_controller_sync()?" — it is kept, and the reasoning is now written into spm::stop_context rather than living only in this thread. Short version: the sync originally served two purposes, and only one of them went away with the migration. Avoiding dangling callback pointers once the per-queue callbacks are unregistered is genuinely gone. Letting in-flight dispatches complete is not — provenance routing guarantees that a completion which arrives is delivered, but the drain is what bounds when completions arrive at all, and the rest of the teardown depends on that bound.

The visibility half of that comes from ordering rather than a separate draining flag: context::stop_context calls spm::stop_context while the context is still in the active list, so for the duration of the drain the enter hook still reaches pre_kernel_call, whose disabled path returns serialize=true to coordinate the serialized-to-unserialized transition. The active slot is only cleared once stop_context returns. That ordering is load-bearing — if the slot were cleared first, the enter hook would go blind before disable_serialization() ran, and a dispatch could be submitted unserialized while the serializer was still enabled.

For completeness on the migration itself: SPM no longer touches the callback registry anywhere. There is no add_callback or remove_callback left in spm/core.cpp — start_context only scopes serialization to the context's agents, and the HSA write interceptor calls spm::write_hook / spm::signal_completion_hook directly from hsa/queue.cpp.

amd-vkale · 2026-09-21T20:11:30Z

Correction to my previous comment, which said this branch's head was the complete change with nothing else to pull in. That was accurate when I wrote it, but the stack has since been rearranged and it is no longer the right instruction for a reviewer.

Where to review the code: #11968. The SPM migration now lives on users/vkale/remove-callbacks-spm--with-cursor-fixes, a branch in ROCm/rocm-systems rather than in my fork, because CodeQL does not run on pull requests from a fork. #11968 targets develop and carries every commit of the migration.

This PR (#8887) is now stacked on #11968 and contains only a marker commit with no file changes, so the "Files changed" tab here will look empty. It stays open to keep this review history — and in particular the threads with @SrirakshaNag on the client id enum and on hsa::queue_controller_sync() — attached to the work.

Everything described in my previous comment is true of #11968's content; only the location changed. Happy to keep the discussion in this thread and carry conclusions across.

SrirakshaNag · 2026-09-21T19:31:34Z

+        spm_agents.push_back(rocp_agent->id);
+    }
+
+    if(spm_agents.empty()) GTEST_SKIP() << "SPM unavailable";


In other tests GTEST_SKIP is not used to skip tests, I believe. I was wondering if it can be consistent

This is by Cursor, but I (amd-vkale) own this post.

Agreed. Pre-existing tests in this file (check_packet_generation, check_callbacks, stop_context_removes_callbacks, stop_context_sync_and_restart, and the rest) do not use GTEST_SKIP; they log ROCP_ERROR << "SPM unavailable" and return. The ctest registration already uses SKIP_REGULAR_EXPRESSION "SPM unavailable".

The three new sites now follow that idiom: ROCP_ERROR << "SPM unavailable" plus teardown/return (or the in-flight test's existing fail-vs-unavailable branch). No change to what is asserted when an SPM-capable agent is present.

SrirakshaNag · 2026-09-21T19:42:09Z

+    for(size_t i = 0; i < num_agents; ++i)
+    {
+        const auto* agent = rocprofiler::agent::get_agent(agents[i]);
+        if(!agent || agent->type != ROCPROFILER_AGENT_TYPE_GPU)


for agent->type != GPU, the status code can be ROCPROFILER_STATUS_ERROR_INVALID_ARGUMENT instead

This is by Cursor, but I (amd-vkale) own this post.

Split the two checks in spm::set_dispatch_agents (spm/core.cpp). A failed get_agent lookup still returns ROCPROFILER_STATUS_ERROR_AGENT_NOT_FOUND; a resolved agent whose type is not GPU now returns ROCPROFILER_STATUS_ERROR_INVALID_ARGUMENT.

Sibling check: counters set_dispatch_agents on users/vkale/remove-callbacks-counters--with-cursor-fixes still folds both cases into AGENT_NOT_FOUND. PC sampling and thread trace have no equivalent set_agents API. No existing SPM test asserted AGENT_NOT_FOUND for the non-GPU case, so none was changed. Counters will need the same split if we want the four PRs to match.

SrirakshaNag · 2026-09-21T19:47:54Z

+// callback registration used to provide to the HSA write interceptor gate. With
+// no SPM context active, it must report inactive. This requires no GPU / HSA
+// runtime.
+TEST(spm_queue_hooks, is_any_active_false_when_no_context_active)


Is it possible to have an unit test for write hook or it is intended to be tested indirectly?

This is by Cursor, but I (amd-vkale) own this post.

Added spm_queue_hooks.write_hook_noop_when_no_context_active in spm/tests/queue_hooks_test.cpp. It needs no HSA runtime: with no active SPM context, write_hook must leave inst_pkt unchanged and must not clear is_serialized.

To make that path testable the same way as signal_completion_hook, write_hook now takes const hsa::Queue* and returns before dereferencing the queue when no dispatch_spm context is active. The production call in hsa/queue.cpp passes &queue. Counters/PC-sampling still take a reference on their enter hooks; only the exit hooks were already nullable, which is the pattern reused here.

…, test skips, and write_hook coverage Co-authored-by: Cursor <cursoragent@cursor.com>

amd-vkale · 2026-09-21T23:19:23Z

This PR was accidentally merged into its stacked base branch users/vkale/remove-callbacks-spm--with-cursor-fixes rather than into develop, and GitHub does not allow a merged PR to be reopened or retargeted. No SPM work was lost.

Review continues on #11968, which has the identical head commit (4b3ad153b86) and the full diff against develop. Every review item from this PR — including the threads above — is reproduced there with its current status: #11968 (comment)

Apologies for the churn.

…istry (#11968) Stop registering dispatch SPM with the HSA queue controller's per-queue callback registry. The write interceptor and `AsyncSignalHandler` now call `spm::kernel_dispatch_phase_enter_hook` and `spm::kernel_dispatch_phase_exit_hook` explicitly. The enter hook walks active contexts and tags each SPM packet with `SPM_CLIENT_ID`; the exit hook walks registered contexts and picks out its own packets by that tag, so dispatches still in flight when a context stops complete normally. Add `spm::is_active_on_agent` to the `no_real_consumers` gate in `WriteInterceptor`, scoped to the queue's agent, replacing the notifier count the old registration used to bump, and keep per-packet mode while SPM is active on that agent. Scope SPM to GPU agents with the new experimental `rocprofiler_spm_dispatch_counting_service_set_agents()`. Two SPM contexts may be active together when their agent sets are disjoint. Queue-controller serialization is now reference counted per agent, so an agent stays serialized until every service that asked for it (counters, thread trace, SPM) has released it. SPM stop drains the GPU before releasing its hold. Thread trace's `resource_deinit` now releases serialization only if the tracer still held it, so it can no longer drop another service's hold. Other changes: - Read the context registry off a published snapshot, and serialize context start/stop transitions so lifecycle calls cannot act on a half-started or half-stopped context. - Take the contexts mutex in `spm::set_dispatch_agents`. - Add `hsa/queue_hooks/client_ids.hpp`: negative producer tags for migrated services, disjoint from the registry's positive ids. - Guard `rocprofiler_spm_destroy_counter_config` on the aqlprofile ABI. - Add tests: in-flight completion after stop, agent restriction, disjoint and overlapping contexts, concurrent start/stop, the write-interceptor gate, and thread trace's foreign-owner release. Counter collection (#11970) and thread trace (#11967) land after this; PC sampling migrated in #11969. Moved here from the fork-hosted #8887, which keeps the review history. Related: #8586, #8730, #7960 (kernel replay, AIPROFSDK-68) Sibling PRs: #11967 (thread trace), #11970 (counters) Combined view: #12135 JIRA ID: AIPROFSDK-1018 Co-authored-by: Cursor <cursoragent@cursor.com>

amd-vkale assigned amd-vkale and bwelton Jul 20, 2026

github-actions Bot added the project: rocprofiler-sdk label Jul 20, 2026

therock-pr-bot Bot added the Not ready to Review label Jul 20, 2026

systems-assistant Bot added the organization: ROCm label Jul 20, 2026

amd-vkale force-pushed the users/vkale/remove-callbacks-spm branch 4 times, most recently from 8515be4 to 99b69d8 Compare July 20, 2026 23:15

amd-vkale unassigned bwelton Jul 20, 2026

amd-vkale requested review from ApoKalipse-V and bwelton July 20, 2026 23:17

amd-vkale force-pushed the users/vkale/remove-callbacks-spm branch from 99b69d8 to 194cdec Compare July 20, 2026 23:18

amd-vkale marked this pull request as ready for review July 20, 2026 23:18

amd-vkale requested review from a team as code owners July 20, 2026 23:18

bwelton requested changes Jul 23, 2026

View reviewed changes

SrirakshaNag reviewed Jul 29, 2026

View reviewed changes

SrirakshaNag reviewed Jul 30, 2026

View reviewed changes

amd-vkale mentioned this pull request Jul 30, 2026

feat(rocprofiler-sdk): kernel replay callback api #8622

Closed

1 task

amd-vkale mentioned this pull request Jul 31, 2026

refactor(rocprofiler-sdk): address review feedback on #8891 for remove-callbacks-counters #9586

Merged

1 of 3 tasks

This was referenced Aug 10, 2026

Remove Callbacks from rocProfiler-SDK #8586

Closed

refactor(rocprofiler-sdk): migrate counter collection off the per-queue callback registry #8891

Closed

amd-vkale requested a review from a team as a code owner September 21, 2026 20:00

amd-vkale mentioned this pull request Sep 21, 2026

refactor(rocprofiler-sdk): migrate thread trace off the per-queue callback registry #11964

Closed

1 task done

github-actions Bot added the github actions Pull requests that update GitHub Actions code label Sep 21, 2026

amd-vkale changed the base branch from develop to users/vkale/remove-callbacks-spm--with-cursor-fixes September 21, 2026 20:04

amd-vkale requested review from SrirakshaNag and bwelton September 21, 2026 20:11

amd-vkale changed the base branch from users/vkale/remove-callbacks-spm--with-cursor-fixes to develop September 21, 2026 20:30

amd-vkale marked this pull request as draft September 21, 2026 20:33

amd-vkale mentioned this pull request Sep 21, 2026

refactor(rocprofiler-sdk): migrate thread trace off the per-queue callback registry (step 1) #11973

Closed

amd-vkale changed the base branch from develop to users/vkale/remove-callbacks-spm--with-cursor-fixes September 21, 2026 20:33

amd-vkale marked this pull request as ready for review September 21, 2026 20:39

SrirakshaNag requested changes Sep 21, 2026

View reviewed changes

fix(rocprofiler-sdk): address SPM review comments on agent validation…

4b3ad15

…, test skips, and write_hook coverage Co-authored-by: Cursor <cursoragent@cursor.com>

amd-vkale force-pushed the users/vkale/remove-callbacks-spm branch from dcd8b01 to 4b3ad15 Compare September 21, 2026 23:08

amd-vkale merged commit 4b3ad15 into ROCm:users/vkale/remove-callbacks-spm--with-cursor-fixes Sep 21, 2026
1 check passed

Conversation

amd-vkale commented Jul 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Motivation

Underlying Problem

Notes on Bigger Picture

Technical Details

Hooks and call sites

Reviewer Guide

Issue Tracking

Test Plan

Test Result

Submission Checklist

Uh oh!

therock-pr-bot Bot commented Jul 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✅ All Policy Checks Passed

Uh oh!

therock-pr-bot Bot commented Jul 20, 2026

Uh oh!

bwelton left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cursor Bot Sep 21, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cursor Bot Sep 21, 2026

Choose a reason for hiding this comment

Uh oh!

amd-vkale commented Sep 19, 2026

Uh oh!

amd-vkale commented Sep 21, 2026

Uh oh!

amd-vkale commented Sep 21, 2026

Uh oh!

amd-vkale commented Sep 21, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

amd-vkale commented Sep 21, 2026

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

amd-vkale commented Jul 20, 2026 •

edited

Loading

therock-pr-bot Bot commented Jul 20, 2026 •

edited

Loading