Skip to content

Handles to Assets leaking #4589

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

Open
semtexzv opened this issue Apr 25, 2022 · 4 comments
Open

Handles to Assets leaking #4589

semtexzv opened this issue Apr 25, 2022 · 4 comments
Labels
A-Assets Load files from disk to use for things like images, models, and sounds C-Bug An unexpected or incorrect behavior C-Performance A change motivated by improving speed, memory usage or compile times

Comments

@semtexzv
Copy link

Bevy version

Applicable to both 0.6 and 0.7

Operating system & version

MacOs Big Sur

What you did

use bevy::core::FixedTimestep;
use bevy::diagnostic::{FrameTimeDiagnosticsPlugin, LogDiagnosticsPlugin};
use bevy::prelude::*;
use bevy::sprite::MaterialMesh2dBundle;

const SCALE: f32 = 32.;
const GRID: usize = 16;

#[derive(Default)]
pub struct Grid([[f32; GRID]; GRID]);

#[derive(Default)]
pub struct GridElems([[Option<Entity>; GRID]; GRID]);

#[derive(Component)]
pub struct Idx([usize; 2]);

fn main() {
    App::new()
        .add_plugins(DefaultPlugins)
        .add_plugin(LogDiagnosticsPlugin::default())
        .add_plugin(FrameTimeDiagnosticsPlugin::default())
        .add_startup_system(setup)
        .add_system_set(
            SystemSet::new()
                .with_run_criteria(FixedTimestep::steps_per_second(60.))
                .with_system(noisify)
                .with_system(update)
        )
        .init_resource::<Grid>()
        .init_resource::<GridElems>()
        .run();
}

fn setup(mut commands: Commands, mut meshes: ResMut<Assets<Mesh>>, mut mats: ResMut<Assets<ColorMaterial>>) {
    commands.spawn_bundle(OrthographicCameraBundle::new_2d());
}

fn noisify(
    mut commands: Commands,
    grid: ResMut<Grid>,
) {
    grid.into_inner().0.iter_mut().for_each(|row| {
        row.iter_mut().for_each(|e| {
            *e = ::rand::random();
        })
    })
}

fn update(
    mut commands: Commands,
    grid: Res<Grid>,
    mut elems: ResMut<GridElems>,
    mut meshes: ResMut<Assets<Mesh>>,
    mut mats: ResMut<Assets<ColorMaterial>>) {
    if !grid.is_added() && !grid.is_changed() {
        return;
    }
    let elems = elems.into_inner();

    for x in 0..GRID {
        for y in 0..GRID {
            if let Some(elem) = elems.0[y][x] {
                commands.entity(elem).despawn_recursive();
            }
        }
    }

    for y in 0..GRID {
        for x in 0..GRID {
            elems.0[y][x] = Some(
                commands.spawn_bundle(MaterialMesh2dBundle {
                    mesh: meshes.add(Mesh::from(shape::Quad::default())).into(),
                    material: mats.add(ColorMaterial::from(Color::from([0.0, grid.0[y][x], 0.0]))).into(),
                    transform: Transform::default().with_scale(Vec3::splat(SCALE))
                        .with_translation(Vec3::new(SCALE * x as f32, SCALE * y as f32, 0.0)),
                    ..Default::default()
                }).id());
        }
    }

    println!("{:?}", meshes.into_inner().iter().count() );
}
// fn greet_people(time: Res<Time>) {
//     // for name in query.iter() {
//         println!("hello {time}!", time = time.into_inner().delta_seconds_f64());
//     // }
// }

What you expected to happen

Stable FPS, dropping and re-creating meshes on every iteraton of the update system

What actually happened

Meshes are not freed

@semtexzv semtexzv added C-Bug An unexpected or incorrect behavior S-Needs-Triage This issue needs to be labelled labels Apr 25, 2022
@alice-i-cecile alice-i-cecile added A-Assets Load files from disk to use for things like images, models, and sounds C-Performance A change motivated by improving speed, memory usage or compile times and removed S-Needs-Triage This issue needs to be labelled labels Apr 25, 2022
@oddfacade
Copy link
Contributor

Minimal reproduction on linux:

use bevy::core::FixedTimestep;
use bevy::diagnostic::{FrameTimeDiagnosticsPlugin, LogDiagnosticsPlugin};
use bevy::prelude::*;

const COUNT: usize = 1024;

fn main() {
    App::new()
        .add_plugins(DefaultPlugins)
        .add_plugin(LogDiagnosticsPlugin::default())
        .add_plugin(FrameTimeDiagnosticsPlugin::default())
        .add_system_set(
            SystemSet::new()
                .with_run_criteria(FixedTimestep::steps_per_second(60.))
                .with_system(update)
        )
        .run();
}

fn update(
    mut meshes: ResMut<Assets<Mesh>>,
    mut mesh_handles: Local<Vec<Handle<Mesh>>>,
    ) {

    mesh_handles.drain(..).for_each(drop);

    for _ in 0..COUNT {
        mesh_handles.push(meshes.add(Mesh::from(shape::Cube::default())));
    }

    println!("{:?}", meshes.into_inner().iter().count() );
}

It has something to do with the fixed timestep. The issue is not present if update is registered via add_system. The issue is also not present for a low enough value of COUNT (try 16), although the number of meshes reported does fluctuate in that case. This indicates to me that the assets are being created faster than they can be dropped.

@mockersf
Copy link
Member

the code above runs at less than 60 fps on my computer, so it's running the system several times per frame, every time being slower, so every time running the system more time per frame, which is why the count of meshes keeps increasing

@oddfacade
Copy link
Contributor

oddfacade commented Apr 27, 2022

this is the case on my system as well. explicitly removing the meshes functions as a workaround. edit: this seems to only mask the issue by changing the number of meshes reported.

fn update(mut meshes: ResMut<Assets<Mesh>>, mut mesh_handles: Local<Vec<Handle<Mesh>>>) {
    mesh_handles.drain(..).for_each(|handle| {
        meshes
            .remove(handle)
            .map(drop)
            .expect("Failed to remove asset.")
    });

    mesh_handles
        .extend(iter::repeat_with(|| meshes.add(Mesh::from(shape::Cube::default()))).take(COUNT));

    println!("{:?}", meshes.into_inner().iter().count());
}

@HackerFoo
Copy link
Contributor

#5692 might help by letting the FixedTimestep system slip if it gets too far behind.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Assets Load files from disk to use for things like images, models, and sounds C-Bug An unexpected or incorrect behavior C-Performance A change motivated by improving speed, memory usage or compile times
Projects
None yet
Development

No branches or pull requests

5 participants