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

Bevy 0.10 regression: very long startup if WinitSettings.unfocused_mode is set to UpdateMode::Reactive #7974

Open
JayceFayne opened this issue Mar 8, 2023 · 6 comments · May be fixed by #8953
Labels
A-Windowing Platform-agnostic interface layer to run your app in C-Bug An unexpected or incorrect behavior C-Regression Functionality that used to work but no longer does. Add a test for this! O-Linux Specific to the Linux desktop operating system

Comments

@JayceFayne
Copy link

JayceFayne commented Mar 8, 2023

Bevy version

The release number or commit hash of the version you're using.

0.10/0.11/0.12/0.13

Relevant system information

Arch Linux

What you did

Running https://github.com/JayceFayne/bevy_regression
Pasting main.rs below:

use bevy::prelude::*;
use bevy::winit::WinitSettings;

fn main() {
    App::new()
        .insert_resource(WinitSettings::desktop_app())
        .add_plugins(DefaultPlugins.build())
        .run();
}

What went wrong

Takes very long to start

Additional information

Downgrading to 0.9 fixes the issue

@JayceFayne JayceFayne added C-Bug An unexpected or incorrect behavior S-Needs-Triage This issue needs to be labelled labels Mar 8, 2023
@alice-i-cecile alice-i-cecile added A-ECS Entities, components, systems, and events C-Performance A change motivated by improving speed, memory usage or compile times and removed C-Bug An unexpected or incorrect behavior S-Needs-Triage This issue needs to be labelled labels Mar 8, 2023
@JayceFayne JayceFayne changed the title Bevy 0.10 regression: very long startup Bevy 0.10 regression: very long startup if unforcused_mode is set to UpdateMode::Reactive Mar 8, 2023
@JayceFayne JayceFayne changed the title Bevy 0.10 regression: very long startup if unforcused_mode is set to UpdateMode::Reactive Bevy 0.10 regression: very long startup if WinitSettings::unforcused_mode is set to UpdateMode::Reactive Mar 8, 2023
@alice-i-cecile alice-i-cecile added S-Needs-Investigation This issue requires detective work to figure out what's going wrong and removed A-ECS Entities, components, systems, and events labels Mar 8, 2023
@JayceFayne JayceFayne changed the title Bevy 0.10 regression: very long startup if WinitSettings::unforcused_mode is set to UpdateMode::Reactive Bevy 0.10 regression: very long startup if WinitSettings.unforcused_mode is set to UpdateMode::Reactive Mar 8, 2023
@alice-i-cecile
Copy link
Member

Could you give us a before / after tracing flamegraph? This might be due to the more involved system graph processing.

If that's the case, there are a number of things we can do to improve performance there, ranging from simple optimizations to caching the graph on disk.

See https://github.com/bevyengine/bevy/blob/main/docs/profiling.md for more info.

@alice-i-cecile
Copy link
Member

Alternatively, given your reproduction this could just be a bug with how we / winit is handling that specific mode.

@JayceFayne
Copy link
Author

JayceFayne commented Mar 8, 2023

Could you give us a before / after tracing flamegraph? This might be due to the more involved system graph processing.

If that's the case, there are a number of things we can do to improve performance there, ranging from simple optimizations to caching the graph on disk.

See https://github.com/bevyengine/bevy/blob/main/docs/profiling.md for more info.

Sure, I have traces for 0.9 and 0.10 in chrome tracing format, I'll attach them here. Since github does not allow me to upload json files I'll upload the rendered flamegraph.
0 9
0 10

My guess is that the 60 sec wait in 0.10 is because max_wait is set to 60 secs in unfocused mode for the desktop_app preset.

@JayceFayne JayceFayne changed the title Bevy 0.10 regression: very long startup if WinitSettings.unforcused_mode is set to UpdateMode::Reactive Bevy 0.10 regression: very long startup if WinitSettings.unfocused_mode is set to UpdateMode::Reactive Mar 8, 2023
@alice-i-cecile
Copy link
Member

FYI in the future you can bypass that limitation by using .zip files :) And agreed: that looks squarely like a windowing bug: let's see if we can't get this fixed for 0.10.1...

@alice-i-cecile alice-i-cecile added C-Bug An unexpected or incorrect behavior A-Windowing Platform-agnostic interface layer to run your app in and removed C-Performance A change motivated by improving speed, memory usage or compile times S-Needs-Investigation This issue requires detective work to figure out what's going wrong labels Mar 8, 2023
@alice-i-cecile alice-i-cecile added this to the 0.10.1 milestone Mar 8, 2023
@cart cart modified the milestones: 0.10.1, 0.11 Mar 31, 2023
@geieredgar
Copy link
Contributor

I've had a look at this and there are two parts that cause this behavior:

  1. Wayland initializes the window with an unfocused state and only displays and focuses the window after a frame is submitted.
  2. Pipelined rendering, which can cause app.update() to finish before the frame is submitted.

Tasks like running the prepare_windows system are spawned by a render thread to run on the main thread. If app.update() is already finished, those tasks have to wait until app.update() is called again.

With DefaultPlugins.build().disable::<PipelinedRenderingPlugin>() this issue does not appear, since the frame is submitted during the first app.update(). Winit sends a WindowEvent::Focused(false) event after creating the window, therefore the app is always updated at startup.

I think the most clean solution (from a task-stall-prevention perspective) would be to wake the main thread (by sending a user event through the winit event proxy) when a main-thread task is spawned by a render-thread, but this would require a lot of changes in a lot of unrelated places and probably hurt performance.

A more pragmatic solution could be to consider a Wayland window focused at creation (ignoring the WindowEvent::Focused(false) event unless we already received a WindowEvent::Focused(true) event for the same window). I can submit a PR for that if such a solution is acceptable.

@geieredgar geieredgar linked a pull request Jun 25, 2023 that will close this issue
@james7132 james7132 added O-Linux Specific to the Linux desktop operating system C-Regression Functionality that used to work but no longer does. Add a test for this! labels Jun 25, 2023
@B-head B-head mentioned this issue Jul 4, 2023
8 tasks
@cart cart removed this from the 0.11 milestone Jul 7, 2023
@cart cart added this to the 0.12 milestone Jul 7, 2023
@alice-i-cecile alice-i-cecile modified the milestones: 0.12, 0.13 Oct 26, 2023
@esensar
Copy link
Contributor

esensar commented Jan 24, 2024

Just checking in after seeing Mastodon post: https://fosstodon.org/@alice_i_cecile@mastodon.gamedev.place/111811705662087955

I can reproduce this issue on Wayland (Fedora 39, Sway WM) still, so I guess it is still a valid issue.

@alice-i-cecile alice-i-cecile removed this from the 0.13 milestone Feb 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Windowing Platform-agnostic interface layer to run your app in C-Bug An unexpected or incorrect behavior C-Regression Functionality that used to work but no longer does. Add a test for this! O-Linux Specific to the Linux desktop operating system
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants