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

Spawning child entities is extremely slow #17301

Closed
aevyrie opened this issue Jan 11, 2025 · 6 comments · Fixed by #17398
Closed

Spawning child entities is extremely slow #17301

aevyrie opened this issue Jan 11, 2025 · 6 comments · Fixed by #17398
Labels
A-ECS Entities, components, systems, and events A-Hierarchy Parent-child entity hierarchies C-Performance A change motivated by improving speed, memory usage or compile times D-Modest A "normal" level of difficulty; suitable for simple features or challenging fixes S-Needs-Design This issue requires design work to think about how it would best be accomplished

Comments

@aevyrie
Copy link
Member

aevyrie commented Jan 11, 2025

Bevy version

0.15, has existed for as long as constructing Parent and Children has been locked down.

What you did

Attempt to optimize spawning large batches of children in a custom command for big_space.

What went wrong

Profiling shows that applying system commands dominates system time. Spawning 320k entities took 31s.

The problem appears to be that even when spawning a brand new entity, bevy is doing reparenting checks for every child when its Parent component is added, and again for every child when the children are added to the parent entity's Children component. These checks are completely unnecessary - the entities are new and cannot yet exist in the hierarchy.

By doing some transmute shenanigans, I was able to see what a niave solution would look like if I could simply construct the Parent and Children entities myself. The performance improvement was stark: what previously took 31s now took 83ms.

I was also able to work around this without unsafe by using reflection to construct a Parent and Children, bypassing the private constructors.

@aevyrie aevyrie added A-ECS Entities, components, systems, and events A-Hierarchy Parent-child entity hierarchies C-Performance A change motivated by improving speed, memory usage or compile times labels Jan 11, 2025
@BenjaminBrienen BenjaminBrienen added S-Needs-Design This issue requires design work to think about how it would best be accomplished D-Modest A "normal" level of difficulty; suitable for simple features or challenging fixes labels Jan 11, 2025
@aevyrie
Copy link
Member Author

aevyrie commented Jan 13, 2025

I had a lot of trouble replicating this performance issue on main, and it turned out to be that it only happens in a specific, non-obvious case. When an entity has no other children, and you spawn a bunch of children, this is fast. If that entity already had other children, you start to run into accidentally quadratic scaling.

The fix is to recognize that when the children we are adding to an entity have just been spawned, we can skip all of these checks. There is no way for the hierarchy to become malformed, because these entities are brand new. In the linked benchmark, the 1 million entity case was projected to take 7.5 hours, but completes in 111ms with fixes.

@aevyrie
Copy link
Member Author

aevyrie commented Jan 13, 2025

Another note: the linked PR doesn't fully fix this issue, it only resolves one set of cases that are easy to optimize for (and common). In the case where an entity is not new, adding this as a child still scales exponentially after the change. This is because to check if an entity already exists, bevy currently does a linear scan of a vector, which scales linearly with the length of the vector. Instead, we should probably be using a hashmap once the vector is large enough to spill onto the heap. This would make these checks constant time.

@cart
Copy link
Member

cart commented Jan 13, 2025

As a heads up, if we land something in the "non-fragmenting relations" category (which I have an incoming PR for), that would fully resolve this. I'm not opposed to landing your optimizations first though, in the interest of ensuring some form of improvement lands in the next release.

@aevyrie
Copy link
Member Author

aevyrie commented Jan 13, 2025

"non-fragmenting relations"

What does this mean? The relations live in a central store or something? It's still possible for these optimizations to be missed, unless I'm missing something.

Edit: #16920

After skimming this, it still appears possible to hit the same performance issue, it completely depends on the implementation. I don't see how the design would prevent this from happening.

@cart
Copy link
Member

cart commented Jan 14, 2025

After skimming this, it still appears possible to hit the same performance issue, it completely depends on the implementation. I don't see how the design would prevent this from happening.

You're right it isn't inherent to the space. My particular implementation does not suffer from the problem. The Parent(Entity) component is the "single source of truth" and the Children component is just a reflection of the children's Parent state. Component lifecycle hooks on Parent are used to update the Children component. Because the only way to update children is through the Parent component lifecycle, there is no way to introduce duplicates, so we don't need to check for dupes on spawn. Children just blissfully insert into their parent's Children collection.

Normal per-child removal does still require a scan to find the entity. But with the new on_despawn hook I've added we can skip the scan per-entity on despawn by clearing the children list early.

@aevyrie
Copy link
Member Author

aevyrie commented Jan 15, 2025

Thanks for the details, that all makes sense.

