From ca8194882318cc8f3b7a2f696aa895bdddfb7e05 Mon Sep 17 00:00:00 2001 From: LeshaInc Date: Mon, 24 Jul 2023 16:55:53 +0300 Subject: [PATCH 1/4] Replace `DiagnosticId` by `DiagnosticPath` --- .../asset_count_diagnostics_plugin.rs | 62 +++--- crates/bevy_diagnostic/Cargo.toml | 2 + crates/bevy_diagnostic/src/diagnostic.rs | 184 +++++++++++++----- .../src/entity_count_diagnostics_plugin.rs | 9 +- .../src/frame_time_diagnostics_plugin.rs | 28 ++- crates/bevy_diagnostic/src/lib.rs | 5 +- .../src/log_diagnostics_plugin.rs | 118 +++++------ .../system_information_diagnostics_plugin.rs | 34 ++-- examples/diagnostics/custom_diagnostic.rs | 18 +- 9 files changed, 265 insertions(+), 195 deletions(-) diff --git a/crates/bevy_asset/src/diagnostic/asset_count_diagnostics_plugin.rs b/crates/bevy_asset/src/diagnostic/asset_count_diagnostics_plugin.rs index dafd21567347b..b595405638def 100644 --- a/crates/bevy_asset/src/diagnostic/asset_count_diagnostics_plugin.rs +++ b/crates/bevy_asset/src/diagnostic/asset_count_diagnostics_plugin.rs @@ -1,60 +1,60 @@ +use std::marker::PhantomData; + use crate::{Asset, Assets}; use bevy_app::prelude::*; -use bevy_diagnostic::{ - Diagnostic, DiagnosticId, Diagnostics, DiagnosticsStore, MAX_DIAGNOSTIC_NAME_WIDTH, -}; +use bevy_diagnostic::{Diagnostic, DiagnosticPath, Diagnostics, DiagnosticsStore}; use bevy_ecs::prelude::*; /// Adds an asset count diagnostic to an [`App`] for assets of type `T`. pub struct AssetCountDiagnosticsPlugin { - marker: std::marker::PhantomData, + marker: PhantomData, } impl Default for AssetCountDiagnosticsPlugin { fn default() -> Self { Self { - marker: std::marker::PhantomData, + marker: PhantomData, } } } impl Plugin for AssetCountDiagnosticsPlugin { fn build(&self, app: &mut App) { - app.add_systems(Startup, Self::setup_system) - .add_systems(Update, Self::diagnostic_system); + app.insert_resource(AssetDiagnosticPath { + inner: Self::diagnostic_path(), + marker: PhantomData::, + }) + .add_systems(Startup, Self::setup_system) + .add_systems(Update, Self::diagnostic_system); } } impl AssetCountDiagnosticsPlugin { - /// Gets unique id of this diagnostic. - /// - /// The diagnostic id is the type uuid of `T`. - pub fn diagnostic_id() -> DiagnosticId { - DiagnosticId(T::TYPE_UUID) + /// Gets diagnostic path of format `asset/{T::type_path}/count` + pub fn diagnostic_path() -> DiagnosticPath { + DiagnosticPath::from_components(["asset", T::type_path(), "count"]) } /// Registers the asset count diagnostic for the current application. - pub fn setup_system(mut diagnostics: ResMut) { - let asset_type_name = std::any::type_name::(); - let max_length = MAX_DIAGNOSTIC_NAME_WIDTH - "asset_count ".len(); - diagnostics.add(Diagnostic::new( - Self::diagnostic_id(), - format!( - "asset_count {}", - if asset_type_name.len() > max_length { - asset_type_name - .split_at(asset_type_name.len() - max_length + 1) - .1 - } else { - asset_type_name - } - ), - 20, - )); + pub fn setup_system( + path: Res>, + mut diagnostics: ResMut, + ) { + diagnostics.add(Diagnostic::new(path.inner.clone())); } /// Updates the asset count of `T` assets. - pub fn diagnostic_system(mut diagnostics: Diagnostics, assets: Res>) { - diagnostics.add_measurement(Self::diagnostic_id(), || assets.len() as f64); + pub fn diagnostic_system( + path: Res>, + mut diagnostics: Diagnostics, + assets: Res>, + ) { + diagnostics.add_measurement(&path.inner, || assets.len() as f64); } } + +#[derive(Debug, Clone, Resource)] +pub struct AssetDiagnosticPath { + pub inner: DiagnosticPath, + marker: std::marker::PhantomData, +} diff --git a/crates/bevy_diagnostic/Cargo.toml b/crates/bevy_diagnostic/Cargo.toml index 52e3b1b86f15c..ef0b62694066d 100644 --- a/crates/bevy_diagnostic/Cargo.toml +++ b/crates/bevy_diagnostic/Cargo.toml @@ -21,6 +21,8 @@ bevy_log = { path = "../bevy_log", version = "0.12.0-dev" } bevy_time = { path = "../bevy_time", version = "0.12.0-dev" } bevy_utils = { path = "../bevy_utils", version = "0.12.0-dev" } +const-fnv1a-hash = "1.1.0" + # MacOS [target.'cfg(all(target_os="macos"))'.dependencies] # Some features of sysinfo are not supported by apple. This will disable those features on apple devices diff --git a/crates/bevy_diagnostic/src/diagnostic.rs b/crates/bevy_diagnostic/src/diagnostic.rs index 6fa04ce573f20..0494856bba4de 100644 --- a/crates/bevy_diagnostic/src/diagnostic.rs +++ b/crates/bevy_diagnostic/src/diagnostic.rs @@ -1,24 +1,108 @@ +use std::hash::{Hash, Hasher}; +use std::{borrow::Cow, collections::VecDeque}; + use bevy_app::App; use bevy_ecs::system::{Deferred, Res, Resource, SystemBuffer, SystemParam}; -use bevy_log::warn; -use bevy_utils::{Duration, Instant, StableHashMap, Uuid}; -use std::{borrow::Cow, collections::VecDeque}; +use bevy_utils::{Duration, Instant, StableHashMap}; +use const_fnv1a_hash::fnv1a_hash_str_64; + +use crate::DEFAULT_MAX_HISTORY_LENGTH; + +/// Unique diagnostic path, separated by `/`. +/// +/// Requirements: +/// - Can't be empty +/// - Can't have leading or trailing `/` +/// - Can't have empty components. +#[derive(Debug, Clone)] +pub struct DiagnosticPath { + path: Cow<'static, str>, + hash: u64, +} + +impl DiagnosticPath { + /// Create a new `DiagnosticPath`. Usable in const contexts. + /// + /// **Note**: path is not validated, so make sure it follows all the requirements. + pub const fn const_new(path: &'static str) -> DiagnosticPath { + DiagnosticPath { + path: Cow::Borrowed(path), + hash: fnv1a_hash_str_64(path), + } + } + + /// Create a new `DiagnosticPath` from the specified string. + pub fn new(path: impl Into>) -> DiagnosticPath { + let path = path.into(); + + debug_assert!(!path.is_empty(), "diagnostic path can't be empty"); + debug_assert!( + !path.starts_with('/'), + "diagnostic path can't be start with `/`" + ); + debug_assert!( + !path.ends_with('/'), + "diagnostic path can't be end with `/`" + ); + debug_assert!( + !path.contains("//"), + "diagnostic path can't contain empty components" + ); + + DiagnosticPath { + hash: fnv1a_hash_str_64(&path), + path, + } + } + + /// Create a new `DiagnosticPath` from an iterator over components. + pub fn from_components<'a>(components: impl IntoIterator) -> DiagnosticPath { + let mut buf = String::new(); + + for (i, component) in components.into_iter().enumerate() { + if i > 0 { + buf.push('/') + } + buf.push_str(component) + } + + DiagnosticPath::new(buf) + } -use crate::MAX_DIAGNOSTIC_NAME_WIDTH; + /// Returns full path, joined by `/` + pub fn as_str(&self) -> &str { + &self.path + } -/// Unique identifier for a [Diagnostic] -#[derive(Debug, Copy, Clone, Hash, Eq, PartialEq, PartialOrd, Ord)] -pub struct DiagnosticId(pub Uuid); + /// Returns an iterator over path components. + pub fn components(&self) -> impl Iterator + '_ { + self.path.split('/') + } +} -impl DiagnosticId { - pub const fn from_u128(value: u128) -> Self { - DiagnosticId(Uuid::from_u128(value)) +impl From for String { + fn from(path: DiagnosticPath) -> Self { + path.path.into() } } -impl Default for DiagnosticId { - fn default() -> Self { - DiagnosticId(Uuid::new_v4()) +impl Eq for DiagnosticPath {} + +impl PartialEq for DiagnosticPath { + fn eq(&self, other: &Self) -> bool { + self.hash == other.hash + } +} + +impl Hash for DiagnosticPath { + fn hash(&self, state: &mut H) { + state.write_u64(self.hash); + } +} + +impl std::fmt::Display for DiagnosticPath { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + self.path.fmt(f) } } @@ -33,8 +117,7 @@ pub struct DiagnosticMeasurement { /// Diagnostic examples: frames per second, CPU usage, network latency #[derive(Debug)] pub struct Diagnostic { - pub id: DiagnosticId, - pub name: Cow<'static, str>, + path: DiagnosticPath, pub suffix: Cow<'static, str>, history: VecDeque, sum: f64, @@ -71,27 +154,13 @@ impl Diagnostic { self.history.push_back(measurement); } - /// Create a new diagnostic with the given ID, name and maximum history. - pub fn new( - id: DiagnosticId, - name: impl Into>, - max_history_length: usize, - ) -> Diagnostic { - let name = name.into(); - if name.chars().count() > MAX_DIAGNOSTIC_NAME_WIDTH { - // This could be a false positive due to a unicode width being shorter - warn!( - "Diagnostic {:?} has name longer than {} characters, and so might overflow in the LogDiagnosticsPlugin\ - Consider using a shorter name.", - name, MAX_DIAGNOSTIC_NAME_WIDTH - ); - } + /// Create a new diagnostic with the given path. + pub fn new(path: DiagnosticPath) -> Diagnostic { Diagnostic { - id, - name, + path, suffix: Cow::Borrowed(""), - history: VecDeque::with_capacity(max_history_length), - max_history_length, + history: VecDeque::with_capacity(DEFAULT_MAX_HISTORY_LENGTH), + max_history_length: DEFAULT_MAX_HISTORY_LENGTH, sum: 0.0, ema: 0.0, ema_smoothing_factor: 2.0 / 21.0, @@ -99,6 +168,15 @@ impl Diagnostic { } } + /// Set the maximum history length. + #[must_use] + pub fn with_max_history_length(mut self, max_history_length: usize) -> Self { + self.max_history_length = max_history_length; + self.history.reserve(self.max_history_length); + self.history.shrink_to(self.max_history_length); + self + } + /// Add a suffix to use when logging the value, can be used to show a unit. #[must_use] pub fn with_suffix(mut self, suffix: impl Into>) -> Self { @@ -122,6 +200,10 @@ impl Diagnostic { self } + pub fn path(&self) -> &DiagnosticPath { + &self.path + } + /// Get the latest measurement from this diagnostic. #[inline] pub fn measurement(&self) -> Option<&DiagnosticMeasurement> { @@ -200,7 +282,7 @@ impl Diagnostic { pub struct DiagnosticsStore { // This uses a [`StableHashMap`] to ensure that the iteration order is deterministic between // runs when all diagnostics are inserted in the same order. - diagnostics: StableHashMap, + diagnostics: StableHashMap, } impl DiagnosticsStore { @@ -208,21 +290,21 @@ impl DiagnosticsStore { /// /// If possible, prefer calling [`App::register_diagnostic`]. pub fn add(&mut self, diagnostic: Diagnostic) { - self.diagnostics.insert(diagnostic.id, diagnostic); + self.diagnostics.insert(diagnostic.path.clone(), diagnostic); } - pub fn get(&self, id: DiagnosticId) -> Option<&Diagnostic> { - self.diagnostics.get(&id) + pub fn get(&self, path: &DiagnosticPath) -> Option<&Diagnostic> { + self.diagnostics.get(path) } - pub fn get_mut(&mut self, id: DiagnosticId) -> Option<&mut Diagnostic> { - self.diagnostics.get_mut(&id) + pub fn get_mut(&mut self, path: &DiagnosticPath) -> Option<&mut Diagnostic> { + self.diagnostics.get_mut(path) } /// Get the latest [`DiagnosticMeasurement`] from an enabled [`Diagnostic`]. - pub fn get_measurement(&self, id: DiagnosticId) -> Option<&DiagnosticMeasurement> { + pub fn get_measurement(&self, path: &DiagnosticPath) -> Option<&DiagnosticMeasurement> { self.diagnostics - .get(&id) + .get(path) .filter(|diagnostic| diagnostic.is_enabled) .and_then(|diagnostic| diagnostic.measurement()) } @@ -244,13 +326,13 @@ impl<'w, 's> Diagnostics<'w, 's> { /// Add a measurement to an enabled [`Diagnostic`]. The measurement is passed as a function so that /// it will be evaluated only if the [`Diagnostic`] is enabled. This can be useful if the value is /// costly to calculate. - pub fn add_measurement(&mut self, id: DiagnosticId, value: F) + pub fn add_measurement(&mut self, path: &DiagnosticPath, value: F) where F: FnOnce() -> f64, { if self .store - .get(id) + .get(path) .filter(|diagnostic| diagnostic.is_enabled) .is_some() { @@ -258,13 +340,13 @@ impl<'w, 's> Diagnostics<'w, 's> { time: Instant::now(), value: value(), }; - self.queue.0.insert(id, measurement); + self.queue.0.insert(path.clone(), measurement); } } } #[derive(Default)] -struct DiagnosticsBuffer(StableHashMap); +struct DiagnosticsBuffer(StableHashMap); impl SystemBuffer for DiagnosticsBuffer { fn apply( @@ -273,8 +355,8 @@ impl SystemBuffer for DiagnosticsBuffer { world: &mut bevy_ecs::world::World, ) { let mut diagnostics = world.resource_mut::(); - for (id, measurement) in self.0.drain() { - if let Some(diagnostic) = diagnostics.get_mut(id) { + for (path, measurement) in self.0.drain() { + if let Some(diagnostic) = diagnostics.get_mut(&path) { diagnostic.add_measurement(measurement); } } @@ -293,12 +375,12 @@ impl RegisterDiagnostic for App { /// /// ```rust /// use bevy_app::App; - /// use bevy_diagnostic::{Diagnostic, DiagnosticsPlugin, DiagnosticId, RegisterDiagnostic}; + /// use bevy_diagnostic::{Diagnostic, DiagnosticsPlugin, DiagnosticPath, RegisterDiagnostic}; /// - /// const UNIQUE_DIAG_ID: DiagnosticId = DiagnosticId::from_u128(42); + /// const UNIQUE_DIAG_PATH: DiagnosticPath = DiagnosticPath::const_new("foo/bar"); /// /// App::new() - /// .register_diagnostic(Diagnostic::new(UNIQUE_DIAG_ID, "example", 10)) + /// .register_diagnostic(Diagnostic::new(UNIQUE_DIAG_PATH)) /// .add_plugins(DiagnosticsPlugin) /// .run(); /// ``` diff --git a/crates/bevy_diagnostic/src/entity_count_diagnostics_plugin.rs b/crates/bevy_diagnostic/src/entity_count_diagnostics_plugin.rs index 8999163aa9720..163ae49b010ee 100644 --- a/crates/bevy_diagnostic/src/entity_count_diagnostics_plugin.rs +++ b/crates/bevy_diagnostic/src/entity_count_diagnostics_plugin.rs @@ -1,7 +1,7 @@ use bevy_app::prelude::*; use bevy_ecs::entity::Entities; -use crate::{Diagnostic, DiagnosticId, Diagnostics, RegisterDiagnostic}; +use crate::{Diagnostic, DiagnosticPath, Diagnostics, RegisterDiagnostic}; /// Adds "entity count" diagnostic to an App #[derive(Default)] @@ -9,16 +9,15 @@ pub struct EntityCountDiagnosticsPlugin; impl Plugin for EntityCountDiagnosticsPlugin { fn build(&self, app: &mut App) { - app.register_diagnostic(Diagnostic::new(Self::ENTITY_COUNT, "entity_count", 20)) + app.register_diagnostic(Diagnostic::new(Self::ENTITY_COUNT)) .add_systems(Update, Self::diagnostic_system); } } impl EntityCountDiagnosticsPlugin { - pub const ENTITY_COUNT: DiagnosticId = - DiagnosticId::from_u128(187513512115068938494459732780662867798); + pub const ENTITY_COUNT: DiagnosticPath = DiagnosticPath::const_new("entity_count"); pub fn diagnostic_system(mut diagnostics: Diagnostics, entities: &Entities) { - diagnostics.add_measurement(Self::ENTITY_COUNT, || entities.len() as f64); + diagnostics.add_measurement(&Self::ENTITY_COUNT, || entities.len() as f64); } } diff --git a/crates/bevy_diagnostic/src/frame_time_diagnostics_plugin.rs b/crates/bevy_diagnostic/src/frame_time_diagnostics_plugin.rs index a17a6a19d4c13..927e5e95b1c29 100644 --- a/crates/bevy_diagnostic/src/frame_time_diagnostics_plugin.rs +++ b/crates/bevy_diagnostic/src/frame_time_diagnostics_plugin.rs @@ -1,4 +1,4 @@ -use crate::{Diagnostic, DiagnosticId, Diagnostics, RegisterDiagnostic}; +use crate::{Diagnostic, DiagnosticPath, Diagnostics, RegisterDiagnostic}; use bevy_app::prelude::*; use bevy_core::FrameCount; use bevy_ecs::prelude::*; @@ -10,38 +10,32 @@ pub struct FrameTimeDiagnosticsPlugin; impl Plugin for FrameTimeDiagnosticsPlugin { fn build(&self, app: &mut bevy_app::App) { - app.register_diagnostic( - Diagnostic::new(Self::FRAME_TIME, "frame_time", 20).with_suffix("ms"), - ) - .register_diagnostic(Diagnostic::new(Self::FPS, "fps", 20)) - .register_diagnostic( - Diagnostic::new(Self::FRAME_COUNT, "frame_count", 1).with_smoothing_factor(0.0), - ) - .add_systems(Update, Self::diagnostic_system); + app.register_diagnostic(Diagnostic::new(Self::FRAME_TIME).with_suffix("ms")) + .register_diagnostic(Diagnostic::new(Self::FPS)) + .register_diagnostic(Diagnostic::new(Self::FRAME_COUNT).with_smoothing_factor(0.0)) + .add_systems(Update, Self::diagnostic_system); } } impl FrameTimeDiagnosticsPlugin { - pub const FPS: DiagnosticId = DiagnosticId::from_u128(288146834822086093791974408528866909483); - pub const FRAME_COUNT: DiagnosticId = - DiagnosticId::from_u128(54021991829115352065418785002088010277); - pub const FRAME_TIME: DiagnosticId = - DiagnosticId::from_u128(73441630925388532774622109383099159699); + pub const FPS: DiagnosticPath = DiagnosticPath::const_new("fps"); + pub const FRAME_COUNT: DiagnosticPath = DiagnosticPath::const_new("frame_count"); + pub const FRAME_TIME: DiagnosticPath = DiagnosticPath::const_new("frame_time"); pub fn diagnostic_system( mut diagnostics: Diagnostics, time: Res