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

High entity counts cause decreased rendering performance even when rendering nothing. #3953

Closed
StarArawn opened this issue Feb 14, 2022 · 10 comments
Labels
A-Rendering Drawing game state to the screen C-Bug An unexpected or incorrect behavior C-Performance A change motivated by improving speed, memory usage or compile times P-High This is particularly urgent, and deserves immediate attention

Comments

@StarArawn
Copy link
Contributor

StarArawn commented Feb 14, 2022

Bevy version

Main branch commit: d8974e7c3d84189313268f475ceeff9393cedffa

Operating system & version

Windows 10 Vulkan and DX12 API.

What you did

The example code is small and easy to understand:

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

#[derive(Component)]
pub struct Nothing;

#[derive(Bundle)]
pub struct NoBundle {
    nothing: Nothing,
}

fn startup(
    mut commands: Commands,
) {
    let mut entities = Vec::new();
    for _ in 0..5000000 {
        entities.push(NoBundle {
            nothing: Nothing,
        });
    }

    commands.spawn_batch(entities);
}

fn main() {
    App::new()
        .insert_resource(WindowDescriptor {
            width: 1270.0,
            height: 720.0,
            title: String::from("Bug"),
            ..Default::default()
        })
        .insert_resource(ClearColor(Color::rgb(0.211, 0.643, 0.949)))
        .add_plugin(FrameTimeDiagnosticsPlugin::default())
        .add_plugin(LogDiagnosticsPlugin::default())
        .add_plugins(DefaultPlugins)
        .add_startup_system(startup)
        .run();
}

What you expected to happen

Creating large amounts of entities shouldn't decrease rendering performance unless they are actually drawing something to the screen.

What actually happened

Performance decreases rapidly with increased entity counts. On higher end machines this is less noticeable, but still becomes an issue.

Additional information

The decrease in performance can be traced to this:

let meta_len = app_world.entities().meta.len();
render_app
.world
.entities()
.reserve_entities(meta_len as u32);
// flushing as "invalid" ensures that app world entities aren't added as "empty archetype" entities by default
// these entities cannot be accessed without spawning directly onto them
// this _only_ works as expected because clear_entities() is called at the end of every frame.
render_app.world.entities_mut().flush_as_invalid();

Reserving entity ID's for every entity seems to be costly when dealing with a large number of entities.

Possible solutions:

  • Don't reserve entities that don't render anything. I'm not sure how performant that will be overall?
  • Possibly this could be mitigated by using a sub world where you could store extremely large numbers of entities without the rendering world being aware of said entities.
@StarArawn StarArawn added C-Bug An unexpected or incorrect behavior S-Needs-Triage This issue needs to be labelled labels Feb 14, 2022
@alice-i-cecile alice-i-cecile added A-Rendering Drawing game state to the screen 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 Feb 14, 2022
@alice-i-cecile
Copy link
Member

Could we cache the reserved entities in some fashion? It seems strange that we're reserving this list from scratch each time.

@StarArawn
Copy link
Contributor Author

Could we cache the reserved entities in some fashion? It seems strange that we're reserving this list from scratch each time.

Possibly, I'm not sure. We might have to benchmark a few different approaches to see which one is best. 🤔

@zyllian
Copy link

zyllian commented Mar 24, 2022

I've implemented a possible solution in my fork which reserves the number of entities used in the render world for the previous frame.

This seems to fix performance with large numbers of non-rendering entities but does mean there's a single-frame slowdown if there are a lot of new rendering entities spawned. I think this is generally solvable by having a (configurable, maybe) minimum number of entities reserved.

I tested this with the many_sprites and many_cubes examples (with an additional 50,000,000 entities each) with no apparent slowdown. Still, I'm not familiar with Bevy's internals and there might be something I've missed here.

@alice-i-cecile
Copy link
Member

Excellent! @zyllian, are you comfortable making a PR for this? It will be easier to discuss and verify this further there.

@zyllian
Copy link

zyllian commented Mar 24, 2022

@alice-i-cecile Sure, here it is.

@alice-i-cecile
Copy link
Member

So, thinking about this more: can we use the ExactSizeIterator tech in #4244 to get a quick and exact estimate? If so, we should merge that ASAP and fix it like that.

@sambiak
Copy link

sambiak commented May 29, 2022

Hello, what issues were there with Zylian's fix ? I see it has been closed, but there was no explanation given.

@alice-i-cecile
Copy link
Member

alice-i-cecile commented May 29, 2022

From the discussion:

Rob: The point of reserving the entities is so that the same entity from the main world can be used to refer to an entity in the render world. If we don’t reserve the entire active main world entity address space, something could be spawned after what is reserved and then something else is supposed to refer to that. I’m not convinced this change is safe to do. I’ll need to think about it some more. Maybe @cart can beat me to it.

Cart: Yup the approach in this PR breaks the intended behavior. This line isn't about reserving space for rendering entities. Its about ensuring that rendering entities don't get spawned in the "current app id space". We should not merge this PR.

@alice-i-cecile alice-i-cecile added the P-High This is particularly urgent, and deserves immediate attention label Oct 24, 2022
bors bot pushed a commit that referenced this issue Oct 24, 2022
# Objective

- Improve #3953

## Solution

- The very specific circumstances under which the render world is reset meant that the flush_as_invalid function could be replaced with one that had a noop as its init method.
- This removes a double-writing issue leading to greatly increased performance.

Running the reproduction code in the linked issue, this change nearly doubles the framerate.

Co-authored-by: Carter Anderson <mcanders1@gmail.com>
@JMS55
Copy link
Contributor

JMS55 commented Oct 26, 2022

Closed by #5509?

@alice-i-cecile
Copy link
Member

Yeah, let's close this for now. If further issues crop up, we can make a new issue.

james7132 pushed a commit to james7132/bevy that referenced this issue Oct 28, 2022
# Objective

- Improve bevyengine#3953

## Solution

- The very specific circumstances under which the render world is reset meant that the flush_as_invalid function could be replaced with one that had a noop as its init method.
- This removes a double-writing issue leading to greatly increased performance.

Running the reproduction code in the linked issue, this change nearly doubles the framerate.

Co-authored-by: Carter Anderson <mcanders1@gmail.com>
Pietrek14 pushed a commit to Pietrek14/bevy that referenced this issue Dec 17, 2022
# Objective

- Improve bevyengine#3953

## Solution

- The very specific circumstances under which the render world is reset meant that the flush_as_invalid function could be replaced with one that had a noop as its init method.
- This removes a double-writing issue leading to greatly increased performance.

Running the reproduction code in the linked issue, this change nearly doubles the framerate.

Co-authored-by: Carter Anderson <mcanders1@gmail.com>
ItsDoot pushed a commit to ItsDoot/bevy that referenced this issue Feb 1, 2023
# Objective

- Improve bevyengine#3953

## Solution

- The very specific circumstances under which the render world is reset meant that the flush_as_invalid function could be replaced with one that had a noop as its init method.
- This removes a double-writing issue leading to greatly increased performance.

Running the reproduction code in the linked issue, this change nearly doubles the framerate.

Co-authored-by: Carter Anderson <mcanders1@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Rendering Drawing game state to the screen C-Bug An unexpected or incorrect behavior C-Performance A change motivated by improving speed, memory usage or compile times P-High This is particularly urgent, and deserves immediate attention
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants