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.

Sign up for GitHub

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

Jump to bottom

Add Commands::insert_batch for faster component modification #2693

Closed
alice-i-cecile opened this issue Aug 19, 2021 · 2 comments · Fixed by #15702
Closed

Add Commands::insert_batch for faster component modification #2693

alice-i-cecile opened this issue Aug 19, 2021 · 2 comments · Fixed by #15702
Labels
A-ECS Entities, components, systems, and events C-Performance A change motivated by improving speed, memory usage or compile times D-Straightforward Simple bug fixes and API improvements, docs, test and examples S-Ready-For-Implementation This issue is ready for an implementation PR. Go for it! X-Uncontroversial This work is generally agreed upon

Comments

@alice-i-cecile
Copy link
Member

alice-i-cecile commented Aug 19, 2021 •
edited
Loading

What problem does this solve or what need does it fill?

Inserting components one at a time is slow.

What solution would you like?

Add Commands::insert_batch and remove_batch to modify components en-masse in a more efficient way.

As discussed in #3227 (comment), this should be a safe, typed equivalent to EntityWorldMut::insert_by_ids that's exposed to commands.

I think that a builder API that iteratively modifies the underlying Command struct is probably the correct design.

What alternative(s) have you considered?

We could use insert_or_spawn_batch for the insertion case, but that does not fail in the desired ways at all.

Additional context

Follows up on the work from #2673, which added insert_or_spawn_batch. Ideally we can use the same strategy for this work, and perform similar benchmarks.
@alice-i-cecile alice-i-cecile added A-ECS Entities, components, systems, and events C-Performance A change motivated by improving speed, memory usage or compile times S-Ready-For-Implementation This issue is ready for an implementation PR. Go for it! labels Aug 19, 2021
@alice-i-cecile
Copy link
Member Author

We should also include despawn_batch as part of this PR (or in an immediate follow-up PR) for performance reasons.

@cart
Copy link
Member

cart commented Sep 17, 2021

I don't think despawn_batch would meaningfully benefit from batching. Removing arbitrary entities is random-access (even if you somehow managed to sort by archetype, the internal swap remove would still be random). And the recent Commands rework ensures that we reuse allocations. In fact, despawn(a), despawn(b), ... would be faster than despawn_batch(![a, b, ...]) because we could reuse the command allocations across frames, whereas the Vec would need to be allocated each frame.

@alice-i-cecile alice-i-cecile added D-Straightforward Simple bug fixes and API improvements, docs, test and examples X-Uncontroversial This work is generally agreed upon labels Sep 26, 2024
@alice-i-cecile alice-i-cecile mentioned this issue Sep 26, 2024
Closed
@alice-i-cecile alice-i-cecile changed the title Add Commands::insert_batch and remove_batch for faster component modification Add Commands::insert_batch for faster component modification Sep 26, 2024
@JaySpruce JaySpruce mentioned this issue Oct 7, 2024
Merged
github-merge-queue bot pushed a commit that referenced this issue Oct 13, 2024
@JaySpruce
Add insert_batch and variations (#15702)
3d6b248 
# Objective

`insert_or_spawn_batch` exists, but a version for just inserting doesn't
- Closes #2693 
- Closes #8384 
- Adopts/supersedes #8600 

## Solution

Add `insert_batch`, along with the most common `insert` variations:
- `World::insert_batch`
- `World::insert_batch_if_new`
- `World::try_insert_batch`
- `World::try_insert_batch_if_new`
- `Commands::insert_batch`
- `Commands::insert_batch_if_new`
- `Commands::try_insert_batch`
- `Commands::try_insert_batch_if_new`

## Testing

Added tests, and added a benchmark for `insert_batch`.
Performance is slightly better than `insert_or_spawn_batch` when only
inserting:


![Code_HPnUN0QeWe](https://github.com/user-attachments/assets/53091e4f-6518-43f4-a63f-ae57d5470c66)

<details>
<summary>old benchmark</summary>

This was before reworking it to remove the `UnsafeWorldCell`:


![Code_QhXJb8sjlJ](https://github.com/user-attachments/assets/1061e2a7-a521-48e1-a799-1b6b8d1c0b93)
</details>

---

## Showcase

Usage is the same as `insert_or_spawn_batch`:
```
use bevy_ecs::{entity::Entity, world::World, component::Component};
#[derive(Component)]
struct A(&'static str);
#[derive(Component, PartialEq, Debug)]
struct B(f32);

let mut world = World::new();
let entity_a = world.spawn_empty().id();
let entity_b = world.spawn_empty().id();
world.insert_batch([
    (entity_a, (A("a"), B(0.0))),
    (entity_b, (A("b"), B(1.0))),
]);

assert_eq!(world.get::<B>(entity_a), Some(&B(0.0)));

```
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
A-ECS Entities, components, systems, and events C-Performance A change motivated by improving speed, memory usage or compile times D-Straightforward Simple bug fixes and API improvements, docs, test and examples S-Ready-For-Implementation This issue is ready for an implementation PR. Go for it! X-Uncontroversial This work is generally agreed upon
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Add insert_batch and variations JaySpruce/bevy
2 participants
@cart @alice-i-cecile