From 2bb91e8d7d3332aa27b58a06ae027211e718fded Mon Sep 17 00:00:00 2001 From: Imbris Date: Fri, 18 Jun 2021 02:23:48 -0400 Subject: [PATCH] Fix span macros by putting cfgs outside the macro (they are evaluated in the crate where the macro is used), add shorthand for common case of prof_span macro, add some spans to the client code and spiff bits of it --- client/src/lib.rs | 41 ++++++++++++++++--------- common/base/src/lib.rs | 70 ++++++++++++++++++++++++++++++------------ 2 files changed, 77 insertions(+), 34 deletions(-) diff --git a/client/src/lib.rs b/client/src/lib.rs index de2f3d56b4..36b2d4bb79 100644 --- a/client/src/lib.rs +++ b/client/src/lib.rs @@ -43,7 +43,7 @@ use common::{ uid::{Uid, UidAllocator}, vol::RectVolSize, }; -use common_base::span; +use common_base::{prof_span, span}; use common_net::{ msg::{ self, validate_chat_msg, @@ -760,6 +760,7 @@ impl Client { where S: Into, { + prof_span!("send_msg_err"); let msg: ClientMsg = msg.into(); #[cfg(debug_assertions)] { @@ -1426,6 +1427,7 @@ impl Client { // 1) Handle input from frontend. // Pass character actions from frontend input to the player's entity. if self.presence.is_some() { + prof_span!("handle and send inputs"); if let Err(e) = self .state .ecs() @@ -1457,21 +1459,25 @@ impl Client { // Prepare for new events { + prof_span!("Last comps update"); let ecs = self.state.ecs(); - for (entity, _) in (&ecs.entities(), &ecs.read_storage::()).join() { - let mut last_character_states = - ecs.write_storage::>(); - if let Some(client_character_state) = - ecs.read_storage::().get(entity) + let mut last_character_states = ecs.write_storage::>(); + for (entity, _, character_state) in ( + &ecs.entities(), + &ecs.read_storage::(), + &ecs.read_storage::(), + ) + .join() + { + if let Some(l) = last_character_states + .entry(entity) + .ok() + .map(|l| l.or_insert_with(|| comp::Last(character_state.clone()))) + // TODO: since this just updates when the variant changes we should + // just store the variant to avoid the clone overhead + .filter(|l| !character_state.same_variant(&l.0)) { - if let Some(l) = last_character_states - .entry(entity) - .ok() - .map(|l| l.or_insert_with(|| comp::Last(client_character_state.clone()))) - .filter(|l| !client_character_state.same_variant(&l.0)) - { - *l = comp::Last(client_character_state.clone()); - } + *l = comp::Last(character_state.clone()); } } } @@ -1520,6 +1526,7 @@ impl Client { .get(self.entity()) .cloned(); if let (Some(pos), Some(view_distance)) = (pos, self.view_distance) { + prof_span!("terrain"); let chunk_pos = self.state.terrain().pos_key(pos.0.map(|e| e as i32)); // Remove chunks that are too far from the player. @@ -1653,6 +1660,7 @@ impl Client { frontend_events: &mut Vec, msg: ServerGeneral, ) -> Result<(), Error> { + prof_span!("handle_server_msg"); match msg { ServerGeneral::Disconnect(reason) => match reason { DisconnectReason::Shutdown => return Err(Error::ServerShutdown), @@ -1805,6 +1813,7 @@ impl Client { frontend_events: &mut Vec, msg: ServerGeneral, ) -> Result<(), Error> { + prof_span!("handle_server_in_game_msg"); match msg { ServerGeneral::GroupUpdate(change_notification) => { use comp::group::ChangeNotification::*; @@ -1979,6 +1988,7 @@ impl Client { #[allow(clippy::unnecessary_wraps)] fn handle_server_terrain_msg(&mut self, msg: ServerGeneral) -> Result<(), Error> { + prof_span!("handle_server_terrain_mgs"); match msg { ServerGeneral::TerrainChunkUpdate { key, chunk } => { if let Some(chunk) = chunk.ok().and_then(|c| c.to_chunk()) { @@ -2004,6 +2014,7 @@ impl Client { events: &mut Vec, msg: ServerGeneral, ) -> Result<(), Error> { + prof_span!("handle_character_screen_msg"); match msg { ServerGeneral::CharacterListUpdate(character_list) => { self.character_list.characters = character_list; @@ -2034,6 +2045,7 @@ impl Client { } fn handle_ping_msg(&mut self, msg: PingMsg) -> Result<(), Error> { + prof_span!("handle_ping_msg"); match msg { PingMsg::Ping => { self.send_msg_err(PingMsg::Pong)?; @@ -2088,6 +2100,7 @@ impl Client { /// Handle new server messages. fn handle_new_messages(&mut self) -> Result, Error> { + prof_span!("handle_new_messages"); let mut frontend_events = Vec::new(); // Check that we have an valid connection. diff --git a/common/base/src/lib.rs b/common/base/src/lib.rs index d05b952127..800869cf01 100644 --- a/common/base/src/lib.rs +++ b/common/base/src/lib.rs @@ -6,20 +6,21 @@ pub use userdata_dir::userdata_dir; #[cfg(feature = "tracy")] pub use tracy_client; +#[cfg(not(feature = "tracy"))] #[macro_export] macro_rules! plot { ($name:expr, $value:expr) => { - #[cfg(feature = "tracy")] - { - use $crate::tracy_client::{create_plot, Plot}; - static PLOT: Plot = create_plot!($name); - PLOT.point($value); - } - #[cfg(not(feature = "tracy"))] - { - // type check - let _: f64 = $value; - } + // type check + let _: f64 = $value; + }; +} +#[cfg(feature = "tracy")] +#[macro_export] +macro_rules! plot { + ($name:expr, $value:expr) => { + use $crate::tracy_client::{create_plot, Plot}; + static PLOT: Plot = create_plot!($name); + PLOT.point($value); }; } @@ -45,6 +46,7 @@ macro_rules! dev_panic { } // https://discordapp.com/channels/676678179678715904/676685797524766720/723358438943621151 +#[cfg(not(feature = "tracy"))] #[macro_export] macro_rules! span { ($guard_name:tt, $level:ident, $name:expr, $($fields:tt)*) => { @@ -56,12 +58,27 @@ macro_rules! span { let $guard_name = span.enter(); }; ($guard_name:tt, $name:expr) => { - #[cfg(not(feature = "tracy"))] let span = tracing::span!(tracing::Level::TRACE, $name); - #[cfg(not(feature = "tracy"))] let $guard_name = span.enter(); + }; + ($guard_name:tt, $no_tracy_name:expr, $tracy_name:expr) => { + $crate::span!($guard_name, $no_tracy_name); + }; +} + +#[cfg(feature = "tracy")] +#[macro_export] +macro_rules! span { + ($guard_name:tt, $level:ident, $name:expr, $($fields:tt)*) => { + let span = tracing::span!(tracing::Level::$level, $name, $($fields)*); + let $guard_name = span.enter(); + }; + ($guard_name:tt, $level:ident, $name:expr) => { + let span = tracing::span!(tracing::Level::$level, $name); + let $guard_name = span.enter(); + }; + ($guard_name:tt, $name:expr) => { // Directly use `tracy_client` to decrease overhead for better timing - #[cfg(feature = "tracy")] let $guard_name = $crate::tracy_client::Span::new( $name, "", @@ -72,9 +89,6 @@ macro_rules! span { ); }; ($guard_name:tt, $no_tracy_name:expr, $tracy_name:expr) => { - #[cfg(not(feature = "tracy"))] - $crate::span!($guard_name, $no_tracy_name); - #[cfg(feature = "tracy")] $crate::span!($guard_name, $tracy_name); }; } @@ -87,9 +101,22 @@ pub struct ProfSpan; /// Like the span macro but only used when profiling and not in regular tracing /// operations #[macro_export] +#[cfg(not(feature = "tracy"))] +macro_rules! prof_span { + ($guard_name:tt, $name:expr) => { + let $guard_name = $crate::ProfSpan; + }; + // Shorthand for when you want the guard to just be dropped at the end of the scope instead + // of controlling it manually + ($name:expr) => {}; +} + +/// Like the span macro but only used when profiling and not in regular tracing +/// operations +#[macro_export] +#[cfg(feature = "tracy")] macro_rules! prof_span { ($guard_name:tt, $name:expr) => { - #[cfg(feature = "tracy")] let $guard_name = $crate::ProfSpan($crate::tracy_client::Span::new( $name, "", @@ -98,8 +125,11 @@ macro_rules! prof_span { // No callstack since this has significant overhead 0, )); - #[cfg(not(feature = "tracy"))] - let $guard_name = $crate::ProfSpan; + }; + // Shorthand for when you want the guard to just be dropped at the end of the scope instead + // of controlling it manually + ($name:expr) => { + $crate::prof_span!(_guard, $name); }; }