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

Faster entity cloning #16717

Merged
merged 46 commits into from
Dec 18, 2024
Merged
Show file tree
Hide file tree
Changes from 19 commits
Commits
Show all changes
46 commits
Select commit Hold shift + click to select a range
0b0ab27
add simple entity cloning benchmark
eugineerd Dec 4, 2024
6316f3c
first draft of faster cloning
eugineerd Dec 4, 2024
d4ac17a
use bumpalo to store component data
eugineerd Dec 7, 2024
c710dff
add `read_source_component_reflect`
eugineerd Dec 7, 2024
581d2d0
redesign `ComponentCloneHandler`
eugineerd Dec 7, 2024
c65f20d
clippy fixes
eugineerd Dec 7, 2024
1e34bfa
Merge remote-tracking branch 'upstream/main' into entity_cloning_faster
eugineerd Dec 7, 2024
676b7f2
implement fast entity cloning for reflect handler
eugineerd Dec 8, 2024
fd73075
improve entity cloning benchmark
eugineerd Dec 8, 2024
34f4d30
Merge branch 'entity_cloning_faster_reflect' into entity_cloning_faster
eugineerd Dec 8, 2024
56e6ab2
clippy fixes
eugineerd Dec 8, 2024
b4f53d5
fix doctests
eugineerd Dec 8, 2024
8699057
actually fix doctests
eugineerd Dec 8, 2024
4cf7e6e
last doc fixes (hopefully)
eugineerd Dec 8, 2024
8b6c9a1
fix entity cloning using reflect tests to actually use correct handlers
eugineerd Dec 10, 2024
3aefdaf
actually write pointers in `write_target_component_reflect`
eugineerd Dec 10, 2024
81558c0
don't allow double write in `write_target_component_reflect`
eugineerd Dec 10, 2024
fbaefd8
Merge remote-tracking branch 'upstream/main' into entity_cloning_faster
eugineerd Dec 10, 2024
a9739a1
fix `entity_cloning` benchmark after rename
eugineerd Dec 10, 2024
588eba7
use `ReflectFromReflect` in `component_clone_via_reflect` fast path
eugineerd Dec 16, 2024
5c7c42e
use `default_handler` instead of `default` for `ComponentCloneHandler`
eugineerd Dec 16, 2024
41d5619
Merge remote-tracking branch 'upstream/main' into entity_cloning_faster
eugineerd Dec 16, 2024
7b41bf9
Add clarification for `clone_fast` and `clone_slow` in `component_clo…
eugineerd Dec 16, 2024
53ec75d
fix some post-merge weirdness
eugineerd Dec 16, 2024
cff4ba6
Update crates/bevy_ecs/src/entity/clone_entities.rs
eugineerd Dec 16, 2024
9601364
add test to make sure reflect handler doesn't panic if component can'…
eugineerd Dec 17, 2024
56ff78d
rewrite `component_clone_via_reflect` to not panic
eugineerd Dec 17, 2024
fe31086
put all reflect tests in one module
eugineerd Dec 17, 2024
d484a9f
Merge remote-tracking branch 'origin/entity_cloning_faster' into enti…
eugineerd Dec 17, 2024
52b1f26
remove unused import in benchmark
eugineerd Dec 17, 2024
e86b963
remove outdated comment
eugineerd Dec 17, 2024
2d11e57
fix leak in `component_clone_via_reflect`'s `ReflectFromWorld` path
eugineerd Dec 18, 2024
7b0f45d
make `read_source_component_reflect` return `None` when `ReflectFromP…
eugineerd Dec 18, 2024
3cb6891
use `debug_checked_unwrap` for getting source component ptr
eugineerd Dec 18, 2024
0de432d
use `type_id()` instead of `reflect_type_info().type_id()` in `write_…
eugineerd Dec 18, 2024
ca17d74
store `ComponentInfo` in `ComponentCloneCtx`
eugineerd Dec 18, 2024
3c70318
use stored `ComponentInfo` instead of getting it from `Components`
eugineerd Dec 18, 2024
05a307a
add `write_target_component_ptr` to allow writing component clone han…
eugineerd Dec 18, 2024
b15c917
Merge remote-tracking branch 'upstream/main' into entity_cloning_faster
eugineerd Dec 18, 2024
6f03851
allow `clone_fn` in `write_target_component_ptr` to fail
eugineerd Dec 18, 2024
fbaa0cb
clippy
eugineerd Dec 18, 2024
1ae0d0d
`no_std`
eugineerd Dec 18, 2024
a2a78d6
Fix missed replacement of `self.components` with `self.component_info…
eugineerd Dec 18, 2024
35a1902
add `TypeId` check in `ReflectFromWorld` component clone path
eugineerd Dec 18, 2024
b02678d
fix getting wrong `type_id`
eugineerd Dec 18, 2024
4abf6b9
fix build without default features
eugineerd Dec 18, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions benches/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,11 @@ name = "change_detection"
path = "benches/bevy_ecs/change_detection.rs"
harness = false

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

