[Merged by Bors] - Spawn now takes a Bundle by cart · Pull Request #6054 · bevyengine/bevy

cart · 2022-09-21T23:41:38Z

Objective

Now that we can consolidate Bundles and Components under a single insert (thanks to #2975 and #6039), almost 100% of world spawns now look like world.spawn().insert((Some, Tuple, Here)). Spawning an entity without any components is an extremely uncommon pattern, so it makes sense to give spawn the "first class" ergonomic api. This consolidated api should be made consistent across all spawn apis (such as World and Commands).

Solution

All spawn apis (World::spawn, Commands:;spawn, ChildBuilder::spawn, and WorldChildBuilder::spawn) now accept a bundle as input:

// before:
commands
  .spawn()
  .insert((A, B, C));
world
  .spawn()
  .insert((A, B, C);

// after
commands.spawn((A, B, C));
world.spawn((A, B, C));

All existing instances of spawn_bundle have been deprecated in favor of the new spawn api. A new spawn_empty has been added, replacing the old spawn api.

By allowing world.spawn(some_bundle) to replace world.spawn().insert(some_bundle), this opened the door to removing the initial entity allocation in the "empty" archetype / table done in spawn() (and subsequent move to the actual archetype in .insert(some_bundle)).

This improves spawn performance by over 10%:

To take this measurement, I added a new world_spawn benchmark.

Unfortunately, optimizing Commands::spawn is slightly less trivial, as Commands expose the Entity id of spawned entities prior to actually spawning. Doing the optimization would (naively) require assurances that the spawn(some_bundle) command is applied before all other commands involving the entity (which would not necessarily be true, if memory serves). Optimizing Commands::spawn this way does feel possible, but it will require careful thought (and maybe some additional checks), which deserves its own PR. For now, it has the same performance characteristics of the current Commands::spawn_bundle on main.

Note that 99% of this PR is simple renames and refactors. The only code that needs careful scrutiny is the new World::spawn() impl, which is relatively straightforward, but it has some new unsafe code (which re-uses battle tested BundlerSpawner code path).

Changelog

All spawn apis (World::spawn, Commands:;spawn, ChildBuilder::spawn, and WorldChildBuilder::spawn) now accept a bundle as input
All instances of spawn_bundle have been deprecated in favor of the new spawn api
World and Commands now have spawn_empty(), which is equivalent to the old spawn() behavior.

Migration Guide

// Old (0.8):
commands
  .spawn()
  .insert_bundle((A, B, C));
// New (0.9)
commands.spawn((A, B, C));

// Old (0.8):
commands.spawn_bundle((A, B, C));
// New (0.9)
commands.spawn((A, B, C));

// Old (0.8):
let entity = commands.spawn().id();
// New (0.9)
let entity = commands.spawn_empty().id();

// Old (0.8)
let entity = world.spawn().id();
// New (0.9)
let entity = world.spawn_empty();

alice-i-cecile · 2022-09-22T01:24:09Z

crates/bevy_ecs/src/world/mod.rs

            AllocAtWithoutReplacement::DidNotExist => {
                // SAFETY: entity was just allocated
-                Some(unsafe { self.spawn_at_internal(entity) })
+                Some(unsafe { self.spawn_at_empty_internal(entity) })


Note to reviewers: this is where the interesting bits start.

alice-i-cecile

I've gone through these changes, and taken a close look at the changes in bevy_ecs/world/mod.rs. I'm on board with the direction, and convinced by the impl.

tim-blackbird

Just some doc nits.

crates/bevy_ecs/src/system/commands/mod.rs

crates/bevy_ecs/src/world/mod.rs

Co-authored-by: ira <[email protected]>

cart · 2022-09-22T21:06:50Z

bors r+

# Objective Now that we can consolidate Bundles and Components under a single insert (thanks to #2975 and #6039), almost 100% of world spawns now look like `world.spawn().insert((Some, Tuple, Here))`. Spawning an entity without any components is an extremely uncommon pattern, so it makes sense to give spawn the "first class" ergonomic api. This consolidated api should be made consistent across all spawn apis (such as World and Commands). ## Solution All `spawn` apis (`World::spawn`, `Commands:;spawn`, `ChildBuilder::spawn`, and `WorldChildBuilder::spawn`) now accept a bundle as input: ```rust // before: commands .spawn() .insert((A, B, C)); world .spawn() .insert((A, B, C); // after commands.spawn((A, B, C)); world.spawn((A, B, C)); ``` All existing instances of `spawn_bundle` have been deprecated in favor of the new `spawn` api. A new `spawn_empty` has been added, replacing the old `spawn` api. By allowing `world.spawn(some_bundle)` to replace `world.spawn().insert(some_bundle)`, this opened the door to removing the initial entity allocation in the "empty" archetype / table done in `spawn()` (and subsequent move to the actual archetype in `.insert(some_bundle)`). This improves spawn performance by over 10%: ![image](https://user-images.githubusercontent.com/2694663/191627587-4ab2f949-4ccd-4231-80eb-80dd4d9ad6b9.png) To take this measurement, I added a new `world_spawn` benchmark. Unfortunately, optimizing `Commands::spawn` is slightly less trivial, as Commands expose the Entity id of spawned entities prior to actually spawning. Doing the optimization would (naively) require assurances that the `spawn(some_bundle)` command is applied before all other commands involving the entity (which would not necessarily be true, if memory serves). Optimizing `Commands::spawn` this way does feel possible, but it will require careful thought (and maybe some additional checks), which deserves its own PR. For now, it has the same performance characteristics of the current `Commands::spawn_bundle` on main. **Note that 99% of this PR is simple renames and refactors. The only code that needs careful scrutiny is the new `World::spawn()` impl, which is relatively straightforward, but it has some new unsafe code (which re-uses battle tested BundlerSpawner code path).** --- ## Changelog - All `spawn` apis (`World::spawn`, `Commands:;spawn`, `ChildBuilder::spawn`, and `WorldChildBuilder::spawn`) now accept a bundle as input - All instances of `spawn_bundle` have been deprecated in favor of the new `spawn` api - World and Commands now have `spawn_empty()`, which is equivalent to the old `spawn()` behavior. ## Migration Guide ```rust // Old (0.8): commands .spawn() .insert_bundle((A, B, C)); // New (0.9) commands.spawn((A, B, C)); // Old (0.8): commands.spawn_bundle((A, B, C)); // New (0.9) commands.spawn((A, B, C)); // Old (0.8): let entity = commands.spawn().id(); // New (0.9) let entity = commands.spawn_empty().id(); // Old (0.8) let entity = world.spawn().id(); // New (0.9) let entity = world.spawn_empty(); ```

bors · 2022-09-22T21:11:23Z

Build failed:

build (ubuntu-latest)

cart · 2022-09-23T19:49:30Z

Haha the newly merged ambiguity tests need to updated

cart · 2022-09-23T19:55:38Z

bors r+

# Objective Now that we can consolidate Bundles and Components under a single insert (thanks to #2975 and #6039), almost 100% of world spawns now look like `world.spawn().insert((Some, Tuple, Here))`. Spawning an entity without any components is an extremely uncommon pattern, so it makes sense to give spawn the "first class" ergonomic api. This consolidated api should be made consistent across all spawn apis (such as World and Commands). ## Solution All `spawn` apis (`World::spawn`, `Commands:;spawn`, `ChildBuilder::spawn`, and `WorldChildBuilder::spawn`) now accept a bundle as input: ```rust // before: commands .spawn() .insert((A, B, C)); world .spawn() .insert((A, B, C); // after commands.spawn((A, B, C)); world.spawn((A, B, C)); ``` All existing instances of `spawn_bundle` have been deprecated in favor of the new `spawn` api. A new `spawn_empty` has been added, replacing the old `spawn` api. By allowing `world.spawn(some_bundle)` to replace `world.spawn().insert(some_bundle)`, this opened the door to removing the initial entity allocation in the "empty" archetype / table done in `spawn()` (and subsequent move to the actual archetype in `.insert(some_bundle)`). This improves spawn performance by over 10%: ![image](https://user-images.githubusercontent.com/2694663/191627587-4ab2f949-4ccd-4231-80eb-80dd4d9ad6b9.png) To take this measurement, I added a new `world_spawn` benchmark. Unfortunately, optimizing `Commands::spawn` is slightly less trivial, as Commands expose the Entity id of spawned entities prior to actually spawning. Doing the optimization would (naively) require assurances that the `spawn(some_bundle)` command is applied before all other commands involving the entity (which would not necessarily be true, if memory serves). Optimizing `Commands::spawn` this way does feel possible, but it will require careful thought (and maybe some additional checks), which deserves its own PR. For now, it has the same performance characteristics of the current `Commands::spawn_bundle` on main. **Note that 99% of this PR is simple renames and refactors. The only code that needs careful scrutiny is the new `World::spawn()` impl, which is relatively straightforward, but it has some new unsafe code (which re-uses battle tested BundlerSpawner code path).** --- ## Changelog - All `spawn` apis (`World::spawn`, `Commands:;spawn`, `ChildBuilder::spawn`, and `WorldChildBuilder::spawn`) now accept a bundle as input - All instances of `spawn_bundle` have been deprecated in favor of the new `spawn` api - World and Commands now have `spawn_empty()`, which is equivalent to the old `spawn()` behavior. ## Migration Guide ```rust // Old (0.8): commands .spawn() .insert_bundle((A, B, C)); // New (0.9) commands.spawn((A, B, C)); // Old (0.8): commands.spawn_bundle((A, B, C)); // New (0.9) commands.spawn((A, B, C)); // Old (0.8): let entity = commands.spawn().id(); // New (0.9) let entity = commands.spawn_empty().id(); // Old (0.8) let entity = world.spawn().id(); // New (0.9) let entity = world.spawn_empty(); ```

bors · 2022-09-23T20:15:45Z

Pull request successfully merged into main.

Build succeeded:

# Objective Now that we can consolidate Bundles and Components under a single insert (thanks to bevyengine#2975 and bevyengine#6039), almost 100% of world spawns now look like `world.spawn().insert((Some, Tuple, Here))`. Spawning an entity without any components is an extremely uncommon pattern, so it makes sense to give spawn the "first class" ergonomic api. This consolidated api should be made consistent across all spawn apis (such as World and Commands). ## Solution All `spawn` apis (`World::spawn`, `Commands:;spawn`, `ChildBuilder::spawn`, and `WorldChildBuilder::spawn`) now accept a bundle as input: ```rust // before: commands .spawn() .insert((A, B, C)); world .spawn() .insert((A, B, C); // after commands.spawn((A, B, C)); world.spawn((A, B, C)); ``` All existing instances of `spawn_bundle` have been deprecated in favor of the new `spawn` api. A new `spawn_empty` has been added, replacing the old `spawn` api. By allowing `world.spawn(some_bundle)` to replace `world.spawn().insert(some_bundle)`, this opened the door to removing the initial entity allocation in the "empty" archetype / table done in `spawn()` (and subsequent move to the actual archetype in `.insert(some_bundle)`). This improves spawn performance by over 10%: ![image](https://user-images.githubusercontent.com/2694663/191627587-4ab2f949-4ccd-4231-80eb-80dd4d9ad6b9.png) To take this measurement, I added a new `world_spawn` benchmark. Unfortunately, optimizing `Commands::spawn` is slightly less trivial, as Commands expose the Entity id of spawned entities prior to actually spawning. Doing the optimization would (naively) require assurances that the `spawn(some_bundle)` command is applied before all other commands involving the entity (which would not necessarily be true, if memory serves). Optimizing `Commands::spawn` this way does feel possible, but it will require careful thought (and maybe some additional checks), which deserves its own PR. For now, it has the same performance characteristics of the current `Commands::spawn_bundle` on main. **Note that 99% of this PR is simple renames and refactors. The only code that needs careful scrutiny is the new `World::spawn()` impl, which is relatively straightforward, but it has some new unsafe code (which re-uses battle tested BundlerSpawner code path).** --- ## Changelog - All `spawn` apis (`World::spawn`, `Commands:;spawn`, `ChildBuilder::spawn`, and `WorldChildBuilder::spawn`) now accept a bundle as input - All instances of `spawn_bundle` have been deprecated in favor of the new `spawn` api - World and Commands now have `spawn_empty()`, which is equivalent to the old `spawn()` behavior. ## Migration Guide ```rust // Old (0.8): commands .spawn() .insert_bundle((A, B, C)); // New (0.9) commands.spawn((A, B, C)); // Old (0.8): commands.spawn_bundle((A, B, C)); // New (0.9) commands.spawn((A, B, C)); // Old (0.8): let entity = commands.spawn().id(); // New (0.9) let entity = commands.spawn_empty().id(); // Old (0.8) let entity = world.spawn().id(); // New (0.9) let entity = world.spawn_empty(); ```

cart added 6 commits September 21, 2022 16:16

world.spawn() now takes a bundle

33b779e

Port everything

89a4c23

replace spawn_batch instances

3ba07ee

improve docs

934cfa8

finish port

9a8d382

add spawn bench and update benches

5ae0b0e

cart added this to the Bevy 0.9 milestone Sep 21, 2022

Fix some straggling cases of .spawn()

50c99ba

alice-i-cecile requested review from DJMcNab and alice-i-cecile September 22, 2022 00:04

Fix doctest

a78b6be

alice-i-cecile reviewed Sep 22, 2022

View reviewed changes

alice-i-cecile approved these changes Sep 22, 2022

View reviewed changes

tim-blackbird approved these changes Sep 22, 2022

View reviewed changes

alice-i-cecile added the S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it label Sep 22, 2022

cart and others added 3 commits September 22, 2022 14:03

Update crates/bevy_ecs/src/system/commands/mod.rs

a866d58

Co-authored-by: ira <[email protected]>

Update crates/bevy_ecs/src/system/commands/mod.rs

f851b89

Co-authored-by: ira <[email protected]>

Address comments

d3be558

cart added 2 commits September 23, 2022 12:49

Merge remote-tracking branch 'upstream/main' into spawn-bundle

a4da3dc

update ambiguity tests

bb831c6

bors bot changed the title ~~Spawn now takes a Bundle~~ [Merged by Bors] - Spawn now takes a Bundle Sep 23, 2022

bors bot closed this Sep 23, 2022

alice-i-cecile mentioned this pull request Oct 10, 2022

World.spawn(()) panics with index out of bounds #6226

Closed

james7132 mentioned this pull request Nov 12, 2022

Consider not allocating in the empty archetype on world.spawn() #1556

Closed

james7132 mentioned this pull request Dec 4, 2022

Enforce type-uniqueness of Bundles #2387

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[Merged by Bors] - Spawn now takes a Bundle#6054

[Merged by Bors] - Spawn now takes a Bundle#6054
cart wants to merge 13 commits intobevyengine:mainfrom
cart:spawn-bundle

cart commented Sep 21, 2022 •

edited

Loading

Uh oh!

alice-i-cecile Sep 22, 2022 •

edited

Loading

Uh oh!

alice-i-cecile left a comment

Uh oh!

tim-blackbird left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

cart commented Sep 22, 2022

Uh oh!

bors bot commented Sep 22, 2022

Uh oh!

cart commented Sep 23, 2022

Uh oh!

cart commented Sep 23, 2022

Uh oh!

bors bot commented Sep 23, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

cart commented Sep 21, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Objective

Solution

Changelog

Migration Guide

Uh oh!

alice-i-cecile Sep 22, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

alice-i-cecile left a comment

Choose a reason for hiding this comment

Uh oh!

tim-blackbird left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

cart commented Sep 22, 2022

Uh oh!

bors bot commented Sep 22, 2022

Uh oh!

cart commented Sep 23, 2022

Uh oh!

cart commented Sep 23, 2022

Uh oh!

bors bot commented Sep 23, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

cart commented Sep 21, 2022 •

edited

Loading

alice-i-cecile Sep 22, 2022 •

edited

Loading