Skip to content

Conversation

@Lectem
Copy link
Collaborator

@Lectem Lectem commented Apr 8, 2025

This PR does the following things:

  • Allow to lock the highlighted thread using left mouse click or lane popup menu
  • Add more descriptions for wait reasons in View::DecodeContextSwitchReason
  • Stores the Wakeup CPU so that we may display it in the CPU data view
  • QueueThreadWakeup now contains cpu and adjust info (currently unused). Fits in the padding, no struct size change.
  • QueueContextSwitch now also contains thread priorities information (windows+linux) and CState (windows), but currently unused by the profiler. Fits in the padding, no struct size change.
  • Fixes Linux EventWakeup which tries to remove PERF_SAMPLE_CALLCHAIN on the wrong member. Set the whole perf_event_attr explicitely instead.
  • Fixes Windows ReadyThread event being dropped if triggered before the thread switches out (this is confirmed to be normal behaviour, probably due to thread being switched out but context switch actually happening later)
  • Bumps tracy file version and protocol versions

Screenshots of the wakeup viz:

image
image

In the future we should be able to retrieve the readying thread information, such as its callstack, by iterating the per-core context switch data. Ideally we would also have a way to draw an arrow between the thread lanes and not just the CPU cores data ?

This PR does the following things:
- Allow to lock the highlighted thread using left mouse click or lane popup menu
- Add more descriptions for wait reasons in View::DecodeContextSwitchReason
- Stores the Wakeup CPU so that we may display it in the CPU data view
- `QueueThreadWakeup` now contains cpu and adjust info (currently unused). Fits in the padding, no struct size change.
- `QueueContextSwitch` now also contains thread priorities information (windows+linux) and CState (windows), but currently unused by the profiler. Fits in the padding, no struct size change.
- Fixes Linux `EventWakeup` which tries to remove PERF_SAMPLE_CALLCHAIN on the wrong member. Set the whole `perf_event_attr` explicitely instead.
- Fixes Windows ReadyThread event being dropped if triggered before the thread switches out (this is confirmed to be normal behaviour, probably due to thread being switched out but context switch actually happening later)
- Bumps tracy file version and protocol versions
@wolfpld
Copy link
Owner

wolfpld commented Apr 8, 2025

Please update the manual to explain the changes you've made. As it now is I don't really know what the screenshots show. Having a proper writeup will help with code review.

@Lectem
Copy link
Collaborator Author

Lectem commented Apr 8, 2025

I added a new paragraph and illustration to the manual (pages 78-79 for me).
Please tell me what you think about it.

@Lectem
Copy link
Collaborator Author

Lectem commented Apr 8, 2025

Fixed the latest comments (I think), and also added

Clicking again somewhere empty on the timeline with the left mouse button will unlock the selection.

@wolfpld
Copy link
Owner

wolfpld commented Apr 9, 2025

Lines on the CPU data graph no longer work as intended.

Correct behavior:

obraz

After moving the leftmost element out of view:

obraz

The wakeup markers have the same issue.

@wolfpld
Copy link
Owner

wolfpld commented Apr 9, 2025

The vertical lines are drawn on top of the above and below CPU lanes:

obraz

@wolfpld
Copy link
Owner

wolfpld commented Apr 9, 2025

Elements too small to be visible are not culled, which results in an unreadable display and severely degraded performance.

obraz

@wolfpld
Copy link
Owner

wolfpld commented Apr 9, 2025

Selecting a thread on the CPU data list hijacks the highlight functionality. Threads hovered over with the mouse will not be highlighted until the selection is removed. These should be two separate things.

@wolfpld
Copy link
Owner

wolfpld commented Apr 9, 2025

I want to see how the two wakeup colors differ, but there is no explanation in the tooltip.

obraz

@wolfpld
Copy link
Owner

wolfpld commented Apr 9, 2025

The marker has incorrect offset.

image

@wolfpld
Copy link
Owner