[[bench]]
name = "ecs"
path = "benches/bevy_ecs/benches.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::prelude::ReflectDefault;
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, Default)]
struct C1(Mat4);

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

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

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

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

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

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

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

#[derive(Component, Reflect, Default, Clone)]
#[reflect(Component, Default)]
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 @@ -44,6 +44,7 @@ arrayvec = { version = "0.7.4", optional = true }
smallvec = { version = "1", features = ["union"] }
indexmap = { version = "2.5.0", default-features = false, features = ["std"] }
variadics_please = "1.0"
bumpalo = "3"

[dev-dependencies]
rand = "0.8"
Expand Down
145 changes: 92 additions & 53 deletions crates/bevy_ecs/src/component.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ use crate::{
archetype::ArchetypeFlags,
bundle::BundleInfo,
change_detection::MAX_CHANGE_AGE,
entity::{Entity, EntityCloner},
entity::{ComponentCloneCtx, Entity},
query::DebugCheckedUnwrap,
storage::{SparseSetIndex, SparseSets, Storages, Table, TableRow},
system::{Local, Resource, SystemParam},
Expand Down Expand Up @@ -973,18 +973,44 @@ impl ComponentDescriptor {
}

/// Function type that can be used to clone an entity.
pub type ComponentCloneFn = fn(&mut DeferredWorld, &EntityCloner);
pub type ComponentCloneFn = fn(&mut DeferredWorld, &mut ComponentCloneCtx);

/// An enum instructing how to clone a component.
/// A struct instructing which clone handler to use when cloning a component.
#[derive(Debug, Default)]
pub enum ComponentCloneHandler {
#[default]
pub struct ComponentCloneHandler(Option<ComponentCloneFn>);

impl ComponentCloneHandler {
/// Use the global default function to clone the component with this handler.
Default,
pub fn default_handler() -> Self {
Self(None)
}

/// Do not clone the component. When a command to clone an entity is issued, component with this handler will be skipped.
Ignore,
pub fn ignore() -> Self {
Self(Some(component_clone_ignore))
}

/// Set clone handler based on `Clone` trait.
///
/// If set as a handler for a component that is not the same as the one used to create this handler, it will panic.
pub fn clone_handler<C: Component + Clone>() -> Self {
Self(Some(component_clone_via_clone::<C>))
}

/// Set clone handler based on `Reflect` trait.
pub fn reflect_handler() -> Self {
Self(Some(component_clone_via_reflect))
}

/// Set a custom handler for the component.
Custom(ComponentCloneFn),
pub fn custom_handler(handler: ComponentCloneFn) -> Self {
Self(Some(handler))
}

/// Get [`ComponentCloneFn`] representing this handler or `None` if set to default handler.
pub fn get_handler(&self) -> Option<ComponentCloneFn> {
self.0
}
}

/// A registry of component clone handlers. Allows to set global default and per-component clone function for all components in the world.
Expand All @@ -995,7 +1021,7 @@ pub struct ComponentCloneHandlers {
}

impl ComponentCloneHandlers {
/// Sets the default handler for this registry. All components with [`Default`](ComponentCloneHandler::Default) handler, as well as any component that does not have an
/// Sets the default handler for this registry. All components with [`default`](ComponentCloneHandler::default_handler) handler, as well as any component that does not have an
/// explicitly registered clone function will use this handler.
///
/// See [Handlers section of `EntityCloneBuilder`](crate::entity::EntityCloneBuilder#handlers) to understand how this affects handler priority.
Expand All @@ -1015,11 +1041,7 @@ impl ComponentCloneHandlers {
if id.0 >= self.handlers.len() {
self.handlers.resize(id.0 + 1, None);
}
match handler {
ComponentCloneHandler::Default => self.handlers[id.0] = None,
ComponentCloneHandler::Ignore => self.handlers[id.0] = Some(component_clone_ignore),
ComponentCloneHandler::Custom(handler) => self.handlers[id.0] = Some(handler),
};
self.handlers[id.0] = handler.0;
}

/// Checks if the specified component is registered. If not, the component will use the default global handler.
Expand Down Expand Up @@ -2050,18 +2072,12 @@ impl RequiredComponents {
///
/// See [`ComponentCloneHandlers`] for more details.
pub fn component_clone_via_clone<C: Clone + Component>(
world: &mut DeferredWorld,
entity_cloner: &EntityCloner,
_world: &mut DeferredWorld,
ctx: &mut ComponentCloneCtx,
) {
let component = world
.entity(entity_cloner.source())
.get::<C>()
.expect("Component must exists on source entity")
.clone();
world
.commands()
.entity(entity_cloner.target())
.insert(component);
if let Some(component) = ctx.read_source_component::<C>() {
ctx.write_target_component(component.clone());
}
}

/// Component [clone handler function](ComponentCloneFn) implemented using reflect.
Expand All @@ -2070,42 +2086,65 @@ pub fn component_clone_via_clone<C: Clone + Component>(
///
/// See [`ComponentCloneHandlers`] for more details.
#[cfg(feature = "bevy_reflect")]
pub fn component_clone_via_reflect(world: &mut DeferredWorld, entity_cloner: &EntityCloner) {
let component_id = entity_cloner.component_id();
let source = entity_cloner.source();
let target = entity_cloner.target();
world.commands().queue(move |world: &mut World| {
world.resource_scope::<crate::reflect::AppTypeRegistry, ()>(|world, registry| {
pub fn component_clone_via_reflect(world: &mut DeferredWorld, ctx: &mut ComponentCloneCtx) {
fn clone_fast(ctx: &mut ComponentCloneCtx) -> Option<()> {
eugineerd marked this conversation as resolved.
Show resolved Hide resolved
let registry = ctx.type_registry()?;
let component = {
let source_component_reflect = ctx.read_source_component_reflect()?;
let registry = registry.read();
let component_info = ctx.components().get_info(ctx.component_id())?;
let type_id = component_info.type_id()?;
let reflect_default =
registry.get_type_data::<bevy_reflect::std_traits::ReflectDefault>(type_id)?;
let mut component = reflect_default.default();
component.apply(source_component_reflect.as_partial_reflect());
component
};
ctx.write_target_component_reflect(component);

let component_info = world
.components()
.get_info(component_id)
.expect("Component must be registered");
let Some(type_id) = component_info.type_id() else {
return;
};
let Some(reflect_component) =
registry.get_type_data::<crate::reflect::ReflectComponent>(type_id)
else {
return;
};
let source_component = reflect_component
.reflect(world.get_entity(source).expect("Source entity must exist"))
.expect("Source entity must have reflected component")
.clone_value();
let mut target = world
.get_entity_mut(target)
.expect("Target entity must exist");
reflect_component.apply_or_insert(&mut target, &*source_component, &registry);
Some(())
}

fn clone_slow(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How much slower is this? I'm sure it is slower due to the extra clone_value, but it basically works the same as clone_fast after that: it attempts ReflectFromReflect, then ReflectDefault, then ReflectFromWorld, and then panics otherwise.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How much slower is this

As slow as the results in the main, avg column of the benchmark table.

Yeah, it seems clone_slow is pretty much reserved for ReflectFromWorld now and can panic. Maybe I should try to do it manually instead of using ReflectComponent then.

component_id: ComponentId,
source: Entity,
target: Entity,
registry: &crate::reflect::AppTypeRegistry,
world: &mut World,
) -> Option<()> {
let registry = registry.read();

let component_info = world.components().get_info(component_id)?;
let type_id = component_info.type_id()?;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems like some of this is duplicated from clone_fast. Maybe we could merge the two to avoid double lookups and unnecessary retries (i.e. if component_info.type_id()? fails in clone_fast, there's no need to try again in clone_slow).

let reflect_component =
registry.get_type_data::<crate::reflect::ReflectComponent>(type_id)?;
let source_component = reflect_component
.reflect(world.get_entity(source).ok()?)?
.clone_value();
let mut target = world.get_entity_mut(target).ok()?;
reflect_component.apply_or_insert(&mut target, &*source_component, &registry);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note: This can panic. If that's undesirable, it may be worth creating a non-panicking version.

Some(())
}

// Try to clone fast first, if it doesn't work use the slow path
if clone_fast(ctx).is_some() {
return;
};

let component_id = ctx.component_id();
let source = ctx.source();
let target = ctx.target();
world.commands().queue(move |world: &mut World| {
world.resource_scope(|world, registry| {
clone_slow(component_id, source, target, &registry, world);
});
});
}

/// Noop implementation of component clone handler function.
///
/// See [`ComponentCloneHandlers`] for more details.
pub fn component_clone_ignore(_world: &mut DeferredWorld, _entity_cloner: &EntityCloner) {}
pub fn component_clone_ignore(_world: &mut DeferredWorld, _ctx: &mut ComponentCloneCtx) {}

/// Wrapper for components clone specialization using autoderef.
#[doc(hidden)]
Expand Down Expand Up @@ -2135,6 +2174,6 @@ pub trait ComponentCloneViaClone {
}
impl<C: Clone + Component> ComponentCloneViaClone for &ComponentCloneSpecializationWrapper<C> {
fn get_component_clone_handler(&self) -> ComponentCloneHandler {
ComponentCloneHandler::Custom(component_clone_via_clone::<C>)
ComponentCloneHandler::clone_handler::<C>()
}
}
Loading
Loading