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

WASM perf regression: App updates triggered by input events #13965

Closed
Azorlogh opened this issue Jun 21, 2024 · 2 comments · Fixed by #14023
Closed

WASM perf regression: App updates triggered by input events #13965

Azorlogh opened this issue Jun 21, 2024 · 2 comments · Fixed by #14023
Labels
A-Windowing Platform-agnostic interface layer to run your app in C-Bug An unexpected or incorrect behavior C-Performance A change motivated by improving speed, memory usage or compile times D-Complex Quite challenging from either a design or technical perspective. Ask for help! P-Regression Functionality that used to work but no longer does. Add a test for this! S-Needs-Review Needs reviewer attention (from anyone!) to move forward
Milestone

Comments

@Azorlogh
Copy link
Contributor

Azorlogh commented Jun 21, 2024

Bevy version

  • Original: bevy-0.13.2
  • Current: bevy-0.14.0-rc.3

Relevant system information

Please include:

  • Rust version: 0.78.0
  • Firefox 126.0.1
  • Linux/x11
  • Nvidia proprietary 550.90.07

What's performing poorly?

I discovered that after updating my wasm app to 0.14, it feels very sluggish.

Here is a minimal reproducible example:
When idle the square moves in a circle. Moving the cursor or keeping a key pressed, the square will begin to skip around.

use bevy::{prelude::*, sprite::MaterialMesh2dBundle};

fn main() {
    App::new()
        .add_plugins(DefaultPlugins)
        .insert_resource(A(0))
        .add_systems(Startup, setup)
        .add_systems(Update, run)
        .run();
}

fn setup(
    mut commands: Commands,
    mut meshes: ResMut<Assets<Mesh>>,
    mut materials: ResMut<Assets<ColorMaterial>>,
) {
    commands.spawn(Camera2dBundle::default());
    commands.spawn((
        MaterialMesh2dBundle {
            mesh: meshes.add(Rectangle::default()).into(),
            transform: Transform::default().with_scale(Vec3::splat(128.)),
            material: materials.add(Color::WHITE),
            ..default()
        },
        Angle(0),
    ));
}

#[derive(Debug, Component)]
struct Angle(u8);

#[derive(Resource)]
struct A(u128);

fn run(mut q_angle: Query<(&mut Angle, &mut Transform)>, mut a: ResMut<A>) {
    // do something slow
    for i in 0..10_000_000 {
        a.0 += i * i;
    }

    for (mut angle, mut transform) in &mut q_angle {
        angle.0 = (angle.0 + 1) % 8;
        transform.translation =
            (Vec2::from_angle(angle.0 as f32 / 8.0 * std::f32::consts::TAU) * 250.0).extend(0.0);
    }
}

Additional information

We can see that after 0.14, updates are triggered by pointermove events. Painting only happens about once every 3 frames as a result. The same happens when keeping a key pressed.

  • Before:
    image

  • After:
    image

@Azorlogh Azorlogh 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 Jun 21, 2024
@Azorlogh Azorlogh changed the title WASM perf regression: App updates triggered by pointermove events WASM perf regression: App updates triggered by input events Jun 21, 2024
@alice-i-cecile
Copy link
Member

To me it looks like when should_update is true we are now calling run_app_update directly here
Where as before, we used to only request a redraw

By @tychedelia on Discord.

@alice-i-cecile alice-i-cecile added A-Windowing Platform-agnostic interface layer to run your app in S-Ready-For-Implementation This issue is ready for an implementation PR. Go for it! C-Performance A change motivated by improving speed, memory usage or compile times and removed S-Needs-Triage This issue needs to be labelled C-Performance A change motivated by improving speed, memory usage or compile times labels Jun 21, 2024
@alice-i-cecile alice-i-cecile added this to the 0.14 milestone Jun 21, 2024
@beoboo
Copy link

beoboo commented Jun 23, 2024

My 2c here: the loop is very different from before, and it's more oriented to something like:

  • wait for NewEvents (triggered by window, user or device events or a timeout between frames)
  • execute all events in the middle
  • run AboutToWait

The loop is also not sensitive to the specific event type (e.g., it treats mouse events in the same manner as window resize ones). For each event that wakes up the loop, it updates the app.

It could be more selective, and define which event triggers what: for example, mouse events could reasonably trigger request redraws only, and request redraws could trigger app updates. The only big difference from before is that app updates need to be executed inside AboutToWait, since this also changes the UpdateMode that sets the new wait duration for the next frame, and that needs to happen in the last event of the cycle. (it's not only that, but this gives a rough idea).

@alice-i-cecile alice-i-cecile added the D-Complex Quite challenging from either a design or technical perspective. Ask for help! label Jun 25, 2024
@alice-i-cecile alice-i-cecile added S-Needs-Review Needs reviewer attention (from anyone!) to move forward and removed S-Ready-For-Implementation This issue is ready for an implementation PR. Go for it! labels Jun 26, 2024
github-merge-queue bot pushed a commit that referenced this issue Jun 26, 2024
# Objective

- Fixes #13965 

## Solution

- Don't run multiple updates for a single frame
mockersf added a commit that referenced this issue Jun 30, 2024
# Objective

- Fixes #13965 

## Solution

- Don't run multiple updates for a single frame
zmbush pushed a commit to zmbush/bevy that referenced this issue Jul 3, 2024
# Objective

- Fixes bevyengine#13965 

## Solution

- Don't run multiple updates for a single frame
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-Performance A change motivated by improving speed, memory usage or compile times D-Complex Quite challenging from either a design or technical perspective. Ask for help! P-Regression Functionality that used to work but no longer does. Add a test for this! S-Needs-Review Needs reviewer attention (from anyone!) to move forward
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants