Skip to content

Commit

Permalink
Optimize rendering slow-down at high entity counts (bevyengine#5509)
Browse files Browse the repository at this point in the history
# 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 <[email protected]>
  • Loading branch information
2 people authored and Pietrek14 committed Dec 17, 2022
1 parent 42d0ea7 commit a065123
Show file tree
Hide file tree
Showing 3 changed files with 39 additions and 9 deletions.
4 changes: 4 additions & 0 deletions crates/bevy_ecs/src/archetype.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,14 @@ use std::{
};

#[derive(Debug, Copy, Clone, Eq, PartialEq, Hash)]
#[repr(transparent)]
pub struct ArchetypeId(usize);

impl ArchetypeId {
pub const EMPTY: ArchetypeId = ArchetypeId(0);
/// # Safety:
///
/// This must always have an all-1s bit pattern to ensure soundness in fast entity id space allocation.
pub const INVALID: ArchetypeId = ArchetypeId(usize::MAX);

#[inline]
Expand Down
21 changes: 21 additions & 0 deletions crates/bevy_ecs/src/entity/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -563,6 +563,9 @@ impl Entities {
/// Flush _must_ set the entity location to the correct [`ArchetypeId`] for the given [`Entity`]
/// each time init is called. This _can_ be [`ArchetypeId::INVALID`], provided the [`Entity`]
/// has not been assigned to an [`Archetype`][crate::archetype::Archetype].
///
/// Note: freshly-allocated entities (ones which don't come from the pending list) are guaranteed
/// to be initialized with the invalid archetype.
pub unsafe fn flush(&mut self, mut init: impl FnMut(Entity, &mut EntityLocation)) {
let free_cursor = self.free_cursor.get_mut();
let current_free_cursor = *free_cursor;
Expand Down Expand Up @@ -613,6 +616,20 @@ impl Entities {
}
}

/// # Safety
///
/// This function is safe if and only if the world this Entities is on has no entities.
pub unsafe fn flush_and_reserve_invalid_assuming_no_entities(&mut self, count: usize) {
let free_cursor = self.free_cursor.get_mut();
*free_cursor = 0;
self.meta.reserve(count);
// the EntityMeta struct only contains integers, and it is valid to have all bytes set to u8::MAX
self.meta.as_mut_ptr().write_bytes(u8::MAX, count);
self.meta.set_len(count);

self.len = count as u32;
}

/// Accessor for getting the length of the vec in `self.meta`
#[inline]
pub fn meta_len(&self) -> usize {
Expand All @@ -630,7 +647,10 @@ impl Entities {
}
}

// Safety:
// This type must not contain any pointers at any level, and be safe to fully fill with u8::MAX.
#[derive(Copy, Clone, Debug)]
#[repr(C)]
pub struct EntityMeta {
pub generation: u32,
pub location: EntityLocation,
Expand All @@ -648,6 +668,7 @@ impl EntityMeta {

/// A location of an entity in an archetype.
#[derive(Copy, Clone, Debug)]
#[repr(C)]
pub struct EntityLocation {
/// The archetype index
pub archetype_id: ArchetypeId,
Expand Down
23 changes: 14 additions & 9 deletions crates/bevy_render/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -244,15 +244,20 @@ impl Plugin for RenderPlugin {
// reserve all existing app entities for use in render_app
// they can only be spawned using `get_or_spawn()`
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.
unsafe { render_app.world.entities_mut() }.flush_as_invalid();

assert_eq!(
render_app.world.entities().len(),
0,
"An entity was spawned after the entity list was cleared last frame and before the extract stage began. This is not supported",
);

// This is safe given the clear_entities call in the past frame and the assert above
unsafe {
render_app
.world
.entities_mut()
.flush_and_reserve_invalid_assuming_no_entities(meta_len);
}
}

{
Expand Down

0 comments on commit a065123

Please sign in to comment.