github-merge-queue bot pushed a commit that referenced this issue Jan 18, 2025
This adds support for one-to-many non-fragmenting relationships (with
planned paths for fragmenting and non-fragmenting many-to-many
relationships). "Non-fragmenting" means that entities with the same
relationship type, but different relationship targets, are not forced
into separate tables (which would cause "table fragmentation").

Functionally, this fills a similar niche as the current Parent/Children
system. The biggest differences are:

1. Relationships have simpler internals and significantly improved
performance and UX. Commands and specialized APIs are no longer
necessary to keep everything in sync. Just spawn entities with the
relationship components you want and everything "just works".
2. Relationships are generalized. Bevy can provide additional built in
relationships, and users can define their own.

**REQUEST TO REVIEWERS**: _please don't leave top level comments and
instead comment on specific lines of code. That way we can take
advantage of threaded discussions. Also dont leave comments simply
pointing out CI failures as I can read those just fine._

## Built on top of what we have

Relationships are implemented on top of the Bevy ECS features we already
have: components, immutability, and hooks. This makes them immediately
compatible with all of our existing (and future) APIs for querying,
spawning, removing, scenes, reflection, etc. The fewer specialized APIs
we need to build, maintain, and teach, the better.

## Why focus on one-to-many non-fragmenting first?

