Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Render App systems are blocking the Main app's primary thread #9964

Closed
james7132 opened this issue Sep 29, 2023 · 5 comments · Fixed by #11660 or #11672
Closed

Render App systems are blocking the Main app's primary thread #9964

james7132 opened this issue Sep 29, 2023 · 5 comments · Fixed by #11660 or #11672
Labels
A-ECS Entities, components, systems, and events A-Tasks Tools for parallel and async work C-Bug An unexpected or incorrect behavior C-Performance A change motivated by improving speed, memory usage or compile times D-Trivial Nice and easy! A great choice to get started with Bevy

Comments

@james7132
Copy link
Member

Bevy version

main (f8fd93f)

What you did

I ran cargo run --profile stress-test --features trace_tracy --example many_cubes and viewed the trace in Tracy.

What went wrong

I was expecting that no blocking systems from the render app would block the main app from progressing.

image

In this screenshot, prepare_windows is blocking UpdateAssets from terminating. Delaying it from terminating and blocking PreUpdate and subsequent schedules.

We currently do not schedule tasks from the ComputeTaskPool onto the render app thread, it may be a good idea to avoid doing so with all dedicated app schedule threads.

@james7132 james7132 added A-ECS Entities, components, systems, and events C-Performance A change motivated by improving speed, memory usage or compile times A-Tasks Tools for parallel and async work labels Sep 29, 2023
@alice-i-cecile alice-i-cecile added the C-Bug An unexpected or incorrect behavior label Sep 29, 2023
@hymm
Copy link
Contributor

hymm commented Sep 29, 2023

This is the non send data issue. Prepare Windows has to run on main thread because of that. Will be fixed by #9122.

@hymm
Copy link
Contributor

hymm commented Oct 4, 2023

I think prepare_windows might only need to run on main thread for ios. Someone should survey the winit api's, we're using and if they're all ok, add a conditional compiling to NonSendMarker.

i.e. a lot of the docs here https://docs.rs/winit/latest/winit/window/struct.Window.html say that these api's can only be called from main thread on ios.

@hymm hymm added the D-Trivial Nice and easy! A great choice to get started with Bevy label Oct 4, 2023
@daxpedda
Copy link
Contributor

daxpedda commented Oct 5, 2023

i.e. a lot of the docs here https://docs.rs/winit/latest/winit/window/struct.Window.html say that these api's can only be called from main thread on ios.

This has been addressed in rust-windowing/winit#2985 and rust-windowing/winit#3045.

Though keep mind that functions that return values might block the calling thread until these values are returned.

@jmintb
Copy link

jmintb commented Oct 9, 2023

Can I have a go at this issue? :)

@alice-i-cecile
Copy link
Member

@jmintb absolutely; let us know what you find out!

github-merge-queue bot pushed a commit that referenced this issue Feb 2, 2024
# Objective

- Allow prepare windows to run off of the main thread on platforms that
allow it.
- Fixes #9964 on most
platforms.

## Solution

- Conditionally compile prepare windows for different OS's
- Seems like it's only the call to `create_surface` that needs to run on
the main thread here.
- I've only tested this on windows, but I do see prepare windows running
on other threads.

---

## Changelog

- Allow prepare windows to run off main thread on platforms that allow
it.
github-merge-queue bot pushed a commit that referenced this issue Feb 3, 2024
# Objective

- Allow prepare windows to run off of the main thread on all platforms.
- Fixes #9964 on all platforms.

## Solution

- Running `prepare_windows` on the main thread on apple platforms is
only mandatory to create surface, which is only needed during window
creation. Split that part into its own system that happens before
`prepare_windows`
- Tested on macOS and iOS

---

## Changelog

- Allow prepare windows to run off main thread on all platforms.
tjamaan pushed a commit to tjamaan/bevy that referenced this issue Feb 6, 2024
# Objective

- Allow prepare windows to run off of the main thread on platforms that
allow it.
- Fixes bevyengine#9964 on most
platforms.

## Solution

- Conditionally compile prepare windows for different OS's
- Seems like it's only the call to `create_surface` that needs to run on
the main thread here.
- I've only tested this on windows, but I do see prepare windows running
on other threads.

---

## Changelog

- Allow prepare windows to run off main thread on platforms that allow
it.
tjamaan pushed a commit to tjamaan/bevy that referenced this issue Feb 6, 2024
…gine#11672)

# Objective

- Allow prepare windows to run off of the main thread on all platforms.
- Fixes bevyengine#9964 on all platforms.

## Solution

- Running `prepare_windows` on the main thread on apple platforms is
only mandatory to create surface, which is only needed during window
creation. Split that part into its own system that happens before
`prepare_windows`
- Tested on macOS and iOS

---

## Changelog

- Allow prepare windows to run off main thread on all platforms.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-ECS Entities, components, systems, and events A-Tasks Tools for parallel and async work C-Bug An unexpected or incorrect behavior C-Performance A change motivated by improving speed, memory usage or compile times D-Trivial Nice and easy! A great choice to get started with Bevy
Projects
None yet
5 participants