From c5ebac682ffe229b09f9b04a6c41ba12cd8f284a Mon Sep 17 00:00:00 2001 From: James Liu Date: Sun, 11 Dec 2022 23:04:04 +0000 Subject: [PATCH] Move system_commands spans into apply_buffers (#6900) # Objective A separate `tracing` span for running a system's commands is created, even if the system doesn't have commands. This is adding extra measuring overhead (see #4892) where it's not needed. ## Solution Move the span into `ParallelCommandState` and `CommandQueue`'s `SystemParamState::apply`. To get the right metadata for the span, a additional `&SystemMeta` parameter was added to `SystemParamState::apply`. --- ## Changelog Added: `SystemMeta::name` Changed: Systems without `Commands` and `ParallelCommands` will no longer show a "system_commands" span when profiling. Changed: `SystemParamState::apply` now takes a `&SystemMeta` parameter in addition to the provided `&mut World`. --- crates/bevy_ecs/macros/src/lib.rs | 8 ++-- crates/bevy_ecs/src/schedule/stage.rs | 44 +++---------------- .../src/system/commands/parallel_scope.rs | 8 +++- crates/bevy_ecs/src/system/function_system.rs | 10 ++++- crates/bevy_ecs/src/system/system_param.rs | 17 ++++--- 5 files changed, 35 insertions(+), 52 deletions(-) diff --git a/crates/bevy_ecs/macros/src/lib.rs b/crates/bevy_ecs/macros/src/lib.rs index 010a0e33108de..b8674daf0095f 100644 --- a/crates/bevy_ecs/macros/src/lib.rs +++ b/crates/bevy_ecs/macros/src/lib.rs @@ -285,8 +285,8 @@ pub fn impl_param_set(_input: TokenStream) -> TokenStream { )* } - fn apply(&mut self, world: &mut World) { - self.0.apply(world) + fn apply(&mut self, system_meta: &SystemMeta, world: &mut World) { + self.0.apply(system_meta, world) } #[inline] @@ -466,8 +466,8 @@ pub fn derive_system_param(input: TokenStream) -> TokenStream { self.state.new_archetype(archetype, system_meta) } - fn apply(&mut self, world: &mut #path::world::World) { - self.state.apply(world) + fn apply(&mut self, system_meta: &#path::system::SystemMeta, world: &mut #path::world::World) { + self.state.apply(system_meta, world) } unsafe fn get_param<'w, 's>( diff --git a/crates/bevy_ecs/src/schedule/stage.rs b/crates/bevy_ecs/src/schedule/stage.rs index 238743965d7ef..d7386d2e5990d 100644 --- a/crates/bevy_ecs/src/schedule/stage.rs +++ b/crates/bevy_ecs/src/schedule/stage.rs @@ -229,12 +229,10 @@ impl SystemStage { } pub fn apply_buffers(&mut self, world: &mut World) { + #[cfg(feature = "trace")] + let _span = bevy_utils::tracing::info_span!("stage::apply_buffers").entered(); for container in &mut self.parallel { - let system = container.system_mut(); - #[cfg(feature = "trace")] - let _span = bevy_utils::tracing::info_span!("system_commands", name = &*system.name()) - .entered(); - system.apply_buffers(world); + container.system_mut().apply_buffers(world); } } @@ -781,15 +779,7 @@ impl Stage for SystemStage { .entered(); container.system_mut().run((), world); } - { - #[cfg(feature = "trace")] - let _system_span = bevy_utils::tracing::info_span!( - "system_commands", - name = &*container.name() - ) - .entered(); - container.system_mut().apply_buffers(world); - } + container.system_mut().apply_buffers(world); } } @@ -813,15 +803,7 @@ impl Stage for SystemStage { .entered(); container.system_mut().run((), world); } - { - #[cfg(feature = "trace")] - let _system_span = bevy_utils::tracing::info_span!( - "system_commands", - name = &*container.name() - ) - .entered(); - container.system_mut().apply_buffers(world); - } + container.system_mut().apply_buffers(world); } } @@ -829,12 +811,6 @@ impl Stage for SystemStage { if self.apply_buffers { for container in &mut self.parallel { if container.should_run { - #[cfg(feature = "trace")] - let _span = bevy_utils::tracing::info_span!( - "system_commands", - name = &*container.name() - ) - .entered(); container.system_mut().apply_buffers(world); } } @@ -852,15 +828,7 @@ impl Stage for SystemStage { .entered(); container.system_mut().run((), world); } - { - #[cfg(feature = "trace")] - let _system_span = bevy_utils::tracing::info_span!( - "system_commands", - name = &*container.name() - ) - .entered(); - container.system_mut().apply_buffers(world); - } + container.system_mut().apply_buffers(world); } } diff --git a/crates/bevy_ecs/src/system/commands/parallel_scope.rs b/crates/bevy_ecs/src/system/commands/parallel_scope.rs index 5fccd74228061..72c8118aa5152 100644 --- a/crates/bevy_ecs/src/system/commands/parallel_scope.rs +++ b/crates/bevy_ecs/src/system/commands/parallel_scope.rs @@ -5,7 +5,7 @@ use thread_local::ThreadLocal; use crate::{ entity::Entities, prelude::World, - system::{SystemParam, SystemParamState}, + system::{SystemMeta, SystemParam, SystemParamState}, }; use super::{CommandQueue, Commands}; @@ -60,7 +60,11 @@ unsafe impl SystemParamState for ParallelCommandsState { Self::default() } - fn apply(&mut self, world: &mut World) { + fn apply(&mut self, _system_meta: &SystemMeta, world: &mut World) { + #[cfg(feature = "trace")] + let _system_span = + bevy_utils::tracing::info_span!("system_commands", name = _system_meta.name()) + .entered(); for cq in &mut self.thread_local_storage { cq.get_mut().apply(world); } diff --git a/crates/bevy_ecs/src/system/function_system.rs b/crates/bevy_ecs/src/system/function_system.rs index 3a4c9006ae6d6..2cc88f9e30b77 100644 --- a/crates/bevy_ecs/src/system/function_system.rs +++ b/crates/bevy_ecs/src/system/function_system.rs @@ -37,6 +37,12 @@ impl SystemMeta { } } + /// Returns the system's name + #[inline] + pub fn name(&self) -> &str { + &self.name + } + /// Returns true if the system is [`Send`]. #[inline] pub fn is_send(&self) -> bool { @@ -182,7 +188,7 @@ impl SystemState { /// This function should be called manually after the values returned by [`SystemState::get`] and [`SystemState::get_mut`] /// are finished being used. pub fn apply(&mut self, world: &mut World) { - self.param_state.apply(world); + self.param_state.apply(&self.meta, world); } #[inline] @@ -416,7 +422,7 @@ where #[inline] fn apply_buffers(&mut self, world: &mut World) { let param_state = self.param_state.as_mut().expect(Self::PARAM_MESSAGE); - param_state.apply(world); + param_state.apply(&self.system_meta, world); } #[inline] diff --git a/crates/bevy_ecs/src/system/system_param.rs b/crates/bevy_ecs/src/system/system_param.rs index c9ef8ab840eb3..63f356c4b592c 100644 --- a/crates/bevy_ecs/src/system/system_param.rs +++ b/crates/bevy_ecs/src/system/system_param.rs @@ -140,7 +140,8 @@ pub unsafe trait SystemParamState: Send + Sync + 'static { #[inline] fn new_archetype(&mut self, _archetype: &Archetype, _system_meta: &mut SystemMeta) {} #[inline] - fn apply(&mut self, _world: &mut World) {} + #[allow(unused_variables)] + fn apply(&mut self, system_meta: &SystemMeta, _world: &mut World) {} type Item<'world, 'state>: SystemParam; /// # Safety @@ -615,7 +616,11 @@ unsafe impl SystemParamState for CommandQueue { Default::default() } - fn apply(&mut self, world: &mut World) { + fn apply(&mut self, _system_meta: &SystemMeta, world: &mut World) { + #[cfg(feature = "trace")] + let _system_span = + bevy_utils::tracing::info_span!("system_commands", name = _system_meta.name()) + .entered(); self.apply(world); } @@ -1473,9 +1478,9 @@ macro_rules! impl_system_param_tuple { } #[inline] - fn apply(&mut self, _world: &mut World) { + fn apply(&mut self, _system_meta: &SystemMeta, _world: &mut World) { let ($($param,)*) = self; - $($param.apply(_world);)* + $($param.apply(_system_meta, _world);)* } #[inline] @@ -1611,8 +1616,8 @@ unsafe impl + 'static> SystemPara self.0.new_archetype(archetype, system_meta); } - fn apply(&mut self, world: &mut World) { - self.0.apply(world); + fn apply(&mut self, system_meta: &SystemMeta, world: &mut World) { + self.0.apply(system_meta, world); } unsafe fn get_param<'world, 'state>(