Skip to content

Commit

Permalink
Move system_commands spans into apply_buffers (#6900)
Browse files Browse the repository at this point in the history 
# 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`.
  • Loading branch information
@james7132
james7132 committed Dec 11, 2022
1 parent 4820917 commit 79b9231
Show file tree
Hide file tree
Showing 5 changed files with 35 additions and 52 deletions.
8 changes: 4 additions & 4 deletions crates/bevy_ecs/macros/src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down Expand Up @@ -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>(
Expand Down
44 changes: 6 additions & 38 deletions crates/bevy_ecs/src/schedule/stage.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}

Expand Down Expand Up @@ -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);
}
}

Expand All @@ -813,28 +803,14 @@ 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);
}
}

// Apply parallel systems' buffers.
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);
}
}
Expand All @@ -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);
}
}

Expand Down
8 changes: 6 additions & 2 deletions crates/bevy_ecs/src/system/commands/parallel_scope.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -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);
}
Expand Down
10 changes: 8 additions & 2 deletions crates/bevy_ecs/src/system/function_system.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -182,7 +188,7 @@ impl<Param: SystemParam> SystemState<Param> {
/// 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]
Expand Down Expand Up @@ -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]
Expand Down
17 changes: 11 additions & 6 deletions crates/bevy_ecs/src/system/system_param.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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<State = Self>;
/// # Safety
Expand Down Expand Up @@ -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);
}

Expand Down Expand Up @@ -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]
Expand Down Expand Up @@ -1611,8 +1616,8 @@ unsafe impl<S: SystemParamState, P: SystemParam<State = S> + '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>(
Expand Down

0 comments on commit 79b9231

Please sign in to comment.