Reduce the use of atomics in the render phase (#7084)
# Objective Speed up the render phase of rendering. An extension of #6885. `SystemState::get` increments the `World`'s change tick atomically every time it's called. This is notably more expensive than a unsynchronized increment, even without contention. It also updates the archetypes, even when there has been nothing to update when it's called repeatedly. ## Solution Piggyback off of #6885. Split `SystemState::validate_world_and_update_archetypes` into `SystemState::validate_world` and `SystemState::update_archetypes`, and make the later `pub`. Then create safe variants of `SystemState::get_unchecked_manual` that still validate the `World` but do not update archetypes and do not increment the change tick using `World::read_change_tick` and `World::change_tick`. Update `RenderCommandState` to call `SystemState::update_archetypes` in `Draw::prepare` and `SystemState::get_manual` in `Draw::draw`. ## Performance There's a slight perf benefit (~2%) for `main_opaque_pass_3d` on `many_foxes` (340.39 us -> 333.32 us)  ## Alternatives We can change `SystemState::get` to not increment the `World`'s change tick. Though this would still put updating the archetypes and an atomic read on the hot-path. --- ## Changelog Added: `SystemState::get_manual` Added: `SystemState::get_manual_mut` Added: `SystemState::update_archetypes`
This commit is contained in:
		
							parent
							
								
									9eefd7c022
								
							
						
					
					
						commit
						88b353c4b1
					
				@ -170,7 +170,8 @@ impl<Param: SystemParam> SystemState<Param> {
 | 
			
		||||
    where
 | 
			
		||||
        Param: ReadOnlySystemParam,
 | 
			
		||||
    {
 | 
			
		||||
        self.validate_world_and_update_archetypes(world);
 | 
			
		||||
        self.validate_world(world);
 | 
			
		||||
        self.update_archetypes(world);
 | 
			
		||||
        // SAFETY: Param is read-only and doesn't allow mutable access to World. It also matches the World this SystemState was created with.
 | 
			
		||||
        unsafe { self.get_unchecked_manual(world) }
 | 
			
		||||
    }
 | 
			
		||||
@ -178,7 +179,8 @@ impl<Param: SystemParam> SystemState<Param> {
 | 
			
		||||
    /// Retrieve the mutable [`SystemParam`] values.
 | 
			
		||||
    #[inline]
 | 
			
		||||
    pub fn get_mut<'w, 's>(&'s mut self, world: &'w mut World) -> SystemParamItem<'w, 's, Param> {
 | 
			
		||||
        self.validate_world_and_update_archetypes(world);
 | 
			
		||||
        self.validate_world(world);
 | 
			
		||||
        self.update_archetypes(world);
 | 
			
		||||
        // SAFETY: World is uniquely borrowed and matches the World this SystemState was created with.
 | 
			
		||||
        unsafe { self.get_unchecked_manual(world) }
 | 
			
		||||
    }
 | 
			
		||||
@ -196,8 +198,20 @@ impl<Param: SystemParam> SystemState<Param> {
 | 
			
		||||
        self.world_id == world.id()
 | 
			
		||||
    }
 | 
			
		||||
 | 
			
		||||
    fn validate_world_and_update_archetypes(&mut self, world: &World) {
 | 
			
		||||
    /// Asserts that the [`SystemState`] matches the provided [`World`].
 | 
			
		||||
    #[inline]
 | 
			
		||||
    fn validate_world(&self, world: &World) {
 | 
			
		||||
        assert!(self.matches_world(world), "Encountered a mismatched World. A SystemState cannot be used with Worlds other than the one it was created with.");
 | 
			
		||||
    }
 | 
			
		||||
 | 
			
		||||
    /// Updates the state's internal view of the `world`'s archetypes. If this is not called before fetching the parameters,
 | 
			
		||||
    /// the results may not accurately reflect what is in the `world`.
 | 
			
		||||
    ///
 | 
			
		||||
    /// This is only required if [`SystemState::get_manual`] or [`SystemState::get_manual_mut`] is being called, and it only needs to
 | 
			
		||||
    /// be called if the `world` has been structurally mutated (i.e. added/removed a component or resource). Users using
 | 
			
		||||
    /// [`SystemState::get`] or [`SystemState::get_mut`] do not need to call this as it will be automatically called for them.
 | 
			
		||||
    #[inline]
 | 
			
		||||
    pub fn update_archetypes(&mut self, world: &World) {
 | 
			
		||||
        let archetypes = world.archetypes();
 | 
			
		||||
        let new_generation = archetypes.generation();
 | 
			
		||||
        let old_generation = std::mem::replace(&mut self.archetype_generation, new_generation);
 | 
			
		||||
@ -212,6 +226,43 @@ impl<Param: SystemParam> SystemState<Param> {
 | 
			
		||||
        }
 | 
			
		||||
    }
 | 
			
		||||
 | 
			
		||||
    /// Retrieve the [`SystemParam`] values. This can only be called when all parameters are read-only.
 | 
			
		||||
    /// This will not update the state's view of the world's archetypes automatically nor increment the
 | 
			
		||||
    /// world's change tick.
 | 
			
		||||
    ///
 | 
			
		||||
    /// For this to return accurate results, ensure [`SystemState::update_archetypes`] is called before this
 | 
			
		||||
    /// function.
 | 
			
		||||
    ///
 | 
			
		||||
    /// Users should strongly prefer to use [`SystemState::get`] over this function.
 | 
			
		||||
    #[inline]
 | 
			
		||||
    pub fn get_manual<'w, 's>(&'s mut self, world: &'w World) -> SystemParamItem<'w, 's, Param>
 | 
			
		||||
    where
 | 
			
		||||
        Param: ReadOnlySystemParam,
 | 
			
		||||
    {
 | 
			
		||||
        self.validate_world(world);
 | 
			
		||||
        let change_tick = world.read_change_tick();
 | 
			
		||||
        // SAFETY: Param is read-only and doesn't allow mutable access to World. It also matches the World this SystemState was created with.
 | 
			
		||||
        unsafe { self.fetch(world, change_tick) }
 | 
			
		||||
    }
 | 
			
		||||
 | 
			
		||||
    /// Retrieve the mutable [`SystemParam`] values.  This will not update the state's view of the world's archetypes
 | 
			
		||||
    /// automatically nor increment the world's change tick.
 | 
			
		||||
    ///
 | 
			
		||||
    /// For this to return accurate results, ensure [`SystemState::update_archetypes`] is called before this
 | 
			
		||||
    /// function.
 | 
			
		||||
    ///
 | 
			
		||||
    /// Users should strongly prefer to use [`SystemState::get_mut`] over this function.
 | 
			
		||||
    #[inline]
 | 
			
		||||
    pub fn get_manual_mut<'w, 's>(
 | 
			
		||||
        &'s mut self,
 | 
			
		||||
        world: &'w mut World,
 | 
			
		||||
    ) -> SystemParamItem<'w, 's, Param> {
 | 
			
		||||
        self.validate_world(world);
 | 
			
		||||
        let change_tick = world.change_tick();
 | 
			
		||||
        // SAFETY: World is uniquely borrowed and matches the World this SystemState was created with.
 | 
			
		||||
        unsafe { self.fetch(world, change_tick) }
 | 
			
		||||
    }
 | 
			
		||||
 | 
			
		||||
    /// Retrieve the [`SystemParam`] values. This will not update archetypes automatically.
 | 
			
		||||
    ///
 | 
			
		||||
    /// # Safety
 | 
			
		||||
@ -224,6 +275,19 @@ impl<Param: SystemParam> SystemState<Param> {
 | 
			
		||||
        world: &'w World,
 | 
			
		||||
    ) -> SystemParamItem<'w, 's, Param> {
 | 
			
		||||
        let change_tick = world.increment_change_tick();
 | 
			
		||||
        self.fetch(world, change_tick)
 | 
			
		||||
    }
 | 
			
		||||
 | 
			
		||||
    /// # Safety
 | 
			
		||||
    /// This call might access any of the input parameters in a way that violates Rust's mutability rules. Make sure the data
 | 
			
		||||
    /// access is safe in the context of global [`World`] access. The passed-in [`World`] _must_ be the [`World`] the [`SystemState`] was
 | 
			
		||||
    /// created with.
 | 
			
		||||
    #[inline]
 | 
			
		||||
    unsafe fn fetch<'w, 's>(
 | 
			
		||||
        &'s mut self,
 | 
			
		||||
        world: &'w World,
 | 
			
		||||
        change_tick: u32,
 | 
			
		||||
    ) -> SystemParamItem<'w, 's, Param> {
 | 
			
		||||
        let param = Param::get_param(&mut self.param_state, &self.meta, world, change_tick);
 | 
			
		||||
        self.meta.last_change_tick = change_tick;
 | 
			
		||||
        param
 | 
			
		||||
 | 
			
		||||
@ -244,6 +244,7 @@ where
 | 
			
		||||
    /// Prepares the render command to be used. This is called once and only once before the phase
 | 
			
		||||
    /// begins. There may be zero or more `draw` calls following a call to this function.
 | 
			
		||||
    fn prepare(&mut self, world: &'_ World) {
 | 
			
		||||
        self.state.update_archetypes(world);
 | 
			
		||||
        self.view.update_archetypes(world);
 | 
			
		||||
        self.entity.update_archetypes(world);
 | 
			
		||||
    }
 | 
			
		||||
@ -256,7 +257,7 @@ where
 | 
			
		||||
        view: Entity,
 | 
			
		||||
        item: &P,
 | 
			
		||||
    ) {
 | 
			
		||||
        let param = self.state.get(world);
 | 
			
		||||
        let param = self.state.get_manual(world);
 | 
			
		||||
        let view = self.view.get_manual(world, view).unwrap();
 | 
			
		||||
        let entity = self.entity.get_manual(world, item.entity()).unwrap();
 | 
			
		||||
        // TODO: handle/log `RenderCommand` failure
 | 
			
		||||
 | 
			
		||||
		Loading…
	
		Reference in New Issue
	
	Block a user