Fix application freezing on macOS when trace_tracy is enabled#23201
Fix application freezing on macOS when trace_tracy is enabled#23201alice-i-cecile merged 2 commits intobevyengine:mainfrom
trace_tracy is enabled#23201Conversation
|
Welcome, new contributor! Please make sure you've read our contributing guide and we look forward to reviewing your pull request shortly ✨ |
e7e8bfb to
e7fcc1d
Compare
|
Your PR caused a change in the graphical output of an example or rendering test. This might be intentional, but it could also mean that something broke! If it's expected, please add the M-Deliberate-Rendering-Change label. If this change seems unrelated to your PR, you can consider updating your PR to target the latest main branch, either by rebasing or merging main into it. |
|
@xremming Just confirming that you meant to point this PR to be merged into the Does this issue appear on main? If so, I think it’s better to point this PR to be merged into main, and then we can tag it appropriately for inclusion in the correct milestone |
|
Indeed: we should point this to |
|
@kfc35 I did indeed point this to 0.18.1 branch for a reason, which is that it's where I made the fix at for myself for (and tested it). I can rebase this to main if that's preferable. However 0.18.1 is not yet released at crates.io? |
Yes, this needs to be rebased :)
Correct; just waiting for final sign-off from fellow maintainers. |
e7fcc1d to
8a86699
Compare
|
Rarely does a rebase work on the first try as intended... Nice. Tested it out with and without the patch on main against the 3d_shapes example and can confirm the fix still works (and is still required). |
|
Would be interesting if someone with an older macOS version could test this. Seeing that it has worked on macOS previously, at least based on the profiling.md that has a screenshot from macOS. This also made me wonder if this will need to be gated on iOS too. 🤔 I remember seeing an issue about supporting Tracy on iOS and Android. Perhaps another PR will be required to rework the GPU profiling based on support for it. |
566aa73 to
e3839c5
Compare
|
If possible, could also be useful to also build and test the examples on macOS with Tracy enabled. That should make sure this doesn't happen again. |
kfc35
left a comment
There was a problem hiding this comment.
This looks correct and doesn’t seem unreasonable. I agree with the logic that we should just return none for unsupported backends
ChristopherBiscardi
left a comment
There was a problem hiding this comment.
I've tested this code on macos and it does prevent hangs when using tracy features. That said, I don't understand the fix, and the fix isn't explained in the PR, so I'm not sure anyone currently understands it. I will approve this on the condition that it fixes the crash and is easily revertible, with the following context:
We copy this code from Wumpf/wgpu-profiler, which I've run and also exhibits a similar issue on macos. I filed an issue so they are also aware, since we copy their code.
I can't find where or why the behavior for the equivalent of Poll::wait_indefinitely changed on macos (which is where the hang is occurring), and this PR currently fixes it by not creating the Tracy client, and thus not setting a section of tracy-related span information, which seems like a difference in behavior that we aren't checking.
I feel like there should be a PR somewhere or a code change (perhaps in wgpu?) that we can point to to understand what happened here, but I can't find it.
7aae6d1 to
fa82a1f
Compare
|
Amended the commit to include a comment explaining why the function is returning an optional (and removed the original short-circuiting fix commit so this is easy to revert). |
The unsupported backends used to be mapped to `GpuContextType::Invalid`, changed it to instead use `None` in those cases. This assumes that the `Invalid` type was actually not doing any work.
fa82a1f to
c6d326c
Compare
|
new comment suitably addresses my concerns about notifying future readers about the ambiguity of the source of the issue |
Objective
On macOS (26.3) enabling tracy for tracing causes Bevy to freeze here.
https://github.com/bevyengine/bevy/blob/release-0.18.1/crates/bevy_render/src/diagnostic/tracy_gpu.rs#L64
I assume this problem is likely related to the same root cause as #22365.
Solution
tracy_client::GpuContextat all instead of creating aGpuContextType::Invalid. Whether this is "correct" is up to debate but it does fix the issue.Testing
cargo run --example 3d_shapes -F bevy/trace_tracywithout the patch -> example doesn't start and Tracy only gets events until the line linked above.