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

Possible performance regression in WASM builds #11122

Closed
eerii opened this issue Dec 28, 2023 · 1 comment · Fixed by #11227
Closed

Possible performance regression in WASM builds #11122

eerii opened this issue Dec 28, 2023 · 1 comment · Fixed by #11227
Labels
C-Bug An unexpected or incorrect behavior C-Performance A change motivated by improving speed, memory usage or compile times O-Web Specific to web (WASM) builds P-Regression Functionality that used to work but no longer does. Add a test for this!
Milestone

Comments

@eerii
Copy link
Contributor

eerii commented Dec 28, 2023

Bevy version

  • Original: 0.12.1
  • Current: 8067e46

Relevant system information

Please include:

Cargo version: cargo 1.76.0-nightly

OS: Arch Linux, Kernel 6.6.7

CPU: Intel(R) Core(TM) i7-4770HQ

GPU: None (integrated)

Using trunk for builds.

What's performing poorly?

Updating from 0.12.1 to main makes wasm builds very slow, they take 100% of the GPU and drop a lot of frames. This doesn't happen with native builds, they run the same as before. I tested it on an empty 2d scene with a camera.

Before and After Traces

Additional information

Minimum example (2d shapes from the examples folder with minimum trunk setup)
minimum_project.zip

100ms before

100ms before

100ms after

100ms after

100ms native (no lag) tracy

image

@eerii eerii added C-Bug An unexpected or incorrect behavior C-Performance A change motivated by improving speed, memory usage or compile times P-Regression Functionality that used to work but no longer does. Add a test for this! S-Needs-Triage This issue needs to be labelled labels Dec 28, 2023
@alice-i-cecile alice-i-cecile added O-Web Specific to web (WASM) builds and removed S-Needs-Triage This issue needs to be labelled labels Dec 28, 2023
@alice-i-cecile alice-i-cecile added this to the 0.13 milestone Dec 28, 2023
@alice-i-cecile
Copy link
Member

nicopap added a commit to nicopap/bevy that referenced this issue Jan 5, 2024
github-merge-queue bot pushed a commit that referenced this issue Jan 6, 2024
# Objective

- Since #10702, the way bevy updates the window leads to major slowdowns
as seen in
    - #11122 
    - #11220
- Slow is bad, furthermore, _very_ slow is _very_ bad. We should fix
this issue.

## Solution

- Move the app update code into the `Event::WindowEvent { event:
WindowEvent::RedrawRequested }` branch of the event loop.
- Run `window.request_redraw()` When `runner_state.redraw_requested`
- Instead of swapping `ControlFlow` between `Poll` and `Wait`, we always
keep it at `Wait`, and use `window.request_redraw()` to schedule an
immediate call to the event loop.
- `runner_state.redraw_requested` is set to `true` when
`UpdateMode::Continuous` and when a `RequestRedraw` event is received.
- Extract the redraw code into a separate function, because otherwise
I'd go crazy with the indentation level.
- Fix #11122.

## Testing

I tested the WASM builds as follow:

```sh
cargo run -p build-wasm-example -- --api webgl2 bevymark
python -m http.server --directory examples/wasm/ 8080
# Open browser at http://localhost:8080
```

On main, even spawning a couple sprites is super choppy. Even if it says
"300 FPS". While on this branch, it is smooth as butter.

I also found that it fixes all choppiness on window resize (tested on
Linux/X11). This was another issue from #10702 IIRC.

So here is what I tested:

- On `wasm`: `many_foxes` and `bevymark`, with `argh::from_env()`
commented out, otherwise we get a cryptic error.
- Both with `PresentMode::AutoVsync` and `PresentMode::AutoNoVsync`
  - On main, it is consistently choppy.
- With this PR, the visible frame rate is consistent with the diagnostic
numbers
- On native (linux/x11) I ran similar tests, making sure that
`AutoVsync` limits to monitor framerate, and `AutoNoVsync` doesn't.

## Future work

Code could be improved, I wanted a quick solution easy to review, but we
really need to make the code more accessible.

- #9768
- ~~**`WinitSettings::desktop_app()` is completely borked.**~~ actually
broken on main as well

### Review guide

Consider enable the non-whitespace diff to see the _real_ change set.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-Bug An unexpected or incorrect behavior C-Performance A change motivated by improving speed, memory usage or compile times O-Web Specific to web (WASM) builds P-Regression Functionality that used to work but no longer does. Add a test for this!
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants