 dfea88c64d
			
		
	
	
		dfea88c64d
		
	
	
	
	
		
			
			# Objective Fixes #3184. Fixes #6640. Fixes #4798. Using `Query::par_for_each(_mut)` currently requires a `batch_size` parameter, which affects how it chunks up large archetypes and tables into smaller chunks to run in parallel. Tuning this value is difficult, as the performance characteristics entirely depends on the state of the `World` it's being run on. Typically, users will just use a flat constant and just tune it by hand until it performs well in some benchmarks. However, this is both error prone and risks overfitting the tuning on that benchmark. This PR proposes a naive automatic batch-size computation based on the current state of the `World`. ## Background `Query::par_for_each(_mut)` schedules a new Task for every archetype or table that it matches. Archetypes/tables larger than the batch size are chunked into smaller tasks. Assuming every entity matched by the query has an identical workload, this makes the worst case scenario involve using a batch size equal to the size of the largest matched archetype or table. Conversely, a batch size of `max {archetype, table} size / thread count * COUNT_PER_THREAD` is likely the sweetspot where the overhead of scheduling tasks is minimized, at least not without grouping small archetypes/tables together. There is also likely a strict minimum batch size below which the overhead of scheduling these tasks is heavier than running the entire thing single-threaded. ## Solution - [x] Remove the `batch_size` from `Query(State)::par_for_each` and friends. - [x] Add a check to compute `batch_size = max {archeytpe/table} size / thread count * COUNT_PER_THREAD` - [x] ~~Panic if thread count is 0.~~ Defer to `for_each` if the thread count is 1 or less. - [x] Early return if there is no matched table/archetype. - [x] Add override option for users have queries that strongly violate the initial assumption that all iterated entities have an equal workload. --- ## Changelog Changed: `Query::par_for_each(_mut)` has been changed to `Query::par_iter(_mut)` and will now automatically try to produce a batch size for callers based on the current `World` state. ## Migration Guide The `batch_size` parameter for `Query(State)::par_for_each(_mut)` has been removed. These calls will automatically compute a batch size for you. Remove these parameters from all calls to these functions. Before: ```rust fn parallel_system(query: Query<&MyComponent>) { query.par_for_each(32, |comp| { ... }); } ``` After: ```rust fn parallel_system(query: Query<&MyComponent>) { query.par_iter().for_each(|comp| { ... }); } ``` Co-authored-by: Arnav Choubey <56453634+x-52@users.noreply.github.com> Co-authored-by: Robert Swain <robert.swain@gmail.com> Co-authored-by: François <mockersf@gmail.com> Co-authored-by: Corey Farwell <coreyf@rwell.org> Co-authored-by: Aevyrie <aevyrie@gmail.com>
		
			
				
	
	
		
			423 lines
		
	
	
		
			15 KiB
		
	
	
	
		
			Rust
		
	
	
	
	
	
			
		
		
	
	
			423 lines
		
	
	
		
			15 KiB
		
	
	
	
		
			Rust
		
	
	
	
	
	
| use crate::components::{GlobalTransform, Transform};
 | |
| use bevy_ecs::{
 | |
|     change_detection::Ref,
 | |
|     prelude::{Changed, DetectChanges, Entity, Query, With, Without},
 | |
| };
 | |
| use bevy_hierarchy::{Children, Parent};
 | |
| 
 | |
| /// Update [`GlobalTransform`] component of entities that aren't in the hierarchy
 | |
| ///
 | |
| /// Third party plugins should use [`transform_propagate_system_set`](crate::transform_propagate_system_set)
 | |
| /// to propagate transforms correctly.
 | |
| pub fn sync_simple_transforms(
 | |
|     mut query: Query<
 | |
|         (&Transform, &mut GlobalTransform),
 | |
|         (Changed<Transform>, Without<Parent>, Without<Children>),
 | |
|     >,
 | |
| ) {
 | |
|     query
 | |
|         .par_iter_mut()
 | |
|         .for_each_mut(|(transform, mut global_transform)| {
 | |
|             *global_transform = GlobalTransform::from(*transform);
 | |
|         });
 | |
| }
 | |
| 
 | |
| /// Update [`GlobalTransform`] component of entities based on entity hierarchy and
 | |
| /// [`Transform`] component.
 | |
| ///
 | |
| /// Third party plugins should use [`transform_propagate_system_set`](crate::transform_propagate_system_set)
 | |
| /// to propagate transforms correctly.
 | |
| pub fn propagate_transforms(
 | |
|     mut root_query: Query<
 | |
|         (Entity, &Children, Ref<Transform>, &mut GlobalTransform),
 | |
|         Without<Parent>,
 | |
|     >,
 | |
|     transform_query: Query<(Ref<Transform>, &mut GlobalTransform, Option<&Children>), With<Parent>>,
 | |
|     parent_query: Query<(Entity, Ref<Parent>)>,
 | |
| ) {
 | |
|     root_query.par_iter_mut().for_each_mut(
 | |
|         |(entity, children, transform, mut global_transform)| {
 | |
|             let changed = transform.is_changed();
 | |
|             if changed {
 | |
|                 *global_transform = GlobalTransform::from(*transform);
 | |
|             }
 | |
| 
 | |
|             for (child, actual_parent) in parent_query.iter_many(children) {
 | |
|                 assert_eq!(
 | |
|                     actual_parent.get(), entity,
 | |
|                     "Malformed hierarchy. This probably means that your hierarchy has been improperly maintained, or contains a cycle"
 | |
|                 );
 | |
|                 // SAFETY:
 | |
|                 // - `child` must have consistent parentage, or the above assertion would panic.
 | |
|                 // Since `child` is parented to a root entity, the entire hierarchy leading to it is consistent.
 | |
|                 // - We may operate as if all descendants are consistent, since `propagate_recursive` will panic before 
 | |
|                 //   continuing to propagate if it encounters an entity with inconsistent parentage.
 | |
|                 // - Since each root entity is unique and the hierarchy is consistent and forest-like,
 | |
|                 //   other root entities' `propagate_recursive` calls will not conflict with this one.
 | |
|                 // - Since this is the only place where `transform_query` gets used, there will be no conflicting fetches elsewhere.
 | |
|                 unsafe {
 | |
|                     propagate_recursive(
 | |
|                         &global_transform,
 | |
|                         &transform_query,
 | |
|                         &parent_query,
 | |
|                         child,
 | |
|                         changed || actual_parent.is_changed(),
 | |
|                     );
 | |
|                 }
 | |
|             }
 | |
|         },
 | |
|     );
 | |
| }
 | |
| 
 | |
| /// Recursively propagates the transforms for `entity` and all of its descendants.
 | |
| ///
 | |
| /// # Panics
 | |
| ///
 | |
| /// If `entity`'s descendants have a malformed hierarchy, this function will panic occur before propagating
 | |
| /// the transforms of any malformed entities and their descendants.
 | |
| ///
 | |
| /// # Safety
 | |
| ///
 | |
| /// - While this function is running, `transform_query` must not have any fetches for `entity`,
 | |
| /// nor any of its descendants.
 | |
| /// - The caller must ensure that the hierarchy leading to `entity`
 | |
| /// is well-formed and must remain as a tree or a forest. Each entity must have at most one parent.
 | |
| unsafe fn propagate_recursive(
 | |
|     parent: &GlobalTransform,
 | |
|     transform_query: &Query<
 | |
|         (Ref<Transform>, &mut GlobalTransform, Option<&Children>),
 | |
|         With<Parent>,
 | |
|     >,
 | |
|     parent_query: &Query<(Entity, Ref<Parent>)>,
 | |
|     entity: Entity,
 | |
|     mut changed: bool,
 | |
| ) {
 | |
|     let (global_matrix, children) = {
 | |
|         let Ok((transform, mut global_transform, children)) =
 | |
|             // SAFETY: This call cannot create aliased mutable references.
 | |
|             //   - The top level iteration parallelizes on the roots of the hierarchy.
 | |
|             //   - The caller ensures that each child has one and only one unique parent throughout the entire
 | |
|             //     hierarchy.
 | |
|             //
 | |
|             // For example, consider the following malformed hierarchy:
 | |
|             //
 | |
|             //     A
 | |
|             //   /   \
 | |
|             //  B     C
 | |
|             //   \   /
 | |
|             //     D
 | |
|             //
 | |
|             // D has two parents, B and C. If the propagation passes through C, but the Parent component on D points to B,
 | |
|             // the above check will panic as the origin parent does match the recorded parent.
 | |
|             //
 | |
|             // Also consider the following case, where A and B are roots:
 | |
|             //
 | |
|             //  A       B
 | |
|             //   \     /
 | |
|             //    C   D
 | |
|             //     \ /
 | |
|             //      E
 | |
|             //
 | |
|             // Even if these A and B start two separate tasks running in parallel, one of them will panic before attempting
 | |
|             // to mutably access E.
 | |
|             (unsafe { transform_query.get_unchecked(entity) }) else {
 | |
|                 return;
 | |
|             };
 | |
| 
 | |
|         changed |= transform.is_changed();
 | |
|         if changed {
 | |
|             *global_transform = parent.mul_transform(*transform);
 | |
|         }
 | |
|         (*global_transform, children)
 | |
|     };
 | |
| 
 | |
|     let Some(children) = children else { return };
 | |
|     for (child, actual_parent) in parent_query.iter_many(children) {
 | |
|         assert_eq!(
 | |
|             actual_parent.get(), entity,
 | |
|             "Malformed hierarchy. This probably means that your hierarchy has been improperly maintained, or contains a cycle"
 | |
|         );
 | |
|         // SAFETY: The caller guarantees that `transform_query` will not be fetched
 | |
|         // for any descendants of `entity`, so it is safe to call `propagate_recursive` for each child.
 | |
|         //
 | |
|         // The above assertion ensures that each child has one and only one unique parent throughout the
 | |
|         // entire hierarchy.
 | |
|         unsafe {
 | |
|             propagate_recursive(
 | |
|                 &global_matrix,
 | |
|                 transform_query,
 | |
|                 parent_query,
 | |
|                 child,
 | |
|                 changed || actual_parent.is_changed(),
 | |
|             );
 | |
|         }
 | |
|     }
 | |
| }
 | |
| 
 | |
| #[cfg(test)]
 | |
| mod test {
 | |
|     use bevy_app::prelude::*;
 | |
|     use bevy_ecs::prelude::*;
 | |
|     use bevy_ecs::system::CommandQueue;
 | |
|     use bevy_math::vec3;
 | |
|     use bevy_tasks::{ComputeTaskPool, TaskPool};
 | |
| 
 | |
|     use crate::components::{GlobalTransform, Transform};
 | |
|     use crate::systems::*;
 | |
|     use crate::TransformBundle;
 | |
|     use bevy_hierarchy::{BuildChildren, BuildWorldChildren, Children, Parent};
 | |
| 
 | |
|     #[derive(StageLabel)]
 | |
|     struct Update;
 | |
| 
 | |
|     #[test]
 | |
|     fn did_propagate() {
 | |
|         ComputeTaskPool::init(TaskPool::default);
 | |
|         let mut world = World::default();
 | |
| 
 | |
|         let mut update_stage = SystemStage::parallel();
 | |
|         update_stage.add_system(sync_simple_transforms);
 | |
|         update_stage.add_system(propagate_transforms);
 | |
| 
 | |
|         let mut schedule = Schedule::default();
 | |
|         schedule.add_stage(Update, update_stage);
 | |
| 
 | |
|         // Root entity
 | |
|         world.spawn(TransformBundle::from(Transform::from_xyz(1.0, 0.0, 0.0)));
 | |
| 
 | |
|         let mut children = Vec::new();
 | |
|         world
 | |
|             .spawn(TransformBundle::from(Transform::from_xyz(1.0, 0.0, 0.0)))
 | |
|             .with_children(|parent| {
 | |
|                 children.push(
 | |
|                     parent
 | |
|                         .spawn(TransformBundle::from(Transform::from_xyz(0.0, 2.0, 0.)))
 | |
|                         .id(),
 | |
|                 );
 | |
|                 children.push(
 | |
|                     parent
 | |
|                         .spawn(TransformBundle::from(Transform::from_xyz(0.0, 0.0, 3.)))
 | |
|                         .id(),
 | |
|                 );
 | |
|             });
 | |
|         schedule.run(&mut world);
 | |
| 
 | |
|         assert_eq!(
 | |
|             *world.get::<GlobalTransform>(children[0]).unwrap(),
 | |
|             GlobalTransform::from_xyz(1.0, 0.0, 0.0) * Transform::from_xyz(0.0, 2.0, 0.0)
 | |
|         );
 | |
| 
 | |
|         assert_eq!(
 | |
|             *world.get::<GlobalTransform>(children[1]).unwrap(),
 | |
|             GlobalTransform::from_xyz(1.0, 0.0, 0.0) * Transform::from_xyz(0.0, 0.0, 3.0)
 | |
|         );
 | |
|     }
 | |
| 
 | |
|     #[test]
 | |
|     fn did_propagate_command_buffer() {
 | |
|         let mut world = World::default();
 | |
| 
 | |
|         let mut update_stage = SystemStage::parallel();
 | |
|         update_stage.add_system(sync_simple_transforms);
 | |
|         update_stage.add_system(propagate_transforms);
 | |
| 
 | |
|         let mut schedule = Schedule::default();
 | |
|         schedule.add_stage(Update, update_stage);
 | |
| 
 | |
|         // Root entity
 | |
|         let mut queue = CommandQueue::default();
 | |
|         let mut commands = Commands::new(&mut queue, &world);
 | |
|         let mut children = Vec::new();
 | |
|         commands
 | |
|             .spawn(TransformBundle::from(Transform::from_xyz(1.0, 0.0, 0.0)))
 | |
|             .with_children(|parent| {
 | |
|                 children.push(
 | |
|                     parent
 | |
|                         .spawn(TransformBundle::from(Transform::from_xyz(0.0, 2.0, 0.0)))
 | |
|                         .id(),
 | |
|                 );
 | |
|                 children.push(
 | |
|                     parent
 | |
|                         .spawn(TransformBundle::from(Transform::from_xyz(0.0, 0.0, 3.0)))
 | |
|                         .id(),
 | |
|                 );
 | |
|             });
 | |
|         queue.apply(&mut world);
 | |
|         schedule.run(&mut world);
 | |
| 
 | |
|         assert_eq!(
 | |
|             *world.get::<GlobalTransform>(children[0]).unwrap(),
 | |
|             GlobalTransform::from_xyz(1.0, 0.0, 0.0) * Transform::from_xyz(0.0, 2.0, 0.0)
 | |
|         );
 | |
| 
 | |
|         assert_eq!(
 | |
|             *world.get::<GlobalTransform>(children[1]).unwrap(),
 | |
|             GlobalTransform::from_xyz(1.0, 0.0, 0.0) * Transform::from_xyz(0.0, 0.0, 3.0)
 | |
|         );
 | |
|     }
 | |
| 
 | |
|     #[test]
 | |
|     fn correct_children() {
 | |
|         ComputeTaskPool::init(TaskPool::default);
 | |
|         let mut world = World::default();
 | |
| 
 | |
|         let mut update_stage = SystemStage::parallel();
 | |
|         update_stage.add_system(sync_simple_transforms);
 | |
|         update_stage.add_system(propagate_transforms);
 | |
| 
 | |
|         let mut schedule = Schedule::default();
 | |
|         schedule.add_stage(Update, update_stage);
 | |
| 
 | |
|         // Add parent entities
 | |
|         let mut children = Vec::new();
 | |
|         let parent = {
 | |
|             let mut command_queue = CommandQueue::default();
 | |
|             let mut commands = Commands::new(&mut command_queue, &world);
 | |
|             let parent = commands.spawn(Transform::from_xyz(1.0, 0.0, 0.0)).id();
 | |
|             commands.entity(parent).with_children(|parent| {
 | |
|                 children.push(parent.spawn(Transform::from_xyz(0.0, 2.0, 0.0)).id());
 | |
|                 children.push(parent.spawn(Transform::from_xyz(0.0, 3.0, 0.0)).id());
 | |
|             });
 | |
|             command_queue.apply(&mut world);
 | |
|             schedule.run(&mut world);
 | |
|             parent
 | |
|         };
 | |
| 
 | |
|         assert_eq!(
 | |
|             world
 | |
|                 .get::<Children>(parent)
 | |
|                 .unwrap()
 | |
|                 .iter()
 | |
|                 .cloned()
 | |
|                 .collect::<Vec<_>>(),
 | |
|             children,
 | |
|         );
 | |
| 
 | |
|         // Parent `e1` to `e2`.
 | |
|         {
 | |
|             let mut command_queue = CommandQueue::default();
 | |
|             let mut commands = Commands::new(&mut command_queue, &world);
 | |
|             commands.entity(children[1]).add_child(children[0]);
 | |
|             command_queue.apply(&mut world);
 | |
|             schedule.run(&mut world);
 | |
|         }
 | |
| 
 | |
|         assert_eq!(
 | |
|             world
 | |
|                 .get::<Children>(parent)
 | |
|                 .unwrap()
 | |
|                 .iter()
 | |
|                 .cloned()
 | |
|                 .collect::<Vec<_>>(),
 | |
|             vec![children[1]]
 | |
|         );
 | |
| 
 | |
|         assert_eq!(
 | |
|             world
 | |
|                 .get::<Children>(children[1])
 | |
|                 .unwrap()
 | |
|                 .iter()
 | |
|                 .cloned()
 | |
|                 .collect::<Vec<_>>(),
 | |
|             vec![children[0]]
 | |
|         );
 | |
| 
 | |
|         assert!(world.despawn(children[0]));
 | |
| 
 | |
|         schedule.run(&mut world);
 | |
| 
 | |
|         assert_eq!(
 | |
|             world
 | |
|                 .get::<Children>(parent)
 | |
|                 .unwrap()
 | |
|                 .iter()
 | |
|                 .cloned()
 | |
|                 .collect::<Vec<_>>(),
 | |
|             vec![children[1]]
 | |
|         );
 | |
|     }
 | |
| 
 | |
|     #[test]
 | |
|     fn correct_transforms_when_no_children() {
 | |
|         let mut app = App::new();
 | |
|         ComputeTaskPool::init(TaskPool::default);
 | |
| 
 | |
|         app.add_system(sync_simple_transforms);
 | |
|         app.add_system(propagate_transforms);
 | |
| 
 | |
|         let translation = vec3(1.0, 0.0, 0.0);
 | |
| 
 | |
|         // These will be overwritten.
 | |
|         let mut child = Entity::from_raw(0);
 | |
|         let mut grandchild = Entity::from_raw(1);
 | |
|         let parent = app
 | |
|             .world
 | |
|             .spawn((
 | |
|                 Transform::from_translation(translation),
 | |
|                 GlobalTransform::IDENTITY,
 | |
|             ))
 | |
|             .with_children(|builder| {
 | |
|                 child = builder
 | |
|                     .spawn(TransformBundle::IDENTITY)
 | |
|                     .with_children(|builder| {
 | |
|                         grandchild = builder.spawn(TransformBundle::IDENTITY).id();
 | |
|                     })
 | |
|                     .id();
 | |
|             })
 | |
|             .id();
 | |
| 
 | |
|         app.update();
 | |
| 
 | |
|         // check the `Children` structure is spawned
 | |
|         assert_eq!(&**app.world.get::<Children>(parent).unwrap(), &[child]);
 | |
|         assert_eq!(&**app.world.get::<Children>(child).unwrap(), &[grandchild]);
 | |
|         // Note that at this point, the `GlobalTransform`s will not have updated yet, due to `Commands` delay
 | |
|         app.update();
 | |
| 
 | |
|         let mut state = app.world.query::<&GlobalTransform>();
 | |
|         for global in state.iter(&app.world) {
 | |
|             assert_eq!(global, &GlobalTransform::from_translation(translation));
 | |
|         }
 | |
|     }
 | |
| 
 | |
|     #[test]
 | |
|     #[should_panic]
 | |
|     fn panic_when_hierarchy_cycle() {
 | |
|         ComputeTaskPool::init(TaskPool::default);
 | |
|         // We cannot directly edit Parent and Children, so we use a temp world to break
 | |
|         // the hierarchy's invariants.
 | |
|         let mut temp = World::new();
 | |
|         let mut app = App::new();
 | |
| 
 | |
|         app.add_system(propagate_transforms)
 | |
|             .add_system(sync_simple_transforms);
 | |
| 
 | |
|         fn setup_world(world: &mut World) -> (Entity, Entity) {
 | |
|             let mut grandchild = Entity::from_raw(0);
 | |
|             let child = world
 | |
|                 .spawn(TransformBundle::IDENTITY)
 | |
|                 .with_children(|builder| {
 | |
|                     grandchild = builder.spawn(TransformBundle::IDENTITY).id();
 | |
|                 })
 | |
|                 .id();
 | |
|             (child, grandchild)
 | |
|         }
 | |
| 
 | |
|         let (temp_child, temp_grandchild) = setup_world(&mut temp);
 | |
|         let (child, grandchild) = setup_world(&mut app.world);
 | |
| 
 | |
|         assert_eq!(temp_child, child);
 | |
|         assert_eq!(temp_grandchild, grandchild);
 | |
| 
 | |
|         app.world
 | |
|             .spawn(TransformBundle::IDENTITY)
 | |
|             .push_children(&[child]);
 | |
|         std::mem::swap(
 | |
|             &mut *app.world.get_mut::<Parent>(child).unwrap(),
 | |
|             &mut *temp.get_mut::<Parent>(grandchild).unwrap(),
 | |
|         );
 | |
| 
 | |
|         app.update();
 | |
|     }
 | |
| }
 |