wolfpld commented Apr 9, 2025

The wakeup line does not adapt to DPI.

image

@wolfpld
Copy link
Owner

wolfpld commented Apr 9, 2025

Is the data gathered correctly on Linux?

I have worker threads that are sleeping on a condition variable, and the job dispatching thread is waking the workers. But all I see is that the worker threads are "woken by kernel".

image

What I see instead are some strange wakeup attributions that don't make much sense.

image

Worker#16 is seemingly woken by Worker#12, but the worker threads do not communicate with each other. Workers may block each other on access to the job queue (as seen on the lock at the bottom of the image), but when Worker#16 obtains the lock, the wakeup reason is still "woken by kernel".

@Lectem
Copy link
Collaborator Author

Lectem commented Apr 10, 2025

It seems that there are subtle differences between sched_wakeup and sched_waking (there's also sched_wakeup_new) and that the origin CPU might not be the one of the wakee depending on various things ...

See https://perfetto.dev/docs/data-sources/cpu-scheduling

I'll investigate a bit more.

Note that on windows, it is possible that a thread seemingly unrelated readies (wakes up) another if it triggers the scheduler by calling any kind of synchronization or syscall. (this mostly happens when thread was already ready to be scheduled). From what I saw on windows, we're now displaying the same things as WPA and Pix.

@Lectem
Copy link
Collaborator Author

Lectem commented Apr 10, 2025

Unselecting only works if CPU data is drawn. It's enough to collapse the CPU data section to not be able to unselect the thread.

I suppose I could make it unslect as long as you click anything else than a thread in the cpu timeline then? But then you would un select the thread when clicking zones, I'm not sure this is the behavior we want. And I don't think we have an easy way to know we clicked on an empty space as of now?

I'll add an "unselect" in the lane pop-up menu of the currently selected thread in the meantime

@wolfpld
Copy link
Owner

wolfpld commented Apr 10, 2025

I suppose I could make it unslect as long as you click anything else than a thread in the cpu timeline then? But then you would un select the thread when clicking zones, I'm not sure this is the behavior we want. And I don't think we have an easy way to know we clicked on an empty space as of now?

I'll add an "unselect" in the lane pop-up menu of the currently selected thread in the meantime

The behavior should be consistent, and currently it isn't.

With https://github.com/wolfpld/tracy/pull/1021/files#r2036164986 it is possible to select a thread, but there's no way to unselect it.

Lectem added 5 commits April 13, 2025 19:59
`sched_wakeup` notifies about the kernel saying it will be waking up the thread, not the reason for the actual wakeup. This only allows to measure the scheduling latency, not the full wake up latency.
Instead, use `sched_waking` which is triggered on the ring of the CPU and thread responsible for the wakeup. `target_cpu` contains the cpu on which the thread will be woken up.
…the CPU data timeline, and can unselect from the thread timeline menu
@Lectem
Copy link
Collaborator Author

Lectem commented Apr 13, 2025

The marker has incorrect offset.
image

On my PC it I had a different issue, the marker was not covering the bottom line, while on your screenshot it's the top line. Could it be something related to inverted Y-axis on some renderers ? Anyway, I added +1 to the end of the line Y since sstep = sty + 1, could you confirm this works on your end ?

@Lectem
Copy link
Collaborator Author

Lectem commented Apr 13, 2025

It seems that there are subtle differences between sched_wakeup and sched_waking (there's also sched_wakeup_new) and that the origin CPU might not be the one of the wakee depending on various things ...

See https://perfetto.dev/docs/data-sources/cpu-scheduling

I'll investigate a bit more.

Note that on windows, it is possible that a thread seemingly unrelated readies (wakes up) another if it triggers the scheduler by calling any kind of synchronization or syscall. (this mostly happens when thread was already ready to be scheduled). From what I saw on windows, we're now displaying the same things as WPA and Pix.

In the end it seems that sched_waking is the event we needed, not sched_wakeup.

sched_wakeup is only useful if you need to compute the scheduling latency of the context switch itself (ie: how long it takes for the thread to wakeup since the kernel actually decided to do it). Note that all sched_wakeup events are always followed by a sched_switch event, so it's not necessary to keep it as we do not compute the scheduling latency.

Instead, I replaced sched_wakeup by sched_waking, which does give the information about the thread actually triggering the wakeup. This way now have the same information as the ReadyThread event on Windows. The only impact this has on current behavior is that the "Thread is waking up" part on the thread timeline will now indicate the same thing on both platforms, while it used to show only the scheduling latency on Linux.
image

This was confirmed by analysing the ftrace output (for example here using traceshark (https://github.com/cunctator/traceshark).
image )

@Lectem
Copy link
Collaborator Author

Lectem commented Apr 24, 2025

The marker has incorrect offset.
image

On my PC it I had a different issue, the marker was not covering the bottom line, while on your screenshot it's the top line. Could it be something related to inverted Y-axis on some renderers ? Anyway, I added +1 to the end of the line Y since sstep = sty + 1, could you confirm this works on your end ?

So this was actually a half pixel issue.

I have a setup with the following screens:

  • 2560x1440(1x) (main) + 2560x1600(1.5x) + 1080p (1x)
    When using this setup, whatever screen the application opens up on first, I had the following result (same as you):
    image

  • If I remove the first screen, and thus use the following setup:
    2560x1600(1.5x) (main) + 1080p (1x)
    image

  • Then if I use only the 1080p (1x) screen, issue comes back.
    image.

Using a half pixel offset:

  • 2560x1440(1x) (main) + 2560x1600(1.5x) + 1080p (1x)
    image

  • 2560x1600(1.5x) (main) + 1080p (1x)
    image

  • 1080p only:
    image

This was also tested with different zoom levels and seems correct now.

@wolfpld
Copy link
Owner

wolfpld commented Apr 25, 2025

/home/wolf/tracy/profiler/src/profiler/TracyView_ContextSwitch.cpp:79:53: warning: expression result unused [-Wunused-value]
   79 |     case ContextSwitchData::Win32_WrPoolAllocation: "(Thread is waiting for a system allocation)";
      |                                                     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Lectem added 2 commits May 5, 2025 15:52
…s called

`HandleTimelineMouse` did the following
- `m_highlight.active = true` when clicked
- update range while dragging
- `m_highlight.active = false` when not dragging

This causes issues when `IsMouseClickReleased` is called somewhere else as
- `mousePotentialClickRelease` is set to true on click
- At beginning of frame, `mouseDragging` is set to `false` as long as `mousePotentialClickRelease` is `true` and mouse delta is under the drag threshold
- This means that if the mouse didn't move enough in the duration of a frame, highlight would immediately stop, even though we were still holding the mouse button down

Instead, it now only stops highlighting once the mouse is no longer down (ie: has been "released", cursor having moved or not)
@Lectem
Copy link
Collaborator Author

Lectem commented May 7, 2025

I think with this last fix there should not be any pending issues anymore (as far as I'm concerned).
I've been using this for a while now (both on Windows and Linux), and didn't encounter other issues (not saying there aren't, but I've not seen them if there are).

Do you need me to address anything else for this PR ?

@wolfpld
Copy link
Owner

wolfpld commented May 7, 2025

I'll try to take a look.

@Lectem
Copy link
Collaborator Author

Lectem commented May 7, 2025

Thanks!
No hurry though, I just wanted to make sure I didn't miss any of your comments

@wolfpld wolfpld merged commit 5140a5a into wolfpld:master May 10, 2025
6 checks passed
@Lectem Lectem deleted the wakeup branch May 10, 2025 12:22
@gglin001
Copy link
Contributor

gglin001 commented Jun 8, 2025

can we add support wakeup vis for tracy-import-chrome ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants