tracy icon indicating copy to clipboard operation
tracy copied to clipboard

Merge ZoneEvent and GpuEvent (redo)

Open erieaton-amd opened this issue 2 months ago • 3 comments

This PR accomplishes part of what #1155 does with fewer changes.

The work is split across three changes:

  • Replace GpuEvent with ZoneEvent: This is a smaller revision that accomplishes the removal of GpuEvent, while changing as little else as possible. It is hard to demonstrate the virtue of doing this without further changes; this patch does unify a few small functions.
  • Eliminate the function GetZoneCtx: This function turns out to be unnecessary, this patch introduces a helper struct to carry around the context which is also used in the next patch.
  • Merge the GetZoneColor functions: The GPU and CPU code is made to use the same code to determine zone color. The helper struct from the previous patch is used by the unified functions to distinguish CPU and GPU events. This allows GPU events to have a custom color (although the protocol doesn't allow it to be set yet).

Overall notes:

  • The file format is not changed.
  • The protocol is not changed.
  • The UI is changed because GPU zones now appear as a different color.
  • The original task of ensuring feature parity between GPU and CPU remains unfinished.
  • Compared to the previous PR, the ZoneExtra data is expanded only for GPU, saving some memory. Also, the previous PR eliminated the thread field but this doesn't. That uses more memory but avoids having to go search for the thread.

erieaton-amd avatar Oct 08 '25 00:10 erieaton-amd

How are GPU zone colors determined? With CPU zones thread id is used as a base for the color. What differentiates different GPU tracks?

wolfpld avatar Oct 08 '25 10:10 wolfpld

How are GPU zone colors determined? With CPU zones thread id is used as a base for the color. What differentiates different GPU tracks?

I realized there was an issue that was making the GPU zones always purple. I've posted a small change that makes the GPU zones' color match the CPU thread that is making the GPU calls.

erieaton-amd avatar Oct 08 '25 20:10 erieaton-amd

The whole struct EventAdapter is very hard to read for me due to templates, and I don't paricularly like that instead of passing a single object (ev), it is now expected to construct a wrapper ({ev, extra}).

Given that you have made GpuExtra inherit from ZoneExtra, the extra data is now directly linked to from each GPU ZoneEvent (that's what makes sense for me, not something I have read from the proposed changes).

If the above is true, there is no need to construct the wrapper/adapter struct, as the GPU data can be accessed via the extra pointer, and I think it should be possible for the code to know which ZoneEvent is GPU and has the GpuExtra type extra data from the context of the call.

Does that make sense?

wolfpld avatar Nov 23 '25 15:11 wolfpld