1. This allows us to improve Parent/Children relationships immediately,
in a way that is reasonably uncontroversial. Switching our hierarchy to
fragmenting relationships would have significant performance
implications. ~~Flecs is heavily considering a switch to non-fragmenting
relations after careful considerations of the performance tradeoffs.~~
_(Correction from @SanderMertens: Flecs is implementing non-fragmenting
storage specialized for asset hierarchies, where asset hierarchies are
many instances of small trees that have a well defined structure)_
2. Adding generalized one-to-many relationships is currently a priority
for the [Next Generation Scene / UI
effort](#14437).
Specifically, we're interested in building reactions and observers on
top.

## The changes

This PR does the following:

1. Adds a generic one-to-many Relationship system
3. Ports the existing Parent/Children system to Relationships, which now
lives in `bevy_ecs::hierarchy`. The old `bevy_hierarchy` crate has been
removed.
4. Adds on_despawn component hooks
5. Relationships can opt-in to "despawn descendants" behavior, meaning
that the entire relationship hierarchy is despawned when
`entity.despawn()` is called. The built in Parent/Children hierarchies
enable this behavior, and `entity.despawn_recursive()` has been removed.
6. `world.spawn` now applies commands after spawning. This ensures that
relationship bookkeeping happens immediately and removes the need to
manually flush. This is in line with the equivalent behaviors recently
added to the other APIs (ex: insert).
7. Removes the ValidParentCheckPlugin (system-driven / poll based) in
favor of a `validate_parent_has_component` hook.

## Using Relationships

The `Relationship` trait looks like this:

```rust
pub trait Relationship: Component + Sized {
    type RelationshipSources: RelationshipSources<Relationship = Self>;
    fn get(&self) -> Entity;
    fn from(entity: Entity) -> Self;
}
```

A relationship is a component that:

1. Is a simple wrapper over a "target" Entity.
2. Has a corresponding `RelationshipSources` component, which is a
simple wrapper over a collection of entities. Every "target entity"
targeted by a "source entity" with a `Relationship` has a
`RelationshipSources` component, which contains every "source entity"
that targets it.

For example, the `Parent` component (as it currently exists in Bevy) is
the `Relationship` component and the entity containing the Parent is the
"source entity". The entity _inside_ the `Parent(Entity)` component is
the "target entity". And that target entity has a `Children` component
(which implements `RelationshipSources`).

In practice, the Parent/Children relationship looks like this:

```rust
#[derive(Relationship)]
#[relationship(relationship_sources = Children)]
pub struct Parent(pub Entity);

#[derive(RelationshipSources)]
#[relationship_sources(relationship = Parent)]
pub struct Children(Vec<Entity>);
```

The Relationship and RelationshipSources derives automatically implement
Component with the relevant configuration (namely, the hooks necessary
to keep everything in sync).

The most direct way to add relationships is to spawn entities with
relationship components:

```rust
let a = world.spawn_empty().id();
let b = world.spawn(Parent(a)).id();

assert_eq!(world.entity(a).get::<Children>().unwrap(), &[b]);
```

There are also convenience APIs for spawning more than one entity with
the same relationship:

```rust
world.spawn_empty().with_related::<Children>(|s| {
    s.spawn_empty();
    s.spawn_empty();
})
```

The existing `with_children` API is now a simpler wrapper over
`with_related`. This makes this change largely non-breaking for existing
spawn patterns.

```rust
world.spawn_empty().with_children(|s| {
    s.spawn_empty();
    s.spawn_empty();
})
```

There are also other relationship APIs, such as `add_related` and
`despawn_related`.

## Automatic recursive despawn via the new on_despawn hook

`RelationshipSources` can opt-in to "despawn descendants" behavior,
which will despawn all related entities in the relationship hierarchy:

```rust
#[derive(RelationshipSources)]
#[relationship_sources(relationship = Parent, despawn_descendants)]
pub struct Children(Vec<Entity>);
```

This means that `entity.despawn_recursive()` is no longer required.
Instead, just use `entity.despawn()` and the relevant related entities
will also be despawned.

To despawn an entity _without_ despawning its parent/child descendants,
you should remove the `Children` component first, which will also remove
the related `Parent` components:

```rust
entity
    .remove::<Children>()
    .despawn()
```

This builds on the on_despawn hook introduced in this PR, which is fired
when an entity is despawned (before other hooks).

## Relationships are the source of truth

`Relationship` is the _single_ source of truth component.
`RelationshipSources` is merely a reflection of what all the
`Relationship` components say. By embracing this, we are able to
significantly improve the performance of the system as a whole. We can
rely on component lifecycles to protect us against duplicates, rather
than needing to scan at runtime to ensure entities don't already exist
(which results in quadratic runtime). A single source of truth gives us
constant-time inserts. This does mean that we cannot directly spawn
populated `Children` components (or directly add or remove entities from
those components). I personally think this is a worthwhile tradeoff,
both because it makes the performance much better _and_ because it means
theres exactly one way to do things (which is a philosophy we try to
employ for Bevy APIs).

As an aside: treating both sides of the relationship as "equivalent
source of truth relations" does enable building simple and flexible
many-to-many relationships. But this introduces an _inherent_ need to
scan (or hash) to protect against duplicates.
[`evergreen_relations`](https://github.com/EvergreenNest/evergreen_relations)
has a very nice implementation of the "symmetrical many-to-many"
approach. Unfortunately I think the performance issues inherent to that
approach make it a poor choice for Bevy's default relationship system.

## Followup Work

* Discuss renaming `Parent` to `ChildOf`. I refrained from doing that in
this PR to keep the diff reasonable, but I'm personally biased toward
this change (and using that naming pattern generally for relationships).
* [Improved spawning
ergonomics](#16920)
* Consider adding relationship observers/triggers for "relationship
targets" whenever a source is added or removed. This would replace the
current "hierarchy events" system, which is unused upstream but may have
existing users downstream. I think triggers are the better fit for this
than a buffered event queue, and would prefer not to add that back.
* Fragmenting relations: My current idea hinges on the introduction of
"value components" (aka: components whose type _and_ value determines
their ComponentId, via something like Hashing / PartialEq). By labeling
a Relationship component such as `ChildOf(Entity)` as a "value
component", `ChildOf(e1)` and `ChildOf(e2)` would be considered
"different components". This makes the transition between fragmenting
and non-fragmenting a single flag, and everything else continues to work
as expected.
* Many-to-many support
* Non-fragmenting: We can expand Relationship to be a list of entities
instead of a single entity. I have largely already written the code for
this.
* Fragmenting: With the "value component" impl mentioned above, we get
many-to-many support "for free", as it would allow inserting multiple
copies of a Relationship component with different target entities.

Fixes #3742 (If this PR is merged, I think we should open more targeted
followup issues for the work above, with a fresh tracking issue free of
the large amount of less-directed historical context)
Fixes #17301
Fixes #12235 
Fixes #15299
Fixes #15308 

## Migration Guide

* Replace `ChildBuilder` with `ChildSpawnerCommands`.
* Replace calls to `.set_parent(parent_id)` with
`.insert(Parent(parent_id))`.
* Replace calls to `.replace_children()` with `.remove::<Children>()`
followed by `.add_children()`. Note that you'll need to manually despawn
any children that are not carried over.
* Replace calls to `.despawn_recursive()` with `.despawn()`.
* Replace calls to `.despawn_descendants()` with
`.despawn_related::<Children>()`.
* If you have any calls to `.despawn()` which depend on the children
being preserved, you'll need to remove the `Children` component first.

---------

Co-authored-by: Alice Cecile <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-ECS Entities, components, systems, and events A-Hierarchy Parent-child entity hierarchies C-Performance A change motivated by improving speed, memory usage or compile times D-Modest A "normal" level of difficulty; suitable for simple features or challenging fixes S-Needs-Design This issue requires design work to think about how it would best be accomplished
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants