Skip to content

Commit

Permalink
Faster entity cloning (#16717)
Browse files Browse the repository at this point in the history
# Objective

#16132 introduced entity cloning functionality, and while it works and
is useful, it can be made faster. This is the promised follow-up to
improve performance.

## Solution

**PREFACE**: This is my first time writing `unsafe` in rust and I have
only vague idea about what I'm doing. I would encourage reviewers to
scrutinize `unsafe` parts in particular.

The solution is to clone component data to an intermediate buffer and
use `EntityWorldMut::insert_by_ids` to insert components without
additional archetype moves.

To facilitate this, `EntityCloner::clone_entity` now reads all
components of the source entity and provides clone handlers with the
ability to read component data straight from component storage using
`read_source_component` and write to an intermediate buffer using
`write_target_component`. `ComponentId` is used to check that requested
type corresponds to the type available on source entity.

Reflect-based handler is a little trickier to pull of: we only have
`&dyn Reflect` and no direct access to the underlying data.
`ReflectFromPtr` can be used to get `&dyn Reflect` from concrete
component data, but to write it we need to create a clone of the
underlying data using `Reflect`. For this reason only components that
have `ReflectDefault` or `ReflectFromReflect` or `ReflectFromWorld` can
be cloned, all other components will be skipped. The good news is that
this is actually only a temporary limitation: once #13432 lands we will
be able to clone component without requiring one of these `type data`s.

This PR also introduces `entity_cloning` benchmark to better compare
changes between the PR and main, you can see the results in the
**showcase** section.

## Testing

- All previous tests passing
- Added test for fast reflect clone path (temporary, will be removed
after reflection-based cloning lands)
- Ran miri

## Showcase
Here's a table demonstrating the improvement:

| **benchmark** | **main, avg** | **PR, avg** | **change, avg** |
| ----------------------- | ------------- | ----------- |
--------------- |
| many components reflect | 18.505 µs | 2.1351 µs | -89.095% |
| hierarchy wide reflect* | 22.778 ms | 4.1875 ms | -81.616% |
| hierarchy tall reflect* | 107.24 µs | 26.322 µs | -77.141% |
| hierarchy many reflect | 78.533 ms | 9.7415 ms | -87.596% |
| many components clone | 1.3633 µs | 758.17 ns | -45.937% |
| hierarchy wide clone* | 2.7716 ms | 3.3411 ms | +20.546% |
| hierarchy tall clone* | 17.646 µs | 20.190 µs | +17.379% |
| hierarchy many clone | 5.8779 ms | 4.2650 ms | -27.439% |

*: these benchmarks have entities with only 1 component

## Considerations
Once #10154 is resolved a large part of the functionality in this PR
will probably become obsolete. It might still be a little bit faster
than using command batching, but the complexity might not be worth it.

## Migration Guide
- `&EntityCloner` in component clone handlers is changed to `&mut
ComponentCloneCtx` to better separate data.
- Changed `EntityCloneHandler` from enum to struct and added convenience
functions to add default clone and reflect handler more easily.

---------

Co-authored-by: Alice Cecile <[email protected]>
Co-authored-by: Chris Russell <[email protected]>
  • Loading branch information
3 people authored Dec 18, 2024
1 parent 3ef99cf commit 8235716
Show file tree
Hide file tree
Showing 10 changed files with 926 additions and 190 deletions.
5 changes: 5 additions & 0 deletions benches/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,11 @@ unexpected_cfgs = { level = "warn", check-cfg = ['cfg(docsrs_dep)'] }
unsafe_op_in_unsafe_fn = "warn"
unused_qualifications = "warn"

[[bench]]
name = "entity_cloning"
path = "benches/bevy_ecs/entity_cloning.rs"
harness = false

[[bench]]
name = "ecs"
path = "benches/bevy_ecs/main.rs"
Expand Down
171 changes: 171 additions & 0 deletions benches/benches/bevy_ecs/entity_cloning.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,171 @@
use bevy_ecs::bundle::Bundle;
use bevy_ecs::reflect::AppTypeRegistry;
use bevy_ecs::{component::Component, reflect::ReflectComponent, world::World};
use bevy_hierarchy::{BuildChildren, CloneEntityHierarchyExt};
use bevy_math::Mat4;
use bevy_reflect::{GetTypeRegistration, Reflect};
use criterion::{black_box, criterion_group, criterion_main, Bencher, Criterion};

criterion_group!(benches, reflect_benches, clone_benches);
criterion_main!(benches);

#[derive(Component, Reflect, Default, Clone)]
#[reflect(Component)]
struct C1(Mat4);

#[derive(Component, Reflect, Default, Clone)]
#[reflect(Component)]
struct C2(Mat4);

#[derive(Component, Reflect, Default, Clone)]
#[reflect(Component)]
struct C3(Mat4);

#[derive(Component, Reflect, Default, Clone)]
#[reflect(Component)]
struct C4(Mat4);

#[derive(Component, Reflect, Default, Clone)]
#[reflect(Component)]
struct C5(Mat4);

#[derive(Component, Reflect, Default, Clone)]
#[reflect(Component)]
struct C6(Mat4);

#[derive(Component, Reflect, Default, Clone)]
#[reflect(Component)]
struct C7(Mat4);

#[derive(Component, Reflect, Default, Clone)]
#[reflect(Component)]
struct C8(Mat4);

#[derive(Component, Reflect, Default, Clone)]
#[reflect(Component)]
struct C9(Mat4);

#[derive(Component, Reflect, Default, Clone)]
#[reflect(Component)]
struct C10(Mat4);

type ComplexBundle = (C1, C2, C3, C4, C5, C6, C7, C8, C9, C10);

fn hierarchy<C: Bundle + Default + GetTypeRegistration>(
b: &mut Bencher,
width: usize,
height: usize,
clone_via_reflect: bool,
) {
let mut world = World::default();
let registry = AppTypeRegistry::default();
{
let mut r = registry.write();
r.register::<C>();
}
world.insert_resource(registry);
world.register_bundle::<C>();
if clone_via_reflect {
let mut components = Vec::new();
C::get_component_ids(world.components(), &mut |id| components.push(id.unwrap()));
for component in components {
world
.get_component_clone_handlers_mut()
.set_component_handler(
component,
bevy_ecs::component::ComponentCloneHandler::reflect_handler(),
);
}
}

let id = world.spawn(black_box(C::default())).id();

let mut hierarchy_level = vec![id];

for _ in 0..height {
let current_hierarchy_level = hierarchy_level.clone();
hierarchy_level.clear();
for parent_id in current_hierarchy_level {
for _ in 0..width {
let child_id = world
.spawn(black_box(C::default()))
.set_parent(parent_id)
.id();
hierarchy_level.push(child_id)
}
}
}
world.flush();

b.iter(move || {
world.commands().entity(id).clone_and_spawn_with(|builder| {
builder.recursive(true);
});
world.flush();
});
}

fn simple<C: Bundle + Default + GetTypeRegistration>(b: &mut Bencher, clone_via_reflect: bool) {
let mut world = World::default();
let registry = AppTypeRegistry::default();
{
let mut r = registry.write();
r.register::<C>();
}
world.insert_resource(registry);
world.register_bundle::<C>();
if clone_via_reflect {
let mut components = Vec::new();
C::get_component_ids(world.components(), &mut |id| components.push(id.unwrap()));
for component in components {
world
.get_component_clone_handlers_mut()
.set_component_handler(
component,
bevy_ecs::component::ComponentCloneHandler::reflect_handler(),
);
}
}
let id = world.spawn(black_box(C::default())).id();

b.iter(move || {
world.commands().entity(id).clone_and_spawn();
world.flush();
});
}

fn reflect_benches(c: &mut Criterion) {
c.bench_function("many components reflect", |b| {
simple::<ComplexBundle>(b, true);
});

c.bench_function("hierarchy wide reflect", |b| {
hierarchy::<C1>(b, 10, 4, true);
});

c.bench_function("hierarchy tall reflect", |b| {
hierarchy::<C1>(b, 1, 50, true);
});

c.bench_function("hierarchy many reflect", |b| {
hierarchy::<ComplexBundle>(b, 5, 5, true);
});
}

fn clone_benches(c: &mut Criterion) {
c.bench_function("many components clone", |b| {
simple::<ComplexBundle>(b, false);
});

c.bench_function("hierarchy wide clone", |b| {
hierarchy::<C1>(b, 10, 4, false);
});

c.bench_function("hierarchy tall clone", |b| {
hierarchy::<C1>(b, 1, 50, false);
});

c.bench_function("hierarchy many clone", |b| {
hierarchy::<ComplexBundle>(b, 5, 5, false);
});
}
1 change: 1 addition & 0 deletions crates/bevy_ecs/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,7 @@ spin = { version = "0.9.8", default-features = false, features = [
] }
tracing = { version = "0.1", default-features = false, optional = true }
log = { version = "0.4", default-features = false }
bumpalo = "3"

[dev-dependencies]
rand = "0.8"
Expand Down
Loading

0 comments on commit 8235716

Please sign in to comment.