anugodavar · Jul 20, 2026 · Jul 10, 2026 · Jul 10, 2026 · Jul 10, 2026 · Jul 15, 2026
@@ -2440,13 +2440,9 @@ amd::Command* GraphExecSegmented::EnqueueSegmentedGraph(hip::Stream* launch_stre
 
   // Single AccumulateCommand on launch_stream manages all HW event lifetimes
   // and serves as the dispatch anchor for all segments across all streams.
-  // Pass `this` as the kernel-names owner: the command borrows kernel-name
-  // strings owned by this graph's nodes (via setKernelNamesRef during dispatch)
-  // and reads them in ReportActivity() at completion, after OnLaunchComplete()
-  // drops the launch's reference. Tying the GraphExecBase's lifetime to the command
-  // keeps those strings valid through the report (no copies). We already hold a
-  // launch reference here, so the retain in the constructor needs no trim lock.
-  auto* graph_accumulate = new amd::AccumulateCommand(*launch_stream, {}, nullptr, this);
+  // Kernel names are copied into the command at dispatch time (via addKernelName
+  // in dispatchAqlPacketBatchFlat) — no string borrowing, no GraphExecBase pin.
+  auto* graph_accumulate = new amd::AccumulateCommand(*launch_stream, {}, nullptr);
 
   // Register HW events with graph_accumulate so profiling can read them.
   for (auto& hw_event : segment_hw_events) {
@@ -2564,13 +2560,11 @@ hipError_t GraphExecSegmented::EnqueueSegment(const Segment& segment, hip::Strea
       return hipSuccess;
     }
 
-    const std::vector<const std::string*>* kernelNamesToDispatch;
     const std::vector<uint8_t>* flatData;
     const std::vector<uint32_t>* flatHdrs;
     const std::vector<uint8_t>* metaData = nullptr;
 
     if (packetBatch.disabledNodeCount == 0) {
-      kernelNamesToDispatch = &packetBatch.dispatchKernelNames;
       flatData = &packetBatch.flatPacketData;
       flatHdrs = &packetBatch.validPacketFullHeaders;
       if (!packetBatch.flatMetadataData.empty()) {
@@ -2580,7 +2574,6 @@ hipError_t GraphExecSegmented::EnqueueSegment(const Segment& segment, hip::Strea
       // Guard against stale filtered buffers: rebuildFlatBuffer (called from
       // UpdateAQLPacket) invalidates the cache. This is a no-op when valid.
       packetBatch.rebuildFilteredLists(sync_plan_.patch_list);
-      kernelNamesToDispatch = &packetBatch.enabledKernelNames;
       flatData = &packetBatch.filteredFlatPacketData;
       flatHdrs = &packetBatch.filteredValidPacketFullHeaders;
       if (!packetBatch.filteredFlatMetadataData.empty()) {
@@ -2590,8 +2583,7 @@ hipError_t GraphExecSegmented::EnqueueSegment(const Segment& segment, hip::Strea
 
     if (!flatData->empty()) {
       bool batchStatus = stream->vdev()->dispatchAqlPacketBatchFlat(
-          *flatData, *flatHdrs, accumulate, attach_signal, kernelNamesToDispatch, true,
-          false, metaData);
+          *flatData, *flatHdrs, accumulate, attach_signal, true, false, metaData);
       if (!batchStatus) {
         return hipErrorUnknown;
       }

@@ -1379,7 +1379,6 @@ class VirtualDevice : public amd::ReferenceCountedObject {
                                           const std::vector<uint32_t>& validFullHeaders,
                                           amd::AccumulateCommand* vcmd = nullptr,
                                           bool attach_signal = false,
-                                          const std::vector<const std::string*>* kernelNames = nullptr,
                                           bool pre_patched = false,
                                           bool blocking = false,
                                           const std::vector<uint8_t>* flatMetadataData = nullptr) {

@@ -491,8 +491,7 @@ void Timestamp::ExtractSignalTiming(ProfilingSignal* signal,
   if ((command().type() == CL_COMMAND_TASK) && (signal->flags_.isPacketDispatch_ == true)) {
     static_cast<amd::AccumulateCommand&>(command()).addTimestamps(
         static_cast<uint64_t>(sig_start * ticksToTime_),
-        static_cast<uint64_t>(sig_end * ticksToTime_),
-        signal->queue_index_);
+        static_cast<uint64_t>(sig_end * ticksToTime_));
   }
 
   signal->flags_.done_ = true;
@@ -1560,9 +1559,12 @@ bool VirtualGPU::dispatchAqlPacket(hsa_barrier_and_packet_t* packet, uint16_t he
 bool VirtualGPU::dispatchAqlPacketBatchFlat(const std::vector<uint8_t>& flatPacketData,
                                             const std::vector<uint32_t>& validFullHeaders,
                                             amd::AccumulateCommand* vcmd, bool attach_signal,
-                                            const std::vector<const std::string*>* kernelNames,
                                             bool pre_patched, bool blocking,
                                             const std::vector<uint8_t>* flatMetadataData) {
+  // Both base and ext kernel dispatch packets place kernel_object at the same offset.
+  static_assert(offsetof(hsa_kernel_dispatch_packet_t, kernel_object) ==
+                    offsetof(hsa_amd_ext_kernel_dispatch_packet_t, kernel_object),
+                "kernel_object offset mismatch between base and ext dispatch packets");
   if (vcmd == nullptr || flatPacketData.empty() || validFullHeaders.empty()) {
     return false;
   }
@@ -1571,15 +1573,10 @@ bool VirtualGPU::dispatchAqlPacketBatchFlat(const std::vector<uint8_t>& flatPack
   if (flatPacketData.size() != numPackets * 64) {
     return false;
   }
-
   std::scoped_lock lock(execution());
   profilingBegin(*vcmd);
   dispatchBlockingWait(nullptr);
 
-  if (kernelNames != nullptr && vcmd->profilingInfo().enabled_) {
-    vcmd->addKernelNames(*kernelNames);
-  }
-
   const uint32_t queueSize = gpu_queue_->size;
   const uint32_t queueMask = queueSize - 1;
   const uint32_t sw_queue_size = queueMask;
@@ -1696,8 +1693,11 @@ bool VirtualGPU::dispatchAqlPacketBatchFlat(const std::vector<uint8_t>& flatPack
       lastSlotPtr->completion_signal = Barriers().ActiveSignal();
     }
 
-    // Per-packet fixups: profiling signals, kernel-name printing, and inline barrier logging.
-    if (timestamp_ != nullptr || IsLogEnabled(amd::LOG_DETAIL_DEBUG, amd::LOG_KERN2) ||
+    // Per-packet fixups: profiling signals, kernel-name collection, kernel-name
+    // printing, and inline barrier logging.
+    const bool needKernelNames = amd::activity_prof::IsEnabled(OP_ID_DISPATCH);
+    if (timestamp_ != nullptr || needKernelNames ||
+        IsLogEnabled(amd::LOG_DETAIL_DEBUG, amd::LOG_KERN2) ||
         IsLogEnabled(amd::LOG_DETAIL_DEBUG, amd::LOG_AQL)) {
       for (size_t i = chunkStart; i < chunkEnd; ++i) {
         const uint64_t slotIdx = (startIndex + i) & queueMask;
@@ -1736,12 +1736,18 @@ bool VirtualGPU::dispatchAqlPacketBatchFlat(const std::vector<uint8_t>& flatPack
             slot->reserved2 = timestamp_->command().profilingInfo().correlation_id_;
           }
         }
-        if ((IsLogEnabled(amd::LOG_DETAIL_DEBUG, amd::LOG_KERN2) ||
-             IsLogEnabled(amd::LOG_DETAIL_DEBUG, amd::LOG_AQL)) &&
-            kernelNames != nullptr && i < kernelNames->size() && isKernelDispatch) {
+        if (isKernelDispatch && (needKernelNames ||
+            IsLogEnabled(amd::LOG_DETAIL_DEBUG, amd::LOG_KERN2) ||
+            IsLogEnabled(amd::LOG_DETAIL_DEBUG, amd::LOG_AQL))) {
+          auto kit = dev().KernelMap().find(slot->kernel_object);
+          const char* kname = kit != dev().KernelMap().end()
+                                  ? kit->second.getDemangledName().c_str()
+                                  : "<unknown>";
+          if (needKernelNames) {
+            vcmd->addKernelName(kname);
+          }
           ClPrint(amd::LOG_DETAIL_DEBUG, amd::LOG_KERN2, "Graph ShaderName : %s, device id : %u",
-                  (*kernelNames)[i] != nullptr ? (*kernelNames)[i]->c_str() : "<null>",
-                  dev().index());
+                  kname, dev().index());
           if (isBaseKernelDispatch) {
             logAqlDispatchPacket(roc_device_, gpu_queue_, hdr, slot, slotIdx, priority_);
           } else {

@@ -683,7 +683,6 @@ class VirtualGPU : public device::VirtualDevice {
                                   const std::vector<uint32_t>& validFullHeaders,
                                   amd::AccumulateCommand* vcmd = nullptr,
                                   bool attach_signal = false,
-                                  const std::vector<const std::string*>* kernelNames = nullptr,
                                   bool pre_patched = false,
                                   bool blocking = false,
                                   const std::vector<uint8_t>* flatMetadataData = nullptr) override;

@@ -138,24 +138,13 @@ void ReportActivity(const amd::Command& command) {
     auto timestamps = static_cast<const amd::AccumulateCommand&>(command).getTimestamps();
     const auto& kernel_names =
         static_cast<const amd::AccumulateCommand&>(command).getKernelNames();
-    // timestamps has one entry per HSA_PACKET_TYPE_KERNEL_DISPATCH packet only.
-    // kernel_names has one entry per AQL packet slot (nullptr for barriers and SDMA/copy nodes
-    // that don't generate timestamps). Walk kernel_names; for each non-null entry consume
-    // the next timestamp.
-    // Queue id to fall back on when a timestamp did not record its stream.
-    const uint64_t fallback_queue_id = queue->vdev()->index();
-    uint32_t ti = 0;
-    for (uint32_t ki = 0; ki < kernel_names.size() && ti < timestamps.size(); ki++) {
-      if (kernel_names[ki] == nullptr) continue;
-      const auto& it = timestamps[ti++];
-      record.begin_ns = it.start;
-      record.end_ns = it.end;
-      // Graph kernels can run on different streams under one command; attribute
-      // each to the queue it actually ran on when known.
-      record.queue_id = (it.queue_index != amd::AccumulateCommand::kInvalidQueueIndex)
-          ? static_cast<uint64_t>(it.queue_index)
-          : fallback_queue_id;
-      record.kernel_name = kernel_names[ki]->c_str();
+    // kernel_names and timestamps are both populated only when profiling is active
+    // at dispatch time. Walk the shorter of the two as a safety bound.
+    for (uint32_t i = 0; i < kernel_names.size() && i < timestamps.size(); i++) {
+      auto it = timestamps[i];
+      record.begin_ns = it.first;
+      record.end_ns = it.second;
+      record.kernel_name = kernel_names[i];
       function(ACTIVITY_DOMAIN_HIP_OPS, operation_id, &record);
     }
   } else {

@@ -63,14 +63,6 @@ Event::~Event() {
 
 // ================================================================================================
 AccumulateCommand::~AccumulateCommand() {
-  // Drop the kernel-names owner pin taken in the constructor (e.g. the
-  // GraphExec). Done first so it always runs, regardless of the owns_hw_events_
-  // early return. Releasing here -- after ReportActivity() has read the names --
-  // is what makes the borrowed kernel-name strings safe without copying them.
-  if (kernelNamesOwner_ != nullptr) {
-    kernelNamesOwner_->release();
-    kernelNamesOwner_ = nullptr;
-  }
   // When an external owner (e.g. the graph signal pool) reclaims the signals,
   // do not destroy them here.
   if (!owns_hw_events_) {

@@ -1673,54 +1673,25 @@ class Marker : public Command {
 
 class AccumulateCommand : public Command {
  private:
-  //! Kernel names and timestamps list for activity profiling
-  std::vector<const std::string*> kernelNames_;
-  const std::vector<const std::string*>* kernelNamesRef_ = nullptr;
-  //! Optional owner of the borrowed kernel-name strings (e.g. the GraphExec
-  //! whose nodes own them). Retained while this command lives so the strings
-  //! outlive ReportActivity(), which runs at the end of setStatus(CL_COMPLETE)
-  //! -- after OnLaunchComplete() may have dropped the launch's reference. This
-  //! ties the strings' lifetime to the consumer (this command) rather than to
-  //! the graph launch, with no string copies. Set via the constructor.
-  ReferenceCountedObject* kernelNamesOwner_ = nullptr;
-
- public:
-  //! Sentinel queue index meaning "unknown; fall back to the command's queue".
-  static constexpr uint32_t kInvalidQueueIndex = 0xFFFFFFFFu;
-
-  //! One kernel dispatch's GPU timing plus the queue (vGPU index) it ran on.
-  //! Graphs span multiple streams under one command, so the queue is tracked
-  //! per timestamp rather than taken from the command's queue.
-  struct KernelTimestamp {
-    uint64_t start;
-    uint64_t end;
-    uint32_t queue_index;
-  };
-
- private:
-  std::vector<KernelTimestamp> tsList_;
+  //! Stable kernel name pointers — one entry per kernel dispatch slot (base and
+  //! ext variants). Resolved from KernelMap at dispatch time; point into Kernel
+  //! objects that live for the device lifetime. Non-dispatch slots (barriers,
+  //! SDMA) are skipped so kernelNames_ and timestamps_ are always parallel.
+  //! "<unknown>" is used when the kernel_object is not found in KernelMap.
+  std::vector<const char*> kernelNames_;
+  //! GPU timestamps — one entry per kernel dispatch slot, parallel to
+  //! kernelNames_, populated at signal completion time.
+  std::vector<std::pair<uint64_t, uint64_t>> timestamps_;
   //! HW events that need to be released when this command is destroyed
   std::unordered_map<Device*, std::vector<void*>> hw_events_;
   //! When false, the destructor does not destroy hw_events_ (an external owner,
   //! e.g. the graph signal pool, reclaims them instead).
   bool owns_hw_events_ = true;
 
  public:
-  //! Create a new accumulate command. kernelNamesOwner, when given, is the
-  //! object that owns the borrowed kernel-name strings (e.g. the GraphExec);
-  //! it is retained for the command's whole lifetime and released in the
-  //! destructor, so the borrowed strings stay valid through ReportActivity()
-  //! even after OnLaunchComplete() drops the launch's reference -- with no
-  //! string copies.
   AccumulateCommand(HostQueue& queue, const EventWaitList& eventWaitList = nullWaitList,
-                    const Event* waitingEvent = nullptr,
-                    ReferenceCountedObject* kernelNamesOwner = nullptr)
-      : Command(queue, CL_COMMAND_TASK, eventWaitList, 0, waitingEvent),
-        kernelNamesOwner_(kernelNamesOwner) {
-    if (kernelNamesOwner_ != nullptr) {
-      kernelNamesOwner_->retain();
-    }
-  }
+                    const Event* waitingEvent = nullptr)
+      : Command(queue, CL_COMMAND_TASK, eventWaitList, 0, waitingEvent) {}
 
   //! Destructor - release all retained HW events
   virtual ~AccumulateCommand();
@@ -1748,35 +1719,20 @@ class AccumulateCommand : public Command {
   //! them across launches instead.
   void setOwnsHwEvents(bool owns) { owns_hw_events_ = owns; }
 
-  //! Add kernel name to the list if available
-  void addKernelName(const std::string* kernelName) { kernelNames_.push_back(kernelName); }
-
-  //! Add multiple kernel names in bulk
-  void addKernelNames(const std::vector<const std::string*>& kernelNames) {
-    kernelNames_.insert(kernelNames_.end(), kernelNames.begin(), kernelNames.end());
-  }
-
-  //! Set kernel names by reference (cheap; borrows the caller's vector and
-  //! strings). Safe only while that storage outlives this command. Used on the
-  //! hot path when no profiler is active, where the names are never read.
-  void setKernelNamesRef(const std::vector<const std::string*>* kernelNames) {
-    kernelNamesRef_ = kernelNames;
-  }
+  //! Record a stable kernel name pointer for one kernel dispatch slot.
+  //! Must not be nullptr — use "<unknown>" when the name cannot be resolved.
+  void addKernelName(const char* name) { kernelNames_.push_back(name); }
 
-  //! Add kernel timestamp to the list if available. queue_index is the vGPU
-  //! index of the stream the kernel ran on (kInvalidQueueIndex when unknown).
-  void addTimestamps(uint64_t startTs, uint64_t endTs,
-                     uint32_t queue_index = kInvalidQueueIndex) {
-    tsList_.push_back(KernelTimestamp{startTs, endTs, queue_index});
+  //! Add GPU timestamps for one dispatch slot (called at signal completion).
+  void addTimestamps(uint64_t startTs, uint64_t endTs) {
+    timestamps_.push_back({startTs, endTs});
   }
 
-  //! Return the kernel names (pointers to stable strings, no copies)
-  const std::vector<const std::string*>& getKernelNames() const {
-    return kernelNamesRef_ != nullptr ? *kernelNamesRef_ : kernelNames_;
-  }
+  //! Return kernel name pointers (one per dispatch slot)
+  const std::vector<const char*>& getKernelNames() const { return kernelNames_; }
 
-  //! Return the kernel timestamps
-  const std::vector<KernelTimestamp>& getTimestamps() const { return tsList_; }
+  //! Return GPU timestamps (one per dispatch slot, populated at signal completion)
+  const std::vector<std::pair<uint64_t, uint64_t>>& getTimestamps() const { return timestamps_; }
 
   //! The command implementation
   virtual void submit(device::VirtualDevice& device) { device.submitAccumulate(*this); }

@@ -270,22 +270,18 @@ class HostQueue : public CommandQueue {
     // an invalid access
     command->retain();
 
-    // Extra retain for the last command
-    command->retain();
-
-    // Update lastEnqueueCommand_ before releasing the old one to prevent
-    // use-after-free if the release triggers re-entrant access to this queue
-    // (e.g. ~GraphExec calling finish() on graph-internal streams during
-    // cascading destruction from the old command's release).
-    Command* prevLastEnqueue = lastEnqueueCommand_;
-    lastEnqueueCommand_ = command;
-
-    if (prevLastEnqueue != nullptr) {
-      prevLastEnqueue->release();
+    // Release the last command in the batch
+    if (lastEnqueueCommand_ != nullptr) {
+      lastEnqueueCommand_->release();
     } else {
       // The queue becomes active. Add it to the set of activeQueues.
       device_.addToActiveQueues(this);
     }
+
+    // Extra retain for the last command
+    command->retain();
+
+    lastEnqueueCommand_ = command;
   }
 
   //! Flushes submitted commands if the batch size significantly grew